[FIX] website_sale: retain rental period on attribute

[rupa-odoo]

version - 16.0

Steps:
-Install rental app.
-Activate the date picker from the web-editor.
-Add the start date and end date from that.
-Select some attribute of the product.

Issue:
-When any user adds the start date and end date from the date picker and
after that, the user adds the other attribute(filter) of the product, that s>

Cause:
-The selected rental period (start and end dates) is not persisted when the
page refreshes or updates due to attribute selection.

Fix:
Get the value of 'start_date' and 'end_date'.
Update the hidden input fields for 'start_date' and 'end_date' with the
values before applying the attribute changes.

opw-3774060

[robodoo]

.

[Feyensv]

@rupa-odoo please ping the right team, we have dedicated github teams for sale, loyalty, delivery, ecommerce, sales connector & payment.

[chevalierv]

Hello @rupa-odoo

I'll wait for @kcv-odoo's review before taking a look at this :)

Have a nice day!

[kcv-odoo]

The issue is related to rental and your fix is in website_sale. Is your issue really related to eCom not rental?

About commit message:
First verb of title should be in first form not second you are proposing a fix as an PR so
your title should be like this This commit will {commit_title}
Also cause and fix part should not like this:
Cause: Traceback or issue occur Fix: Fix the trace back or issue
Cause should explain cause of issue what is root of that issue why it occur and may be add cause related commit or PR and Fix should explain your choices why your fix is best fix and why you did what you did this will save time of reviewer of PR also if someone look at your fix after some time they can easily understand it by reading commit message 🙂

Hope this helps 🙏

[rupa-odoo]

Note:
Hello @kcv-odoo 👋
I had moved my changes to the enterprise in website_sale_renting module
and here is this pr of that : https://github.com/odoo/enterprise/pull/64962

[rupa-odoo]

Note :
This fix will only work for the stable version.
If we want to fix for the master we should
-> We need to make a change in '_onChangeAttribute'.
-> Update the hidden input fields for 'start_date' and 'end_date' with the values before applying the attribute changes.

[kcv-odoo]

LGTM for stable

[chevalierv]

Hello @rupa-odoo

Thanks for your work on this fix!

Those changes should be done in website_sale_renting, as it only concerns renting period in a filter only available when website_sale_renting is installed. Could you adapt your changes?

Have a nice day!

[chevalierv]

This won't solve the issue for existing clients as templates are only updated at upgrade. It will work for new customers though

[rupa-odoo]

Hello @rupa-odoo

Thanks for your work on this fix!

Those changes should be done in website_sale_renting, as it only concerns renting period in a filter only available when website_sale_renting is installed. Could you adapt your changes?

Have a nice day!

Yes, Done the changes 👍
Thankyou

[kcv-odoo]

Hello @chevalierv 👋

I thought doing solution here in website_sale would be better because if we want to do it in website_sale_renting then we need upgrade exception because we need to add new template overriding this one.
I told @rupa-odoo to keep your solution only for stable and Improve it in forward port of master this way we don't need an upgrade exception 🤔

Or is it okay to ask for upgrade exception? 🤔

[chevalierv]

Hello @kcv-odoo

Why do you think this would need an upgrade exception? IMO, we shouldn't do a bad fix to avoid exceptions, a good reason would be that the impact on the client is different (like with this ugly fix, clients don't need to upgrade the module to apply the fix)

[chevalierv]

@robodoo r+

[robodoo]

@rupa-odoo @chevalierv 'ci/template' failed on this reviewed PR.