Skip to content

Conversation

@edescalona
Copy link

@BinhexTeam

This new module allows you to add two new fields for the sales margin in the Product Margins report.

…gin in the Product Margins report

[IMP] product_margin_report_sale_margin: Add tests
@edescalona edescalona force-pushed the 17.0-add-product_margin_report_sale_margin branch from 0500fb1 to 64a5592 Compare July 10, 2025 16:51
Copy link

@christian-ramos-tecnativa christian-ramos-tecnativa left a comment

Choose a reason for hiding this comment

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

LGTM! I’d like to add more context to these two new fields. Currently, this report includes all sales and purchases in the range by default when calculating the values.
For example, if I’ve purchased 100 tables but only 10 have been sold so far, the margin will appear negative, which can be misleading. In this case, the report seems to reflect more of a cost vs. revenue view of the product rather than actual profitability.
Here’s a screenshot to illustrate the scenario:
image
This new values add a lot of value to this report.
Thanks

@github-actions
Copy link

github-actions bot commented Nov 9, 2025

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Nov 9, 2025
Copy link

@rrebollo rrebollo left a comment

Choose a reason for hiding this comment

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

Code Review: Great work! The code looks good to me (LGTM). Thank you for your contribution! I've provided a few suggestions for your consideration—feel free to address them as you see fit.

Copy link

@adasatorres adasatorres left a comment

Choose a reason for hiding this comment

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

Code review, LGTM!

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Dec 14, 2025
@edescalona
Copy link
Author

ping @legalsylvain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants