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

[12.0] ADD pos_product_multi_price: Make pricelists based on product_multi_price work in point of sale #541

Closed
wants to merge 1 commit into from

Conversation

eLBati
Copy link
Member

@eLBati eLBati commented Sep 28, 2020

See product_multi_price's documentation about how to configure and use it.

Copy link

@mrcast mrcast left a comment

Choose a reason for hiding this comment

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

Functional test ok!

@eLBati
Copy link
Member Author

eLBati commented Oct 7, 2020

@OCA/pos-maintainers what do you think?

@eLBati eLBati force-pushed the 12.0-pos_product_multi_price branch from 5255e54 to 0f2789c Compare November 12, 2020 13:16
@eLBati
Copy link
Member Author

eLBati commented Nov 12, 2020

Conflict fixed

Comment on lines +32 to +59
_.find(pricelist_items, function (rule) {
if (rule.min_quantity && quantity < rule.min_quantity) {
return false;
}
if (rule.base === 'multi_price' && rule.compute_price == 'formula') {
_.forEach(price_ids_json, function (multi_price) {
if (multi_price.price_id == rule.multi_price_name[0]) {
price = multi_price.price;
var price_limit = price;
price = price - (price * (rule.price_discount / 100));
if (rule.price_round) {
price = round_pr(price, rule.price_round);
}
if (rule.price_surcharge) {
price += rule.price_surcharge;
}
if (rule.price_min_margin) {
price = Math.max(price, price_limit + rule.price_min_margin);
}
if (rule.price_max_margin) {
price = Math.min(price, price_limit + rule.price_max_margin);
}
return true;
}
});
}
return false
});

Choose a reason for hiding this comment

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

Here you ignore possible high-priority rules not based on multi_price.
Price set by those high-priority rules will be overridden by the multi_price.

Copy link
Member Author

Choose a reason for hiding this comment

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

Doesn't the same happen with product_multi_price?

Choose a reason for hiding this comment

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

No it doesn't.
If you have a rule like "50% of standard price on product variant A" and a second rule "Use multi_price X for all products", when you create a sale order for product A, the price will be 50% of the standard price.
The second rule won't override the first one.

Copy link
Contributor

@ivantodorovich ivantodorovich left a comment

Choose a reason for hiding this comment

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

also check @qgroulard 's comment :)

Comment on lines +8 to +11
# technical field used in POS frontend
price_ids_json = fields.Char(
"Multi price data dict", readonly=True,
compute="_compute_price_ids_json")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# technical field used in POS frontend
price_ids_json = fields.Char(
"Multi price data dict", readonly=True,
compute="_compute_price_ids_json")
price_ids_json = fields.Char(
"Multi price data dict",
help="Technical field used in POS frontend",
compute="_compute_price_ids_json",
)
  • computed fields are already readonly by default.
  • you can move the comment to the help attribute, it's more helpful this way

Copy link

@Joaco1980 Joaco1980 left a comment

Choose a reason for hiding this comment

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

Works correctly also on V13

@github-actions
Copy link

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 Jan 30, 2022
@github-actions github-actions bot closed this Mar 6, 2022
@eLBati
Copy link
Member Author

eLBati commented Mar 20, 2022

Hi @legalsylvain any chance for this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants