-
Notifications
You must be signed in to change notification settings - Fork 17
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] l10n_be_national_number #318
base: 16.0
Are you sure you want to change the base?
Conversation
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
…never used Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
09c3dc2
to
8bc56d5
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## 16.0 #318 +/- ##
=======================================
Coverage 94.17% 94.17%
=======================================
Files 34 34
Lines 584 584
Branches 56 56
=======================================
Hits 550 550
Misses 27 27
Partials 7 7 ☔ View full report in Codecov by Sentry. |
73e820b
to
39ec13a
Compare
@huguesdk thanks for the review, PR fixed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for fixing the migration script. sorry, i didn’t look close enough into it the first time, so here are more remarks.
old_id = env.ref("l10n_be_national_number.l10n_be_national_number_category") | ||
new_id = env.ref( | ||
"l10n_be_partner_identification.l10n_be_national_registry_number_category" | ||
).id | ||
for id_numbers in ( | ||
env["res.partner"] | ||
.with_context(active_test=False) | ||
.search([]) | ||
.mapped("id_numbers") | ||
): | ||
if id_numbers.category_id == old_id: | ||
id_numbers.category_id = new_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, i think this won’t work if there are multiple id_numbers
on a partner, which can be the case as its a one2many relationship. instead, you should work with res.partner.id_number
records directly. one way is to do it like this:
old_id = env.ref("l10n_be_national_number.l10n_be_national_number_category") | |
new_id = env.ref( | |
"l10n_be_partner_identification.l10n_be_national_registry_number_category" | |
).id | |
for id_numbers in ( | |
env["res.partner"] | |
.with_context(active_test=False) | |
.search([]) | |
.mapped("id_numbers") | |
): | |
if id_numbers.category_id == old_id: | |
id_numbers.category_id = new_id | |
old_id = env.ref("l10n_be_national_number.l10n_be_national_number_category").id | |
new_id = env.ref( | |
"l10n_be_partner_identification.l10n_be_national_registry_number_category" | |
).id | |
id_numbers = ( | |
env["res.partner.id_number"] | |
.with_context(active_test=False) | |
.search([("category_id", "=", old_id)]) | |
) | |
id_numbers.write({"category_id": new_id}) |
another is like this:
old_id = env.ref("l10n_be_national_number.l10n_be_national_number_category") | |
new_id = env.ref( | |
"l10n_be_partner_identification.l10n_be_national_registry_number_category" | |
).id | |
for id_numbers in ( | |
env["res.partner"] | |
.with_context(active_test=False) | |
.search([]) | |
.mapped("id_numbers") | |
): | |
if id_numbers.category_id == old_id: | |
id_numbers.category_id = new_id | |
old_id = env.ref("l10n_be_national_number.l10n_be_national_number_category").id | |
new_id = env.ref( | |
"l10n_be_partner_identification.l10n_be_national_registry_number_category" | |
).id | |
env.cr.execute( | |
""" | |
update res_partner_id_number | |
set category_id = %(old_id)s | |
where category_id = %(new_id}s | |
""", | |
{"old_id": old_id, "new_id": new_id}, | |
) |
39ec13a
to
8d0aef7
Compare
@huguesdk fixed ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
/ocabot merge nobump |
On my way to merge this fine PR! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
functional check: LGTM
@victor-champonnois your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-318-by-victor-champonnois-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. |
/ocabot merge nobump |
Hey, thanks for contributing! Proceeding to merge this for you. |
@victor-champonnois your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-318-by-victor-champonnois-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. |
This PR has the |
l10n_be_national_number was implemented in v14 and published on https://github.com/OCA/l10n-belgium.
In v16, there is the module https://github.com/OCA/l10n-belgium/tree/16.0/l10n_be_partner_identification that does the same thing with more functionalities.
We need to migrate the data. The module l10n_be_national_number is moved here, and contain a migration script and a dependency to l10n_be_partner_identification.
internal task
https://gestion.coopiteasy.be/web?debug=1#id=12259&action=479&model=project.task&view_type=form&menu_id=536