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

[16.0][ADD] website_sale_loyalty_suggestion_wizard_multi_gift: New Module #211

Open
wants to merge 2 commits into
base: 16.0
Choose a base branch
from

Conversation

pilarvargas-tecnativa
Copy link
Contributor

@pilarvargas-tecnativa pilarvargas-tecnativa commented Apr 25, 2024

@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-add-website_sale_loyalty_suggestion_wizard_multi_gift branch from c92edbd to 8873474 Compare April 25, 2024 10:48
@pedrobaeza pedrobaeza added this to the 16.0 milestone Jul 12, 2024
@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-add-website_sale_loyalty_suggestion_wizard_multi_gift branch from 8873474 to 7001873 Compare December 13, 2024 06:37
Copy link

@carlos-lopez-tecnativa carlos-lopez-tecnativa left a comment

Choose a reason for hiding this comment

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

Functionally tested, just a minor change to make it clearer which product is selected when using variants.

Comment on lines +24 to +39
t-call="website_sale_loyalty_suggestion_wizard.promotion_wizard_reward_product"
>
<t
t-set="reward_qty"
t-value="multi_gift_line.reward_product_quantity"
/>
<t t-set="product" t-value="product" />
<t t-set="id" t-value="multi_gift_line.id" />
<t
t-set="optional_product"
t-value="len(multi_gift_line.reward_product_ids) > 1"
/>
<t
t-set="option_checked"
t-value="product == multi_gift_line.reward_product_ids[:1]"
/>

Choose a reason for hiding this comment

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

Maybe in another module or in this one, but I think it could be useful to add the product name including attributes to differentiate between variants. What do you think?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

image

Comment on lines 12 to 15
for i in range(len(selected_product_ids)):
wizard_id.loyalty_gift_line_ids[i].selected_gift_id = int(
selected_product_ids[i]
)
Copy link
Member

Choose a reason for hiding this comment

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

This loop feels a little bit weird. Without knowing the precise internals I'd say you could better use zip for the task.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, thanks!

When setting up a multi-gift promotion or even in a simple promotion,
showing the product name without differentiating that it may be a variant,
the customer does not know exactly what gift they are receiving.
To avoid this confusion and also when choosing alternatives in a
multi-gift promotion, the full name is shown with its attributes but
omitting the reference.
@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-add-website_sale_loyalty_suggestion_wizard_multi_gift branch from cfc03e9 to 50b2686 Compare December 17, 2024 10:38
Copy link

@carlos-lopez-tecnativa carlos-lopez-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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants