[18.0][MIG] pos_margin: Migration to 18.0

[Tisho99]

Standard migration

T-9200

[cvinh]

Hello @Tisho99
I think margins are already in pos orders with Odoo standard v18
Or maybe you have something in mind ?

[Tisho99]

Hello @cvinh

Thank you for your comment

I've been reviewing the standard margin feature in pos orders. The margin is properly displayed in the backend, and in the product info of the TPV.

However, the pos_margin module adds the margin of the pos order in the main TPV screen, under the order total amount, and I haven't found that feature in standard Odoo. Do you know if I'm missing something?

If the module still need to be migrated, I can improve the README to make this distinction clearer.

[Jaimermaccione]

@Tisho99 Reviewed in a local environment.

The module works as expected:

• Margins are shown in the PoS frontend (configurable on/off from settings).
• Margins are visible in the PoS sales analysis.
• Margins are also shown in the PoS order tree and form views.

It might be worth mentioning the last point in the README, as it’s not currently documented.

Regarding the README images, they don’t seem to load properly. Not sure if this is expected, but it could be clearer to either fix them or replace them with plain links.

Once these two README points are clarified (documenting the additional views and improving or simplifying the image references), I think the module would be fully ready from a functional perspective.

Thanks!

[Tisho99]

Hello @Jaimermaccione

The images can't be changed. The OCA's precommit auto-generates the image links, and i do not have control over that. By the way, I think once the PR is merged, the images would be displayed.

The margin fields of the tree and form views of the pos order are not added by this module, they are added by Odoo point of sale module. I'll add a note in the roadmap, all the reviewers are being confused about this

[Jaimermaccione]

Hello @Jaimermaccione

The images can't be changed. The OCA's precommit auto-generates the image links, and i do not have control over that. By the way, I think once the PR is merged, the images would be displayed.

The margin fields of the tree and form views of the pos order are not added by this module, they are added by Odoo point of sale module. I'll add a note in the roadmap, all the reviewers are being confused about this

@Tisho99

Got it, thanks for the clarification. Regarding the images, no problem at all — I wasn’t aware of that OCA precommit behavior. That’s perfectly fine then.

About the second point, I partly agree with you: the margin field in the form view may indeed come from the core PoS module. However, in my tests, the list (tree) view margin column disappears when uninstalling pos_margin. Unless there is some native toggle that gets disabled on uninstall, this seems to indicate that pos_margin is responsible for adding it to the list view.

Could you double-check this and let me know your thoughts?

Thanks you!

[Tisho99]

@Jaimermaccione

You are right, this module adds the margin field to the tree view. I have added it to the readme's description

I have also added the roadmap

Can you review?

[Jaimermaccione]

Thanks for the update, @Tisho99

Sorry to be a bit picky here, but I think the sentence
“The margin rate (%) is also included in the POS order report, and pos order tree”
doesn’t fully reflect what is actually shown and can be a bit confusing.

In practice, the margin rate (%) is included in the POS order report, while the margin amount (monetary value) is displayed in the POS order tree view.

Maybe something like:
“The margin rate (%) is included in the POS order report, while the margin amount is displayed in the POS order tree view.”

Apart from that, everything looks good to me. Thanks!

[Jaimermaccione]

@Tisho99 I’ve checked that you’ve already applied the changes to the README. From my side, the module is functionally ready. LGTM!

[ValentinVinagre]

@Tisho99 review the tests please

[cvinh]

@Tisho99 review the tests please

@ValentinVinagre I think tests failed because of something else in __w/pos/pos/pos_display_order_number/tests/test_frontend.py
@Tisho99 can you rebase please ?

[pedrobaeza]

/ocabot migration pos_margin

[cristina-hidalgo-tecnativa]

Tested locally both at code level and functionally — everything looks good to me.

The CI failures are not related to this module. They're caused by a pre-existing bug in the OCB, which was fixed in odoo/odoo#241122 and already applied in the OCB. The POS session fails to load entirely because of this, which is why the frontend tours time out in cascade.

A rebase against the updated base branch should fix the CI — it would also be a good opportunity to clean up the bot commits.

[Tisho99]

Hello @cristina-hidalgo-tecnativa

Thank you for the review

I have rebased to correct the CI. I have also cleaned the bot commits