[14.0][IMP] pos_disable_pricelist_selection: selectable pricelists #1110
Conversation
ilyasProgrammer
changed the title
ilyasProgrammer
force-pushed
the
14.0-pos_disable_pricelist_selection-selectable-pricelist
branch
4 times, most recently
from
8da9e45 to
c534b16
Compare
| @@ -7,6 +7,7 @@ | |||
| "license": "LGPL-3", | |||
| "category": "Purchase", | |||
| "website": "https://github.com/OCA/pos", | |||
| "maintainers": ["ilyasprogrammer"], | |||
| "depends": ["point_of_sale"], | |||
There was a problem hiding this comment.
chore: missing dependency on web_domain_field
There was a problem hiding this comment.
web_domain_field is not required. Turns out compute domain thing works without web_domain_field. Just need to use Binary field.
| def write(self, vals): | ||
| if vals.get("available_pricelist_ids"): | ||
| if self and not vals.get("selectable_pricelist_ids"): | ||
| selectable = set(self.selectable_pricelist_ids.ids) |
There was a problem hiding this comment.
issue: self could be a recordset containing multiple records.
| else: | ||
| selectable = set(vals["selectable_pricelist_ids"][0][2]) | ||
| # leave only ids from available_pricelist_ids | ||
| diff = list( |
There was a problem hiding this comment.
todo: this variable name is IMO misleading, since it's the intersection, and not the difference.
| if rec.hide_pricelist_button: | ||
| rec.pricelist_id_domain = [("id", "in", rec.allowed_pricelist_ids.ids)] | ||
| else: | ||
| rec.pricelist_id_domain = [ | ||
| ("id", "in", rec.selectable_pricelist_ids.ids) | ||
| ] |
There was a problem hiding this comment.
chore: Documentation of the web_domain_field recommends using json.dumps to make sure any edge case is handled properly.
There was a problem hiding this comment.
Couldn't find a way to cause an error, so considering this non-blocking.
| async onClick() { | ||
| // Replaced to use selectable_pricelists instead of pricelists | ||
| const selectionList = this.env.pos.selectable_pricelists.map( | ||
| (pricelist) => ({ | ||
| id: pricelist.id, | ||
| label: pricelist.name, | ||
| isSelected: pricelist.id === this.currentOrder.pricelist.id, | ||
| item: pricelist, | ||
| }) | ||
| ); | ||
|
|
||
| const {confirmed, payload: selectedPricelist} = await this.showPopup( | ||
| "SelectionPopup", | ||
| { | ||
| title: this.env._t("Select the pricelist"), | ||
| list: selectionList, | ||
| } | ||
| ); | ||
|
|
||
| if (confirmed) { | ||
| this.currentOrder.set_pricelist(selectedPricelist); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
question (non-blocking): I don't know if this could be a better approach, but I wonder if, instead of overwriting the entire method, we could quietly replace this.env.pos.pricelists with this.env.pos.selectable_pricelists, then call super(), and finally put things back where they belong. My main concern is improving compatibility with other modules. What do you think?
ilyasProgrammer
force-pushed
the
14.0-pos_disable_pricelist_selection-selectable-pricelist
branch
from
ad792b3 to
1a7af56
Compare
|
@pedrobaeza can this be merged based on existing reviews? thanks! |
|
/ocabot merge major |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at c8271ee. Thanks a lot for contributing to OCA. ❤️ |
No description provided.