Apply limitation price on subtotal as well #26
Conversation
|
Yup, it's a tricky one.. With your changes, it looks like conditions will be applied on the price twice, first within the Maybe apply the conditions on |
|
Yeah I wasnt too sure how to handle it so probably got caught between options. Just wanted to push some code to start the discussion.
I’ve updated it now, is that what you meant?
… On 4 Jan 2021, at 17:43, Sam Poyigi ***@***.***> wrote:
Yup, it's a tricky one..
With your changes, it looks like conditions will be applied on the price twice, first within the price method and then subtotal.. Not sure if that's what you intended.
Maybe apply the conditions on $optionSum before calculating the subtotal? But then that would affect the condition calculatedValue property value. I think we should apply conditions only within the subtotal, then we wont have to worry about calculatedValue returning different values...
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#26 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAMVO7VB2DP4YR4F5UZ3NDSYH42VANCNFSM4VQJIX3A>.
|
|
Seen! Shouldn't conditions be applied to the calculated subtotal not before? |
|
Pushed an update. |
|
Do NOT work for me... Please note that this happen only when we apply restrictions to some menu items |
|
Latest commit works and is tested. |
Was trying to work out the best way to handle this without affecting other areas, so settled on applying on subtotal as well as price.
The other option is to add the option price onto the price in the price() method ?