Skip to content

[MIG] sale_order_type_quotation_number: Migration to 16.0#3450

Open
SirAionTech wants to merge 6 commits intoOCA:16.0from
SirAionTech:16.0-mig-sale_order_type_quotation_number
Open

[MIG] sale_order_type_quotation_number: Migration to 16.0#3450
SirAionTech wants to merge 6 commits intoOCA:16.0from
SirAionTech:16.0-mig-sale_order_type_quotation_number

Conversation

@SirAionTech
Copy link
Copy Markdown

Standard migration from 15.0 (https://github.com/OCA/sale-workflow/tree/adfaf57811fdd6c2ce0ce148dcdc2a9c6b79222f/sale_order_type_quotation_number).

I have renamed some commit titles because they were too long, according to https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message:

please check if the commit message is cut with ellipsis

I have included #2719 because field quotation_seq_used is needed for the migrated module.

@SirAionTech SirAionTech marked this pull request as ready for review December 3, 2024 16:39
@SirAionTech
Copy link
Copy Markdown
Author

@manuelregidor @angelgarciadelachica @HaraldPanten @ValentinVinagre you worked on these modules, maybe you want to have a look?

@ValentinVinagre
Copy link
Copy Markdown
Contributor

@manuelregidor @angelgarciadelachica @HaraldPanten @ValentinVinagre you worked on these modules, maybe you want to have a look?

Please do not ping 4 people from the same company, 1 is enough 😆 . Could you add more tests to cover more cases? According to the codecov, there are still cases to be covered

@rousseldenis
Copy link
Copy Markdown
Contributor

/ocabot migration sale_order_type_quotation_number

@rousseldenis
Copy link
Copy Markdown
Contributor

/ocabot rebase

@OCA-git-bot
Copy link
Copy Markdown
Contributor

Congratulations, PR rebased to 16.0.

@OCA-git-bot OCA-git-bot force-pushed the 16.0-mig-sale_order_type_quotation_number branch from 0d714d7 to fb2a47d Compare May 30, 2025 08:11
@rousseldenis
Copy link
Copy Markdown
Contributor

@SirAionTech Do you plan to add more tests as asked? Thanks

Copy link
Copy Markdown

@alexey-pelykh alexey-pelykh left a comment

Choose a reason for hiding this comment

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

Thanks for the migration and for bundling the quotation_seq_used improvement from #2719 -- that's a clean approach.

A few observations after going through the diff:

create() in sale_order.py (new module)

The create method calls super().create([vals]) one record at a time inside a loop. This defeats @api.model_create_multi batch optimisation and will be noticeably slower when creating orders in bulk (e.g. from a website or import). Consider restructuring so super().create(vals_list) is called once and the applied_quotation_seq_id assignment is done in a second pass, similar to how the parent module already works.

Migration script hardcodes "SQ" prefix

sale_quotation_number/migrations/16.0.1.1.1/post-migration.py filters orders with .filtered(lambda a: a.name[:2] == "SQ"). This re-introduces the exact prefix-matching problem that the quotation_seq_used boolean was designed to eliminate. If someone changed their quotation sequence prefix, those draft orders will be missed by the migration. Using the sequence record's actual prefix (or a broader heuristic) would be safer.

.pot / .po still reference 15.0

Both i18n/sale_order_type_quotation_number.pot and i18n/es.po have Project-Id-Version: Odoo Server 15.0. Should be updated to 16.0 for consistency.

Test coverage

Echoing @ValentinVinagre's earlier comment -- the current tests cover the happy path well but miss a few scenarios: keep_name_so=True (should skip renumbering), order type without a quotation sequence (should fall back to default), and the copy() flow. Would be good to fill those gaps before merge.

The rest of the migration (manifest, views, setup scaffolding, get_sale_order_seq / get_quotation_seq overrides) looks correct to me.

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.

6 participants