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_payment_order: Migration to 16.0 #972

Closed
wants to merge 167 commits into from

Conversation

wpichler
Copy link
Contributor

@wpichler wpichler commented Oct 25, 2022

#964

Needs #969

Standard Migration - just one thing - it was necessary to add groups="base.group_multi_company" to the order_id field in account_payment_line_form. Without this i do get this error:

Field 'company_id' used in domain of field 'order_id' ([('company_id', 'in', [company_id, False])]) is restricted to the group(s) base.group_multi_company.

order_id is check_company - so company_id domain gets added indirectly and should be ok without this group. Maybe because of the related company_id this problem occours ?

@pedrobaeza pedrobaeza changed the title 16.0 mig account payment order [16.0][MIG] account_payment_order: Migration to 16.0 Dec 15, 2022
@pedrobaeza
Copy link
Member

#1000 should be needed, but first is to have green CI. Check also the proper PR title that I have put.

@wpichler
Copy link
Contributor Author

#1000 was already included because of the review of dzungtran89

@pedrobaeza
Copy link
Member

And what about the CI?

@wpichler
Copy link
Contributor Author

CI in progress now

@wpichler wpichler force-pushed the 16.0-mig-account_payment_order branch from ec4f57d to 923ce50 Compare December 15, 2022 18:15
@wpichler
Copy link
Contributor Author

@pedrobaeza done - anything else ?

@pedrobaeza
Copy link
Member

Yes, please check pre-commit changes that are not complete, and please squash all your migration commits into one (except the first one [IMP] : pre-commit stuff that is missing the module name by the way.

@wpichler wpichler force-pushed the 16.0-mig-account_payment_order branch from 923ce50 to d5e34fd Compare December 15, 2022 18:41
@wpichler
Copy link
Contributor Author

@pedrobaeza please /ocabot merge

@OCA-git-bot
Copy link
Contributor

Hi @wpichler. Your command failed:

Required option bumpversion_mode for command merge. Possible values : major, minor, patch, nobump.

Ocabot commands

  • ocabot merge major|minor|patch|nobump
  • ocabot rebase
  • ocabot migration {MODULE_NAME}

More information

@fcvalgar
Copy link

fcvalgar commented Dec 23, 2022

I detected some wrong view in Create payment lines.

https://www.loom.com/share/ca4b41d0e9bf4257aeb38b433982f8e0

@wpichler
Copy link
Contributor Author

/ocabot migration account_payment_order

@wpichler
Copy link
Contributor Author

I detected some wrong view in Create payment lines.

https://www.loom.com/share/ca4b41d0e9bf4257aeb38b433982f8e0

I've corrected the wrong view

image

wpichler and others added 4 commits January 27, 2023 10:53
… communication

If some movements have been reconciled with the original invoice,
their references should be added in communication too.

e.g.: Manual credit notes
@wpichler wpichler force-pushed the 16.0-mig-account_payment_order branch from ff43c4d to 9b437af Compare January 27, 2023 09:53
@wpichler
Copy link
Contributor Author

Rebased - all checks are fine - requested changes are done. Please review and approve

@hildickethan
Copy link
Member

[FIX] Fixed bug from cherry-pick in the tests
[FIX] Make pylint_odoo happy - removed string attribute with same nam…
[FIX] Fixed account_internal_type usage
[FIX] Fixed bugs seen in the tests

these commits should be squashed into the migration commit

[IMP] Pre Commit Changes

should be squashed into the pre-commit stuff before the migration or if they are just fixing your migrated changes, then they should be part of the original migration too

i would also put the cherry-picked ones before the migration commit, in case anything has to be adapted to 16.0

AnizR and others added 2 commits February 14, 2023 12:14
[MIG] migration scripts
[IMP] report account_payment_order use t-field
[IMP] field company_id invisible on view account_payment_line
[IMP] create with @api.model_create_multi tag
@wpichler wpichler force-pushed the 16.0-mig-account_payment_order branch from 4711bf7 to 7ae61e2 Compare February 18, 2023 16:46
@wpichler
Copy link
Contributor Author

i've merge PR from @AnizR and pick squashed all commits into one migration commit. I hope this is the correct way do keep history of commits as it should be

@pedrobaeza
Copy link
Member

I'm afraid you have squashed too much. Commits like the account.payment generation should be isolated, or the extra fixes. Can you please undo that?

@wpichler
Copy link
Contributor Author

@pedrobaeza I've undone it - and have no idea how to squash these commits - any hint ?

@AntoniRomera
Copy link

Need any help?

@wpichler
Copy link
Contributor Author

Need any help?

Yes please

@AntoniRomera
Copy link

Show a picture of the commit history to see how you currently have the branch.

@@ -161,6 +164,18 @@ def _insert_payment_line_payment_link(env):

@openupgrade.migrate()
def migrate(env, version):
env.cr.execute(
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to execute this when this has already been executed during 14.0.2.0.0 migration?
This looks valid only if migrating from 14.0.1.0.0 to 16.0.1.0.0.
Same applies to 14.0.3.0.0

@pedrobaeza
Copy link
Member

I have put a bit of order + gather latest changes from previous branches on #1056, so closing this one and merging the other as soon as I get a green CI. Thanks all for your efforts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.