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

[16.0][MIG] account_move_name_sequence: Migration to 16.0 #1510

Merged
merged 34 commits into from
Dec 8, 2022

Conversation

RodrigoBM
Copy link
Contributor

@RodrigoBM RodrigoBM commented Nov 28, 2022

#1472

Add fix return sequence names do not add a space between name and returns

And add fix comment in issue #1465

And add fix comment in issue #1501 and comment #1510 (comment)

alexis-via and others added 30 commits November 22, 2022 14:36
This module restores the good old behavior where journal entry numbers
were generated from a sequence configured on the journal.
Add post-install script to create a sequence for all existing journals
Update README accordingly
…ange_day

More info about:
 - odoo/odoo#91019

TODO: Revert this commit after it is merged
After remove required=True for journal.sequence_id field it is possible to post an invoice with misconfigured journal with empty sequence

So, this constraint will raise an error for this kind of cases since that using '/' could raise the unique constraint for all other moves
The required flag was wrong for sequence_id and refund_sequence_id

So, it was not possible to store any change in journal for journal different to sale and purchase
…efix

In case you want name your invoice YYYY-MM-SEQ (ie: 2022-07-00001)
where:
 * YYYY: is the account move year
 * MM: is the account move month
 * SEQ: is a numerical sequence that is continue along the fiscal year
   assuming fiscal year is over two years (ie: from july to june next year)

Before this commit the sequence prefix use now() to be compute but the
range is selected with the account move date.

This commit make consistency computing prefix with the account
move date as well.

So account move manage the first janunary for the last day of
the previous year will properly use the account move date.

Co-authored-by: Alexis de Lattre <[email protected]>
@rafaelbn
Copy link
Member

/ocabot migration account_move_name_sequence

@rafaelbn rafaelbn requested a review from moylop260 November 30, 2022 16:51
@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Nov 30, 2022
@moylop260
Copy link
Contributor

moylop260 commented Dec 1, 2022

For v16.0 the new PR is fixing almost concurrency issues:

The README of account_move_name_sequence module for v16.0 could be deprecated

I mean, account_move_name_sequence could be good for v16.0 only if you like to allow gaps

It was asked (without answer) in the original PR:

e.g. we have v11.0 instances with custom modules migrated to v14.0 failing with the following case of use:

Subscriptions

  1. Create invoice and post
  2. Create payment and post
  3. Reconcile

Shop

  1. Create Sale Order
  2. Create payment and post
  3. Create invoice and post
  4. Reconcile

(This case was considered in test_sequence_concurrency_95_pay2inv_inv2pay from odoo/odoo#104647)

It could be fixed if you allow gaps for the journal of these kinds of payments

Even if the bottleneck was already fixed in v16.0

you don't have the allow-gaps option anymore, so, the module account_move_name_sequence could be helpful for these cases

So, the README of account_move_name_sequence needs to be rewrite

@alexis-via
Copy link
Contributor

Thanks for this migration to v16. I'm testing your PR and it fails in the following scenario:

  1. create a supplier invoice and confirm it. Keep it as unpaid.
  2. click on "Add credit note". Keep default params of the wizard and click on "Reverse"
    => you get a draft supplier refund
  3. Click on "Confirm". Get you this error:
The operation cannot be completed: A move can not be posted with name "/" or empty value
Check the journal sequence, please

The bug doesn't happen when the supplier refund is created manually from scratch.

@RodrigoBM RodrigoBM force-pushed the 16.0-mig-account_move_name_sequence branch from 105326e to dc30382 Compare December 2, 2022 11:59
@RodrigoBM
Copy link
Contributor Author

Hi @alexis-via

Yea, its error its same this issue #1501 for v14 and v15.

I put de fix #1514 but this not working in v16.

I have added another fix that does solve this case and I have added a test.

@RodrigoBM RodrigoBM force-pushed the 16.0-mig-account_move_name_sequence branch 2 times, most recently from 4971302 to ad51021 Compare December 2, 2022 15:18
@RodrigoBM RodrigoBM changed the title [16.0][MIG] account_move_name_sequence [16.0][MIG] account_move_name_sequence: Migration to 16.0 Dec 4, 2022
@alexis-via
Copy link
Contributor

The problem is fixed, thank you!
I noticed this warning when you start odoo:

odoo.api.create: The model odoo.addons.account_move_name_sequence.models.account_journal is not overriding the create method in batch

Copy link

@andreampiovesana andreampiovesana left a comment

Choose a reason for hiding this comment

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

LGTM
functional test ok

@RodrigoBM RodrigoBM force-pushed the 16.0-mig-account_move_name_sequence branch from ad51021 to ab8e59d Compare December 8, 2022 10:37
@RodrigoBM
Copy link
Contributor Author

Hi @alexis-via,

I have checked the warning, the problem is that account.journal in V16 uses @api.model_create_multi for the create method.

I have added the changes to work with this @api.model_create_multi for the create method.

Regards

Copy link

@marcelofrare marcelofrare left a comment

Choose a reason for hiding this comment

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

runboat is ok

@moylop260
Copy link
Contributor

I think we could make the changes in the readme post-migration PR

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-1510-by-moylop260-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 3bfe850 into OCA:16.0 Dec 8, 2022
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 146c448. Thanks a lot for contributing to OCA. ❤️

@RodrigoBM RodrigoBM deleted the 16.0-mig-account_move_name_sequence branch December 9, 2022 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants