Skip to content

Commit

Permalink
[IMP] point_of_sale: lazy reactive getters
Browse files Browse the repository at this point in the history
This commit introduces an implementation of lazy reactive computed value that is
pull-based -- it only recomputes when it's needed. Check the following PR in
odoo/owl for its origin: odoo/owl#1499

It's basically an automatic caching mechanism that taps to the reactivity
primitive of odoo/owl. Because it depends on odoo/owl's reactivity, the logic
inside a getter is only recomputed when its dependencies changed.

Example:

Given the following base values `A`, `B`, `C` and `D`, and dependent values
`AB`, `BC` and `X`.

```mermaid
graph TD
    A --> AB
    B --> AB
    B --> BC
    C --> BC
    AB --> X
    D --> X
```

- Changing `A` should recompute `AB` and `X`.
- Changing `B` should recompute `AB`, `BC` and `X`.
- Changing `C` should recompute `BC`.
- Changing `D` should recompute `X`.

It's also pull-based (thus called _lazy_) which means that recomputations are
only called when necessary. Imagine a view (component) is only displaying `AB`
and `BC` (check above diagram). Changing `D` should invalidate value of `X`, but
since the view isn't dependent on `X`, there won't be a rerendering and the
logic of `X` won't be called.

An obvious benefit of this change is performance but we can consider it as
minor. A case where performance change is noticeable is when pos loaded 20k
products. In that situation, before this commit, there is a noticeable delay
when clicking a product. With this commit, the app feels as if only few products
are loaded.

Aside from performance, developers also benefits on debugging getters because
the logic is only called once per rendering. DX will also be good because we
just write basic getters when we want them to be lazy and reactive. Should we
want a getter to be called everytime, we define a normal method.

IMPORTANT NOTE: Refrain from making mutations inside a getter. It will likely
result to infinite loop if the object being mutated is reactive. But one can
still write imperative algorithms on locally defined objects.

For initial utilization of this feature, we converted few methods into lazy
getters.

1. `get productsToDisplay` - This was originally in the `ProductScreen`
   component. We moved it to `PosStore`. As a result, `productsToDisplay` is now
   only recomputed when necessary (such as changing the category or changing the
   search word). Logic inside the getter is not called excessively when
   `ProductScreen` rerenders, e.g. when adding a new orderline.
2. `getProductsByCategory(category)` is now `get associatedProducts` in the
   `PosCategory` model.
3. `getUnitDisplayPrice` is now `get unitDisplayPrice` in `PosOrderLine`.
4. `getAllPrices()` and `getAllPrices(1)` calls are now hidden behind the
   `get allPrices` and `get allUnitPrices` getters, respectively.

TASK-ID: 4361605
  • Loading branch information
caburj committed Nov 28, 2024
1 parent 114ad34 commit 5078dd1
Show file tree
Hide file tree
Showing 15 changed files with 597 additions and 141 deletions.
2 changes: 2 additions & 0 deletions addons/point_of_sale/__manifest__.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@
'point_of_sale/static/src/app/models/utils/indexed_db.js',
'point_of_sale/static/src/app/models/data_service_options.js',
'point_of_sale/static/src/utils.js',
'point_of_sale/static/src/proxy_trap.js',
'point_of_sale/static/src/lazy_getter.js',
'point_of_sale/static/src/app/services/data_service.js',
'point_of_sale/static/tests/unit/**/*',
],
Expand Down
8 changes: 8 additions & 0 deletions addons/point_of_sale/static/src/app/models/pos_category.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ export class PosCategory extends Base {

return parents.reverse();
}
get associatedProducts() {
const allCategoryIds = this.getAllChildren().map((cat) => cat.id);
const products = allCategoryIds.flatMap(
(catId) => this.models["product.template"].getBy("pos_categ_ids", catId) || []
);
// Remove duplicates since owl doesn't like them.
return Array.from(new Set(products));
}
}

