Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[17.0][WIP] account_factoring_receivable_balance #555

Open
wants to merge 61 commits into
base: 17.0
Choose a base branch
from

Conversation

dreispt
Copy link
Member

@dreispt dreispt commented Jun 7, 2024

@dreispt dreispt force-pushed the 17-dr-factofrance branch 2 times, most recently from 03c79b7 to 75fc74b Compare June 7, 2024 08:35
@dreispt dreispt force-pushed the 17-dr-factofrance branch from b5be4a6 to 033f39a Compare June 7, 2024 11:27
bealdav and others added 26 commits June 7, 2024 12:28
ADD oca style settings

FIX misc pylint

UPD copier
@bealdav
Copy link
Member

bealdav commented Jun 19, 2024

Thanks for porting this module. Nice Improvements

I asked myself if this module, is not generic enough to move an account OCA repository ?

Happy to hear there

@dreispt dreispt force-pushed the 17-dr-factofrance branch 2 times, most recently from 08633bd to bd299ff Compare July 4, 2024 20:31
@dreispt dreispt force-pushed the 17-dr-factofrance branch from bd299ff to 86dd4e4 Compare July 4, 2024 20:59
@bealdav
Copy link
Member

bealdav commented Jul 16, 2024

@dreispt as I proposed account_factoring_receivable_balance can move to a non localization repo and other modules could stay here . What do you think ?

But it can be done at the end of your process here

@dreispt
Copy link
Member Author

dreispt commented Jul 16, 2024

@bealdav I agree in principle, but in the immediate term it is easier to work and refactor if they are in the same repo.
Once this is stabilized and before a merge, we can introduce those changes.

factor_journal_id = fields.Many2one(
comodel_name="account.journal",
string="Journal",
domain="[('factor_type', '!=', False)," "('type', '=', 'bank')]",
Copy link
Member

@bealdav bealdav Jul 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dreispt
In my experience (minor) but the journal associated to account.move used to send data to factor etablishment (invoices and credit notes) can be a misc one.
In this kind of journal you have not a lot settings, then it's less confusing to use this kind in a such context.
At least, we may consider to offer a choice.

A bank type is require to produce moves with statements data, that's not the same flow

Don't you think Daniel ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bealdav Good question, this was something I struggled with.
For FactoFrance we need to also report the payments on Invoices.
So I settled with using a Bank journal, allowing you to post payments on factored invoices.

Copy link
Member

@rvalyi rvalyi Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bealdav Good question, this was something I struggled with.
For FactoFrance we need to also report the payments on Invoices.
So I settled with using a Bank journal, allowing you to post payments on factored invoices.

@dreis for Factofrance I strongly recommend you look at this other module I did initially, it is used in production since 3 years in a very big company and was designed for FactoFrance: https://github.com/akretion/odoo-factoring/tree/14.0/account_factoring

You can find a demo here: https://youtu.be/N7x7MtfEF1g?si=0FbU3P1O8Iv8MgzE


factor_journal_id = fields.Many2one(
comodel_name="account.journal",
domain="[('factor_type', '!=', False)]",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required in multi-company environment

Suggested change
domain="[('factor_type', '!=', False)]",
domain="[('factor_type', '!=', False)]",
company_dependent=True,

)
partner_identification_type = fields.Selection(
selection=[("1", "1 SIRET"), ("4", "4 TVA")],
)
Copy link
Member

@bealdav bealdav Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dreispt FYI in my repo I have made this settings to match with arbitrary factor keys.
image

https://github.com/akretion/odoo-factoring/pull/21/files#diff-2bf0078cef96b202cab38f4dcae616055927c9c3e252cc6f6b1f74a280737c02R18-R39

May be it could be generic ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO that is a bit complicated.
I would go for the simpler solution, and add those additional settings as fields, only visible for tis Factor Type.
Note that the "Factor Code" is really a "Factor Contract Number", so renaming it could make sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends on your factor settings, in your case you only have one key but like any carrier dev you may have misc keys with unpredictable name or mean unshareable. If you have several factors for different companies in your ERP it becomes crazy.
But yes, it could be not so generic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the DB will have all those fields, but the user form would show only the relevant ones for the selected Factor.
Like what the shipping connectors do, as you pointed out.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A matter of taste

Copy link

github-actions bot commented Jan 5, 2025

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs review stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants