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

[18.0][MIG] l10n_eu_product_adr: Migration to 18.0 #208

Merged
merged 22 commits into from
Nov 19, 2024

Conversation

chaule97
Copy link

@chaule97 chaule97 commented Oct 9, 2024

  • I removed the test case test_06_product_variant_migration because there is no more pre-migration in Odoo 16+.
  • I need to update the _compute_display_name method in AdrClass and AdrGoods because the name_search method in Odoo 18 has changed and use display_name value to return result.

@chaule97 chaule97 force-pushed the 18.0-mig-l10n_eu_product_adr branch 4 times, most recently from 7b6dc21 to 0c9c60f Compare October 10, 2024 08:16
@chaule97 chaule97 force-pushed the 18.0-mig-l10n_eu_product_adr branch from 0c9c60f to 3dccc14 Compare October 15, 2024 09:11
@@ -19,24 +19,18 @@
name="adr"
string="Dangerous Goods"
groups="l10n_eu_product_adr.group_adr_goods_user"
attrs="{'invisible': ['|', ('type', '!=', 'product'), '&', ('is_dangerous', '=', False), ('adr_goods_on_variants', '=', False)]}"
invisible="not (is_storable and (is_dangerous or adr_goods_on_variants))"
Copy link
Member

Choose a reason for hiding this comment

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

are we sure this is the same ?
I would have written this not is_storable or (is_dangerous and adr_goods_on_variants)

Copy link
Author

Choose a reason for hiding this comment

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

no, it will be: not is_storable or (not is_dangerous and not adr_goods_on_variants)

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the change it looks better IMO

@chaule97 chaule97 force-pushed the 18.0-mig-l10n_eu_product_adr branch 2 times, most recently from 94b6550 to 8a6ce61 Compare October 18, 2024 09:14
@chaule97 chaule97 requested a review from mmequignon October 18, 2024 09:15
Copy link
Member

@TDu TDu left a comment

Choose a reason for hiding this comment

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

Testing on runboat I have an issue when activating the Is Dangerous flag on a product...the Dangerous Goods tab is empty.

@chaule97 chaule97 force-pushed the 18.0-mig-l10n_eu_product_adr branch from 8a6ce61 to 7d47552 Compare October 21, 2024 04:15
@chaule97
Copy link
Author

Testing on runboat I have an issue when activating the Is Dangerous flag on a product...the Dangerous Goods tab is empty.

Hi @TDu ,I have checked the differences between PR 16.0 and branch 14.0. The PR 16.0 had incorrect group permissions. I have updated. thank you

@chaule97 chaule97 requested a review from TDu October 21, 2024 04:23
@chaule97 chaule97 changed the title [18.0][MIG] l10n_eu_product_adr [18.0][MIG] l10n_eu_product_adr: Migration to 18.0 Oct 21, 2024
@chaule97 chaule97 force-pushed the 18.0-mig-l10n_eu_product_adr branch from 7d47552 to f5dfe98 Compare October 25, 2024 04:22
Comment on lines 163 to 170

# def test_06_product_variant_migration(self):
# """Test the migration of the adr fields from template to product"""
# try:
# from openupgradelib import openupgrade # noqa: F401
# except ImportError:
# logging.getLogger("odoo.addons.l10n_eu_product_adr.tests").info(
# "OpenUpgrade not found, skip test"
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't drop this test, as it probably had meaning in the past.
It is probably meant to be activated again once code is fixed.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @mmequignon , I think this was the test that went with the migration code for version 14, it's obsolete/not needed now

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@simahawk
Copy link

/ocabot migration l10n_eu_product_adr
/ocabot merge nobump

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Nov 19, 2024
@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 18.0-ocabot-merge-pr-208-by-simahawk-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Nov 19, 2024
Signed-off-by simahawk
@OCA-git-bot OCA-git-bot mentioned this pull request Nov 19, 2024
11 tasks
@OCA-git-bot
Copy link
Contributor

@simahawk your merge command was aborted due to failed check(s), which you can inspect on this commit of 18.0-ocabot-merge-pr-208-by-simahawk-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@chaule97 chaule97 force-pushed the 18.0-mig-l10n_eu_product_adr branch from f5dfe98 to 86b91a3 Compare November 19, 2024 08:34
@chaule97
Copy link
Author

Ciao @simahawk , could you merge again please?

@simahawk
Copy link

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 18.0-ocabot-merge-pr-208-by-simahawk-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 414c5df into OCA:18.0 Nov 19, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at d184a36. 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.

10 participants