registry.category("pos_available_models").add(PosCategory.pythonModel, PosCategory);
2 changes: 1 addition & 1 deletion addons/point_of_sale/static/src/app/models/pos_order.js
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ export class PosOrder extends Base {
getTaxDetails() {
const taxDetails = {};
for (const line of this.lines) {
for (const taxData of line.getAllPrices().taxesData) {
for (const taxData of line.allPrices.taxesData) {
const taxId = taxData.id;
if (!taxDetails[taxId]) {
taxDetails[taxId] = Object.assign({}, taxData, {
Expand Down
34 changes: 21 additions & 13 deletions addons/point_of_sale/static/src/app/models/pos_order_line.js
Original file line number Diff line number Diff line change
Expand Up @@ -361,19 +361,19 @@ export class PosOrderline extends Base {
return window.parseFloat(roundDecimals(this.price_unit || 0, digits).toFixed(digits));
}

getUnitDisplayPrice() {
get unitDisplayPrice() {
if (this.config.iface_tax_included === "total") {
return this.getAllPrices(1).priceWithTax;
return this.allUnitPrices.priceWithTax;
} else {
return this.getAllPrices(1).priceWithoutTax;
return this.allUnitPrices.priceWithoutTax;
}
}

getUnitDisplayPriceBeforeDiscount() {
if (this.config.iface_tax_included === "total") {
return this.getAllPrices(1).priceWithTaxBeforeDiscount;
return this.allUnitPrices.priceWithTaxBeforeDiscount;
} else {
return this.getAllPrices(1).priceWithoutTaxBeforeDiscount;
return this.allUnitPrices.priceWithoutTaxBeforeDiscount;
}
}
getBasePrice() {
Expand Down Expand Up @@ -422,19 +422,19 @@ export class PosOrderline extends Base {
}

getPriceWithoutTax() {
return this.getAllPrices().priceWithoutTax;
return this.allPrices.priceWithoutTax;
}

getPriceWithTax() {
return this.getAllPrices().priceWithTax;
return this.allPrices.priceWithTax;
}

getTax() {
return this.getAllPrices().tax;
return this.allPrices.tax;
}

getTaxDetails() {
return this.getAllPrices().taxDetails;
return this.allPrices.taxDetails;
}

getTotalTaxesIncludedInPrice() {
Expand Down Expand Up @@ -516,6 +516,14 @@ export class PosOrderline extends Base {
};
}

get allPrices() {
return this.getAllPrices();
}

get allUnitPrices() {
return this.getAllPrices(1);
}

displayDiscountPolicy() {
// Sales dropped `discount_policy`, and we only show discount if applied pricelist rule
// is a percentage discount. However we don't have that information in pos
Expand Down Expand Up @@ -584,17 +592,17 @@ export class PosOrderline extends Base {

getComboTotalPrice() {
const allLines = this.getAllLinesInCombo();
return allLines.reduce((total, line) => total + line.getAllPrices(1).priceWithTax, 0);
return allLines.reduce((total, line) => total + line.allUnitPrices.priceWithTax, 0);
}
getComboTotalPriceWithoutTax() {
const allLines = this.getAllLinesInCombo();
return allLines.reduce((total, line) => total + line.getAllPrices(1).priceWithoutTax, 0);
return allLines.reduce((total, line) => total + line.allUnitPrices.priceWithoutTax, 0);
}

getOldUnitDisplayPrice() {
return (
this.displayDiscountPolicy() === "without_discount" &&
roundCurrency(this.getUnitDisplayPrice(), this.currency) <
roundCurrency(this.unitDisplayPrice, this.currency) <
roundCurrency(this.getTaxedlstUnitPrice(), this.currency) &&
this.getTaxedlstUnitPrice()
);
Expand All @@ -616,7 +624,7 @@ export class PosOrderline extends Base {
price: this.getPriceString(),
qty: this.getQuantityStr(),
unit: this.product_id.uom_id ? this.product_id.uom_id.name : "",
unitPrice: formatCurrency(this.getUnitDisplayPrice(), this.currency),
unitPrice: formatCurrency(this.unitDisplayPrice, this.currency),
oldUnitPrice: this.getOldUnitDisplayPrice()
? formatCurrency(this.getOldUnitDisplayPrice(), this.currency)
: "",
Expand Down
76 changes: 36 additions & 40 deletions addons/point_of_sale/static/src/app/models/related_models.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { reactive, toRaw } from "@odoo/owl";
import { uuidv4 } from "@point_of_sale/utils";
import { TrapDisabler } from "@point_of_sale/proxy_trap";
import { WithLazyGetterTrap } from "@point_of_sale/lazy_getter";

const ID_CONTAINER = {};

Expand Down Expand Up @@ -139,12 +141,12 @@ function processModelDefs(modelDefs) {
return [inverseMap, modelDefs];
}

export class Base {
constructor({ models, records, model, proxyTrap }) {
export class Base extends WithLazyGetterTrap {
constructor({ models, records, model, traps }) {
super({ traps });
this.models = models;
this.records = records;
this.model = model;
return new Proxy(this, proxyTrap);
}
/**
* Called during instantiation when the instance is fully-populated with field values.
Expand Down Expand Up @@ -433,19 +435,6 @@ export function createRelatedModels(modelDefs, modelClasses = {}, opts = {}) {
return records[model].has(id);
}

// If value is more than 0, then the proxy trap is disabled.
let proxyTrapDisabled = 0;
function withoutProxyTrap(fn) {
return function (...args) {
try {
proxyTrapDisabled += 1;
return fn(...args);
} finally {
proxyTrapDisabled -= 1;
}
};
}

/**
* This check assumes that if the first element is a command, then the rest are commands.
*/
Expand All @@ -457,30 +446,39 @@ export function createRelatedModels(modelDefs, modelClasses = {}, opts = {}) {
);
}

const proxyTraps = {};
function getProxyTrap(model) {
if (model in proxyTraps) {
return proxyTraps[model];
}
const disabler = new TrapDisabler();
function withoutProxyTrap(fn) {
return (...args) => disabler.call(fn, ...args);
}

const setTrapsCache = {};
function instantiateModel(model, { models, records }) {
const fields = getFields(model);
const proxyTrap = {
set(target, prop, value) {
if (proxyTrapDisabled || !(prop in fields)) {
return Reflect.set(target, prop, value);
const Model = modelClasses[model] || Base;
if (!(model in setTrapsCache)) {
setTrapsCache[model] = function setTrap(target, prop, value, receiver) {
if (disabler.isDisabled() || !(prop in fields)) {
return Reflect.set(target, prop, value, receiver);
}
const field = fields[prop];
if (field && X2MANY_TYPES.has(field.type)) {
if (!isX2ManyCommands(value)) {
value = [["clear"], ["link", ...value]];
return disabler.call(() => {
const field = fields[prop];
if (field && X2MANY_TYPES.has(field.type)) {
if (!isX2ManyCommands(value)) {
value = [["clear"], ["link", ...value]];
}
}
}
target.update({ [prop]: value });
target.model.triggerEvents("update", { field: prop, value, id: target.id });
return true;
},
};
proxyTraps[model] = proxyTrap;
return proxyTrap;
receiver.update({ [prop]: value });
target.model.triggerEvents("update", { field: prop, value, id: target.id });
return true;
});
};
}
return new Model({
models,
records,
model: models[model],
traps: { set: setTrapsCache[model] },
});
}

const create = withoutProxyTrap(_create);
Expand All @@ -495,9 +493,7 @@ export function createRelatedModels(modelDefs, modelClasses = {}, opts = {}) {
vals["id"] = uuid(model);
}

const Model = modelClasses[model] || Base;
const proxyTrap = getProxyTrap(model);
let record = new Model({ models, records, model: models[model], proxyTrap });
let record = instantiateModel(model, { models, records });

const id = vals["id"];
record.id = id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@ import { Orderline } from "@point_of_sale/app/components/orderline/orderline";
import { OrderWidget } from "@point_of_sale/app/components/order_widget/order_widget";
import { OrderSummary } from "@point_of_sale/app/screens/product_screen/order_summary/order_summary";
import { ProductInfoPopup } from "@point_of_sale/app/components/popups/product_info_popup/product_info_popup";
import { fuzzyLookup } from "@web/core/utils/search";
import { ProductCard } from "@point_of_sale/app/components/product_card/product_card";
import {
ControlButtons,
ControlButtonsPopup,
} from "@point_of_sale/app/screens/product_screen/control_buttons/control_buttons";
import { unaccent } from "@web/core/utils/strings";
import { CameraBarcodeScanner } from "@point_of_sale/app/screens/product_screen/camera_barcode_scanner";

export class ProductScreen extends Component {
Expand Down Expand Up @@ -287,75 +285,6 @@ export class ProductScreen extends Component {
return this.pos.searchProductWord.trim();
}

get products() {
return this.pos.models["product.template"].getAll();
}

get productsToDisplay() {
let list = [];

if (this.searchWord !== "") {
list = this.addMainProductsToDisplay(this.getProductsBySearchWord(this.searchWord));
} else if (this.pos.selectedCategory?.id) {
list = this.getProductsByCategory(this.pos.selectedCategory);
} else {
list = this.products;
}

if (!list || list.length === 0) {
return [];
}

const excludedProductIds = [
this.pos.config.tip_product_id?.id,
...this.pos.hiddenProductIds,
...this.pos.session._pos_special_products_ids,
];

list = list
.filter(
(product) => !excludedProductIds.includes(product.id) && product.available_in_pos
)
.slice(0, 100);

return this.searchWord !== ""
? list.sort((a, b) => b.is_favorite - a.is_favorite)
: list.sort((a, b) => {
if (b.is_favorite !== a.is_favorite) {
return b.is_favorite - a.is_favorite;
}
return a.display_name.localeCompare(b.display_name);
});
}

getProductsBySearchWord(searchWord) {
return fuzzyLookup(unaccent(searchWord, false), this.products, (product) =>
unaccent(product.searchString, false)
);
}

addMainProductsToDisplay(products) {
const uniqueProductsMap = new Map();
for (const product of products) {
if (product.id in this.pos.mainProductVariant) {
const mainProduct = this.pos.mainProductVariant[product.id];
uniqueProductsMap.set(mainProduct.id, mainProduct);
} else {
uniqueProductsMap.set(product.id, product);
}
}
return Array.from(uniqueProductsMap.values());
}

getProductsByCategory(category) {
const allCategoryIds = category.getAllChildren().map((cat) => cat.id);
const products = allCategoryIds.flatMap(
(catId) => this.pos.models["product.template"].getBy("pos_categ_ids", catId) || []
);
// Remove duplicates since owl doesn't like it.
return Array.from(new Set(products));
}

async onPressEnterKey() {
const { searchProductWord } = this.pos;
if (!searchProductWord) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
<div class="position-relative d-flex flex-column flex-grow-1 overflow-hidden">
<CategorySelector t-if="!ui.isSmall || !pos.scanning" />
<CameraBarcodeScanner t-if="pos.scanning"/>
<div t-elif="productsToDisplay.length != 0 and pos.session._has_available_products" t-attf-class="product-list {{this.pos.productListViewMode}} overflow-y-auto px-2 pt-0 pb-2">
<div t-elif="pos.productsToDisplay.length != 0 and pos.session._has_available_products" t-attf-class="product-list {{this.pos.productListViewMode}} overflow-y-auto px-2 pt-0 pb-2">
<ProductCard
t-foreach="productsToDisplay" t-as="product" t-key="product.id"
t-foreach="pos.productsToDisplay" t-as="product" t-key="product.id"
productId="product.id"
product="product"
class="pos.productViewMode"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class ReceiptScreen extends Component {
const tipLine = order
.getOrderlines()
.find((line) => tip_product_id && line.product_id.id === tip_product_id);
const tipAmount = tipLine ? tipLine.getAllPrices().priceWithTax : 0;
const tipAmount = tipLine ? tipLine.allPrices.priceWithTax : 0;
const orderAmountStr = this.env.utils.formatCurrency(orderTotalAmount - tipAmount);
if (!tipAmount) {
return orderAmountStr;
Expand Down
Loading

0 comments on commit 5078dd1

Please sign in to comment.