[16.0][ADD] website_sale_product_compatibility and website_sale_product_contract_gift #351
Conversation
This module is a base module for adding compatibility check between product in a sale order for the e-commerce. This comes with the refactoring of the module website_sale_restrict_sepa_dd.
Add product contract that can be set as gift and therefore generate some specific compatibilities and flow.
remytms
force-pushed
the
16.0-add-website_sale_product_contract_gift
branch
from
647cecb to
96eeae1
Compare
remytms
force-pushed
the
16.0-add-website_sale_product_contract_gift
branch
from
b0ed365 to
187c521
Compare
remytms
force-pushed
the
16.0-add-website_sale_product_contract_gift
branch
from
187c521 to
a3d82cd
Compare
Create new partner or match existing partner when creating contract for gift. Create user when the invoice is generated at the gift date. Missing sending an email to the user for telling them they get a gift.
all() retrun true with an empty recordset. Which in this case is not correct.
website_sale_product_compatibility/i18n/website_sale_restrict_sepa_dd.pot
Outdated
Show resolved
Hide resolved
| :retrun: warning message to be shown on the web interface. | ||
| """ | ||
| self.ensure_one() | ||
| return "" |
There was a problem hiding this comment.
i think that returning None is more clean than an empty string.
There was a problem hiding this comment.
Feel strange to return None when you attend to receive a string in return. Or I should check type of returned object when using this method.
There was a problem hiding this comment.
the empty string in this case is actually used as a special value: if it is an empty string, it means that there is no problem: it is thus never used as a string.
replacing it with None will not break the existing code that is checking whether the return value is truthy. but it will allow to write it in a cleaner (more explicit) way:
if warning is None:there are only 2 cases to handle:
- the return value is
None, meaning that there is no warning. - there is a warning.
to me, None means “there is no warning”, while the current code feels more like “there is an empty warning” (which makes not much sense). with this change, it even allows to have a warning with an empty message (not very useful, but it illustrates the difference).
| <!-- Replace confirm button --> | ||
| <xpath expr="//a[@href='/shop/confirm_order']" position="attributes"> | ||
| <attribute name="style">display: none;</attribute> | ||
| </xpath> | ||
|
|
||
| <xpath expr="//a[@href='/shop/confirm_order']" position="after"> | ||
| <button id="gift_form_confirm_button" class="btn btn-primary mb32"> | ||
| Confirm | ||
| <i class="fa fa-chevron-right" /> | ||
| </button> | ||
| </xpath> |
There was a problem hiding this comment.
hiding a button with css and adding another one is kind of ugly. can’t the original button be used differently instead (by overriding some maybe)?
There was a problem hiding this comment.
I’ve not yet find a better method for this particular purpose.
There was a problem hiding this comment.
why not use position="replace" then?
Error when computing gift_date on an order without gift product.
remytms
force-pushed
the
16.0-add-website_sale_product_contract_gift
branch
from
c525269 to
b42a8c1
Compare
remytms
force-pushed
the
16.0-add-website_sale_product_contract_gift
branch
from
b42a8c1 to
73590a1
Compare
remytms
force-pushed
the
16.0-add-website_sale_product_contract_gift
branch
from
73590a1 to
209a068
Compare
Description
Move compatibility features in a new module.
Create gift module.
Odoo task (if applicable)
task
Checklist before approval