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] agreement_rebate: Migration to 16.0 #21

Merged
merged 19 commits into from
Sep 17, 2023

Conversation

ao-landoo
Copy link

Standard migration

@rp-landoo

domain="[('domain', '=', domain)]"
options="{'no_create': True}"
widget="many2many_tags"
nolabel="1"

Choose a reason for hiding this comment

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

Suggested change
nolabel="1"
nolabel="1"
colspan="2"

domain="[('agreement_type_id.domain', '=', domain)]"
options="{'no_create': True}"
widget="many2many_tags"
nolabel="1"

Choose a reason for hiding this comment

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

Suggested change
nolabel="1"
nolabel="1"
colspan="2"

domain="[('line_ids.agreement_id.agreement_type_id.domain', '=', domain)]"
options="{'no_create': True}"
widget="many2many_tags"
nolabel="1"

Choose a reason for hiding this comment

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

Suggested change
nolabel="1"
nolabel="1"
colspan="2"

widget="many2many_tags"
domain="[('type', '=', domain)]"
options="{'no_create': True}"
nolabel="1"

Choose a reason for hiding this comment

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

Suggested change
nolabel="1"
nolabel="1"
colspan="2"

domain="[('domain', '=', domain)]"
options="{'no_create': True}"
widget="many2many_tags"
nolabel="1"

Choose a reason for hiding this comment

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

Suggested change
nolabel="1"
nolabel="1"
colspan="2"

domain="[('agreement_type_id.domain', '=', domain)]"
options="{'no_create': True}"
widget="many2many_tags"
nolabel="1"

Choose a reason for hiding this comment

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

Suggested change
nolabel="1"
nolabel="1"
colspan="2"

raise UserError(
_("Please define an accounting sales journal for" " this company.")
)
vinvoice = self.env["account.move"].new(
Copy link
Member

Choose a reason for hiding this comment

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

In this version, now you can avoid this and directly use the original dictionary. See for example:

https://github.com/odoo/odoo/blob/2af74c260a6223256e3a975f1378bfa25728edfb/addons/account/tests/common.py#L674

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I'm not getting your point, on V15 same function exists

https://github.com/odoo/odoo/blob/15.0/addons/account/tests/common.py#L644

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but it was not complete and fill all the data. Now they are thanks to the computed writable fields.

Copy link
Member

Choose a reason for hiding this comment

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

Check a similar conversion in OCA/l10n-spain@3b66181

Copy link

@stefan-tecnativa stefan-tecnativa left a comment

Choose a reason for hiding this comment

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

Functionally tested. LGTM

Copy link

@rp-landoo rp-landoo left a comment

Choose a reason for hiding this comment

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

Functional approval

@rp-landoo
Copy link

@pedrobaeza could we merge this PR?

@pedrobaeza
Copy link
Member

@sergio-teruel can you confirm that everything is OK?

Copy link
Contributor

@sergio-teruel sergio-teruel left a comment

Choose a reason for hiding this comment

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

Ok.. ready... my comments only was minimal

@@ -0,0 +1,5 @@
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
from . import account_invoice
Copy link
Contributor

Choose a reason for hiding this comment

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

Change filename to to account_move

store=True,
readonly=False,
)
rebate_discount = fields.Float()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
rebate_discount = fields.Float()
rebate_discount = fields.Float(digits='Discount', default=0.0,)

@rp-landoo
Copy link

Is mergeable? @sergio-teruel

@pedrobaeza
Copy link
Member

There are 2 comments to be attended.

@sergio-teruel
Copy link
Contributor

Please @rp-landoo Can you attend the comments ??

@rp-landoo
Copy link

@mof-landoo Can you attend the comments ??

@mof-landoo mof-landoo force-pushed the 16.0-mig-agreement_rebate branch 2 times, most recently from 149fec9 to 0ce3196 Compare September 15, 2023 14:07
@mof-landoo
Copy link

@sergio-teruel changes applied, can you merge it? thanks

@sergio-teruel
Copy link
Contributor

ping @pedrobaeza

@pedrobaeza
Copy link
Member

/ocabot migration agreement_rebate
/ocabot merge nobump

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Sep 17, 2023
@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-21-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot mentioned this pull request Sep 17, 2023
5 tasks
@OCA-git-bot OCA-git-bot merged commit 275c7c5 into OCA:16.0 Sep 17, 2023
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

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.

None yet

9 participants