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

[17.0][MIG] maintenance_equipment_sequence #399

Merged
merged 28 commits into from
Aug 20, 2024

Conversation

Anxo82
Copy link

@Anxo82 Anxo82 commented Jun 3, 2024

Standard migration

AdriaGForgeFlow and others added 26 commits May 31, 2024 12:08
Currently translated at 100.0% (10 of 10 strings)

Translation: maintenance-11.0/maintenance-11.0-maintenance_equipment_sequence
Translate-URL: https://translation.odoo-community.org/projects/maintenance-11-0/maintenance-11-0-maintenance_equipment_sequence/es/
Currently translated at 100.0% (10 of 10 strings)

Translation: maintenance-13.0/maintenance-13.0-maintenance_equipment_sequence
Translate-URL: https://translation.odoo-community.org/projects/maintenance-13-0/maintenance-13-0-maintenance_equipment_sequence/it/
Currently translated at 100.0% (10 of 10 strings)

Translation: maintenance-16.0/maintenance-16.0-maintenance_equipment_sequence
Translate-URL: https://translation.odoo-community.org/projects/maintenance-16-0/maintenance-16-0-maintenance_equipment_sequence/es/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: maintenance-16.0/maintenance-16.0-maintenance_equipment_sequence
Translate-URL: https://translation.odoo-community.org/projects/maintenance-16-0/maintenance-16-0-maintenance_equipment_sequence/
@Anxo82 Anxo82 force-pushed the 17.0-mig-maintenance_equipment_sequence branch 4 times, most recently from c205b1d to f636a75 Compare June 4, 2024 12:57
@Anxo82 Anxo82 force-pushed the 17.0-mig-maintenance_equipment_sequence branch from f636a75 to 42597b4 Compare June 5, 2024 11:19
Copy link
Contributor

@dalonsod dalonsod left a comment

Choose a reason for hiding this comment

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

@Anxo82 see comment, and also, please add sequence_prefix to category tree and search view in a separate commit, if possible.

Comment on lines 156 to 164
cat_01._compute_equipment_code()

self.assertEqual(equipment_01.serial_no, "TST001")
self.assertEqual(equipment_02.serial_no, "TST002")
Copy link
Contributor

Choose a reason for hiding this comment

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

Test improvement suggestion:

Suggested change
cat_01._compute_equipment_code()
self.assertEqual(equipment_01.serial_no, "TST001")
self.assertEqual(equipment_02.serial_no, "TST002")
equipment_03 = self.env["maintenance.equipment"].create(
{
"name": "Test Equipment 3",
"category_id": False,
"serial_no": False,
}
)
self.assertEqual(equipment_01.serial_no, "TST001")
self.assertEqual(equipment_02.serial_no, "TST002")
self.assertFalse(equipment_03.serial_no)
equipment_03.write({"category_id": cat_01.id})
self.assertEqual(equipment_03.serial_no, "TST003")

_compute_equipment_code() isn't needed to be directly called because create() and write() actually call it. With third equipment case test pending uncovered line will be covered, if I'm not wrong.

@Anxo82 Anxo82 force-pushed the 17.0-mig-maintenance_equipment_sequence branch 2 times, most recently from b5aabde to a2b0350 Compare June 17, 2024 06:50
@Anxo82
Copy link
Author

Anxo82 commented Jun 17, 2024

@dalonsod could you review please.
Thanks

Copy link
Contributor

@dalonsod dalonsod left a comment

Choose a reason for hiding this comment

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

See latest comment, not blocking, I've approved it anyway

<field name="inherit_id" ref="maintenance.hr_equipment_category_view_tree" />
<field name="arch" type="xml">
<field name="company_id" position="after">
<field name="sequence_prefix" />
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd only make a little extra change: making new tree field optional:

Suggested change
<field name="sequence_prefix" />
<field name="sequence_prefix" optional="show" />

@Anxo82 Anxo82 force-pushed the 17.0-mig-maintenance_equipment_sequence branch from a2b0350 to ba7e914 Compare June 17, 2024 07:05
@Anxo82
Copy link
Author

Anxo82 commented Jun 17, 2024

@AdriaGForgeFlow, @NuriaXifre could you review please?
Thanks.

Copy link

@peluko00 peluko00 left a comment

Choose a reason for hiding this comment

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

Please, squash migration commits into one, LGTM

@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). 🤖

@dalonsod
Copy link
Contributor

Please, squash migration commits into one, LGTM

Hello @peluko00 if you are referring to the two latest commits, keeping them separated makes possible porting added functionality to previous Odoo versions, if needed

Copy link

@JulienMartinez JulienMartinez left a comment

Choose a reason for hiding this comment

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

test on the runboat
LGTM

@dalonsod
Copy link
Contributor

Hello @etobella could you merge this PR? Thanks!

@dalonsod
Copy link
Contributor

Hello @gurneyalex could you merge this PR? Thanks!

@etobella
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 8094093 into OCA:17.0 Aug 20, 2024
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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