[16.0][ADD] pos_membership_delegated_partner#1374
Conversation
e4d0d51 to
1ebc0cc
Compare
1ebc0cc to
ef78fc9
Compare
huguesdk
left a comment
There was a problem hiding this comment.
some remarks, mostly cosmetic. otherwise, lgtm.
| "pos_membership_delegated_partner/static/src/css/*.css", | ||
| "pos_membership_delegated_partner/static/src/js/*.js", | ||
| "pos_membership_delegated_partner/static/src/xml/*.xml", |
There was a problem hiding this comment.
i think it’s best to have js and xml files in the corresponding directory of the files they extend. for example, ProductScreen.esm.js should go to static/src/js/Screen/ProductScreen/ProductScreen.esm.js and OrderlineDetails.xml should go to static/src/xml/Screens/TicketScreen/OrderlineDetails.xml.
| <t t-if="props.line.delegated_member"> | ||
| <i | ||
| class="fa fa-hand-o-right" | ||
| role="img" | ||
| aria-label="Member" | ||
| title="Member" | ||
| /> | ||
| <t t-out="props.line.delegated_member.name" /> | ||
| </t> | ||
| <t t-else=""> | ||
| <t t-if="props.line.order.get_partner()"> | ||
| <i | ||
| class="fa fa-hand-o-right" | ||
| role="img" | ||
| aria-label="Member" | ||
| title="Member" | ||
| /> | ||
| <t t-out="props.line.order.get_partner().name" /> | ||
| <span> (<i | ||
| class="fa fa-user" | ||
| role="img" | ||
| aria-label="Customer" | ||
| title="Customer" | ||
| />)</span> | ||
| </t> | ||
| </t> |
There was a problem hiding this comment.
instead of having a big t-if/t-else with almost the same thing in both parts, the orderline should have a method returning the membership partner (with the logic there). the display of the customer icon could be in an t-if="!props.line.delegated_member".
| <t t-if="props.line.delegated_member"> | ||
| <i | ||
| class="fa fa-hand-o-right" | ||
| role="img" | ||
| aria-label="Member" | ||
| title="Member" | ||
| /> | ||
| <t t-out="props.line.delegated_member.name" /> | ||
| </t> | ||
| <t t-else=""> | ||
| <t t-if="props.line.order.get_partner()"> | ||
| <i | ||
| class="fa fa-hand-o-right" | ||
| role="img" | ||
| aria-label="Member" | ||
| title="Member" | ||
| /> | ||
| <t t-out="props.line.order.get_partner().name" /> | ||
| <span> (<i | ||
| class="fa fa-user" | ||
| role="img" | ||
| aria-label="Customer" | ||
| title="Customer" | ||
| />)</span> | ||
| </t> |
There was a problem hiding this comment.
same remark as in the Orderline template. better: as these have exactly the same content, this should additionally be defined in a component reused in both places.
This module implements the features of the Membership Delegated Partner module in the point of sale UI. This allows a customer to purchase memberships for other partners.
ef78fc9 to
e1cb5ed
Compare
|
@huguesdk Thanks for the review! I've made the adjustments. |
huguesdk
left a comment
There was a problem hiding this comment.
thanks for the changes! lgtm! 🎉
|
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. |
WIP
This module implements the features of the Membership Delegated Partner module in the point of sale UI. This allows a customer to purchase memberships for other partners.
Dependencies
Usage
Once the order has been validated, the designated partner becomes a member.