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

[14.0, 15.0] account_move_name_sequence: default of account.journal field 'refund_sequence' not respected in method 'create' #1465

Closed
azoellner opened this issue Sep 4, 2022 · 6 comments
Labels

Comments

@azoellner
Copy link
Member

In module "account_move_name_sequence" (of branches "14.0" and "15.0"), the default of account.journal field refund_sequence is changed to True. (Compare e.g. the original in module "account" of branch "14.0"). But this changed default value then is not taken into account in the overwritten method create().

Expected behavior

There should actually be a refund_sequence_id whenever the refund_sequence is True.

Current behavior

If not explicitly providing the refund_sequence (and also no explicit refund_sequence_id) when creating an account.journal record of type "sale" or "purchase", no refund_sequence_id will get created, but refund_sequence will be True.

Causes

In module "account", the value of field refund_sequence is explicitly added to the argument vals during create(), by calling _fill_missing_values(): For "sale" and "purchase" journals, it will be True, and False for all other journal types.
But here in module "account_move_name_sequence", this super method create() is called (and correctly so) only after the creation of the sequences.
As a consequence, if not explicitly providing refund_sequence (and also no refund_sequence_id) when creating an account.journal record of type "sale" or "purchase", no refund_sequence_id will get created (because vals.get("refund_sequence") will be False). But then during the super call, field refund_sequence will be set to True.

Proposed fix

As a fix, do add True as the explicit default when getting refund_sequence from the vals:

             vals["sequence_id"] = self._create_sequence(vals).id
         if (
             vals.get("type") in ("sale", "purchase")
-            and vals.get("refund_sequence")
+            and vals.get("refund_sequence", True)
             and not vals.get("refund_sequence_id")
         ):
             vals["refund_sequence_id"] = self._create_sequence(vals, refund=True).id

In general, getting a value from the vals should always be done with the same default as the field's definition.

@azoellner azoellner added the bug label Sep 4, 2022
RodrigoBM added a commit to factorlibre/account-financial-tools that referenced this issue Nov 28, 2022
RodrigoBM added a commit to factorlibre/account-financial-tools that referenced this issue Nov 28, 2022
RodrigoBM added a commit to factorlibre/account-financial-tools that referenced this issue Nov 28, 2022
RodrigoBM added a commit to factorlibre/account-financial-tools that referenced this issue Nov 28, 2022
@RodrigoBM
Copy link
Contributor

RodrigoBM commented Nov 30, 2022

Same bug in V16.0 i am resolve it in PR #1510 in commit df45bac

@rafaelbn
Copy link
Member

@moylop260 do you have this issue?

@moylop260
Copy link
Contributor

@rafaelbn

No, I was not aware

@RodrigoBM
Could you fix it in v14.0 and v15.0, please?

RodrigoBM added a commit to factorlibre/account-financial-tools that referenced this issue Dec 2, 2022
@RodrigoBM
Copy link
Contributor

RodrigoBM commented Dec 2, 2022

@moylop260 done

RodrigoBM added a commit to factorlibre/account-financial-tools that referenced this issue Dec 2, 2022
@RodrigoBM
Copy link
Contributor

@moylop260 close this issue, thanks

@moylop260
Copy link
Contributor

Fixed from #1520 and #1521

RodrigoBM added a commit to factorlibre/account-financial-tools that referenced this issue Dec 8, 2022
ddejong-therp pushed a commit to ddejong-therp/account-financial-tools that referenced this issue Aug 31, 2023
andreagidaltig pushed a commit to vauxoo-dev/account-financial-tools that referenced this issue Sep 30, 2023
andreagidaltig pushed a commit to vauxoo-dev/account-financial-tools that referenced this issue Oct 9, 2023
andreagidaltig pushed a commit to vauxoo-dev/account-financial-tools that referenced this issue Oct 9, 2023
moitabenfdz pushed a commit to DynAppsNV/account-financial-tools that referenced this issue Nov 13, 2023
keylor2906 pushed a commit to vauxoo-dev/account-financial-tools that referenced this issue Dec 5, 2023
keylor2906 pushed a commit to Vauxoo/account-financial-tools that referenced this issue Dec 5, 2023
BertVGroenendael pushed a commit to DynAppsNV/account-financial-tools that referenced this issue Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants