From 15bc39d3effe06e7dafd474a3f764c29ce6c8a6f Mon Sep 17 00:00:00 2001 From: Samuel Degueldre Date: Wed, 1 Mar 2023 09:42:05 +0000 Subject: [PATCH] [FIX] pos_restaurant, *: allow selecting a table while offline *: point_of sale, web Previously, the error handling code was refactored and centralized, in doing so, we broke the ability to select a table while offline: when selecting a table we ask the server about the orders for the table. When offline, this creates an error that was previously caught, and showed an offline error popup, but continued the rest of the flow normally. The refactoring centralized the display of offline errors to an error handler, but removed the catching, causing the error to interrupt the flow. This commit fixes that by catching the error again, and when it's a ConnectionLostError, it dispatched the error in a separate async call stack (using Promise.reject) so that the display of offline errors is still centralized, the the flow can continue normally. closes odoo/odoo#114077 X-original-commit: f45f5e644b4b219b968fa19737d75cc710c823f3 Signed-off-by: Trinh Jacky (trj) Signed-off-by: Samuel Degueldre --- .../src/app/error_handlers/error_handlers.js | 22 ++++++++++++++----- .../static/src/app/error_handlers.js | 14 ++++++++++++ .../src/app/floor_screen/floor_screen.js | 11 +++++++++- .../web/static/src/core/network/download.js | 11 ++++++++-- .../static/src/core/network/rpc_service.js | 13 +++++++---- .../tests/core/errors/error_service_tests.js | 2 +- 6 files changed, 59 insertions(+), 14 deletions(-) create mode 100644 addons/pos_restaurant/static/src/app/error_handlers.js diff --git a/addons/point_of_sale/static/src/app/error_handlers/error_handlers.js b/addons/point_of_sale/static/src/app/error_handlers/error_handlers.js index 89401f1a208..8f7f6d06f34 100644 --- a/addons/point_of_sale/static/src/app/error_handlers/error_handlers.js +++ b/addons/point_of_sale/static/src/app/error_handlers/error_handlers.js @@ -6,6 +6,7 @@ import { ConnectionLostError, RPCError } from "@web/core/network/rpc_service"; import { ErrorPopup } from "@point_of_sale/js/Popups/ErrorPopup"; import { ErrorTracebackPopup } from "@point_of_sale/js/Popups/ErrorTracebackPopup"; import { OfflineErrorPopup } from "@point_of_sale/js/Popups/OfflineErrorPopup"; +import { _t, _lt } from "@web/core/l10n/translation"; export function identifyError(error) { return error && error.legacy ? error.message : error; @@ -29,14 +30,23 @@ function rpcErrorHandler(env, error, originalError) { } registry.category("error_handlers").add("rpcErrorHandler", rpcErrorHandler); +// TODO: consider only showing a notification instead of an error popup in flows that can work offline +export const urlToMessage = { + "/web/dataset/call_kw/pos.order/create_from_ui": _lt( + "The order couldn't be sent to the server because you are offline" + ), +}; function offlineErrorHandler(env, error, originalError) { error = identifyError(originalError); if (error instanceof ConnectionLostError) { - env.services.popup.add(OfflineErrorPopup, { - title: env._t("Couldn't connect to the server"), - body: env._t( + const body = + urlToMessage[error.url] || + _t( "The operation couldn't be completed because you are offline. Check your internet connection and try again." - ), + ); + env.services.popup.add(OfflineErrorPopup, { + title: _t("Couldn't connect to the server"), + body, }); return true; } @@ -52,8 +62,8 @@ function defaultErrorHandler(env, error, originalError) { }); } else { env.services.popup.add(ErrorPopup, { - title: env._t("Unknown Error"), - body: env._t("Unable to show information about this error."), + title: _t("Unknown Error"), + body: _t("Unable to show information about this error."), }); } return true; diff --git a/addons/pos_restaurant/static/src/app/error_handlers.js b/addons/pos_restaurant/static/src/app/error_handlers.js new file mode 100644 index 00000000000..720300f3630 --- /dev/null +++ b/addons/pos_restaurant/static/src/app/error_handlers.js @@ -0,0 +1,14 @@ +/** @odoo-module */ + +import { patch } from "@web/core/utils/patch"; +import { urlToMessage } from "@point_of_sale/app/error_handlers/error_handlers"; +import { _lt } from "@web/core/l10n/translation"; + +patch(urlToMessage, "pos_restaurant.urlToMessage", { + "/web/dataset/call_kw/pos.order/get_table_draft_orders": _lt( + "The orders for the table could not be loaded because you are offline" + ), + "/web/dataset/call_kw/pos.config/get_tables_order_count": _lt( + "Couldn't synchronize the orders for the tables because you are offline" + ), +}); diff --git a/addons/pos_restaurant/static/src/app/floor_screen/floor_screen.js b/addons/pos_restaurant/static/src/app/floor_screen/floor_screen.js index 2effa6ba97d..e0b655976c6 100644 --- a/addons/pos_restaurant/static/src/app/floor_screen/floor_screen.js +++ b/addons/pos_restaurant/static/src/app/floor_screen/floor_screen.js @@ -1,5 +1,6 @@ /** @odoo-module */ +import { ConnectionLostError } from "@web/core/network/rpc_service"; import { debounce } from "@web/core/utils/timing"; import { registry } from "@web/core/registry"; @@ -198,7 +199,15 @@ export class FloorScreen extends Component { if (this.env.pos.orderToTransfer) { await this.env.pos.transferTable(table); } else { - await this.env.pos.setTable(table); + try { + await this.env.pos.setTable(table); + } catch (e) { + if (!(e.message instanceof ConnectionLostError)) { + throw e; + } + // Reject error in a separate stack to display the offline popup, but continue the flow + Promise.reject(e); + } } const order = this.env.pos.get_order(); this.pos.showScreen(order.get_screen_data().name); diff --git a/addons/web/static/src/core/network/download.js b/addons/web/static/src/core/network/download.js index 007d4045f12..3a8c5d5dcaf 100644 --- a/addons/web/static/src/core/network/download.js +++ b/addons/web/static/src/core/network/download.js @@ -4,6 +4,12 @@ import { _lt } from "../l10n/translation"; import { makeErrorFromResponse, ConnectionLostError } from "@web/core/network/rpc_service"; import { browser } from "@web/core/browser/browser"; +/* eslint-disable */ +/** + * The following sections are from libraries, they have been slightly modified + * to allow patching them during tests, but should not be linted, so that we can + * keep a minimal diff that is easy to reapply when upgrading + */ // ----------------------------------------------------------------------------- // Content Disposition Library // ----------------------------------------------------------------------------- @@ -437,6 +443,7 @@ function _download(data, filename, mimetype) { } return true; } +/* eslint-enable */ // ----------------------------------------------------------------------------- // Exported download function @@ -491,7 +498,7 @@ download._download = (options) => { return resolve(filename); } else if (xhr.status === 502) { // If Odoo is behind another server (nginx) - reject(new ConnectionLostError()); + reject(new ConnectionLostError(options.url)); } else { const decoder = new FileReader(); decoder.onload = () => { @@ -524,7 +531,7 @@ download._download = (options) => { } }; xhr.onerror = () => { - reject(new ConnectionLostError()); + reject(new ConnectionLostError(options.url)); }; xhr.send(data); }); diff --git a/addons/web/static/src/core/network/rpc_service.js b/addons/web/static/src/core/network/rpc_service.js index 52d0bffca7f..16781e6c73f 100644 --- a/addons/web/static/src/core/network/rpc_service.js +++ b/addons/web/static/src/core/network/rpc_service.js @@ -18,7 +18,12 @@ export class RPCError extends Error { } } -export class ConnectionLostError extends Error {} +export class ConnectionLostError extends Error { + constructor(url, ...args) { + super(`Connection to "${url}" couldn't be established or was interrupted`, ...args); + this.url = url; + } +} export class ConnectionAbortedError extends Error {} @@ -61,7 +66,7 @@ export function jsonrpc(env, rpcId, url, params, settings = {}) { if (!settings.silent) { bus.trigger("RPC:RESPONSE", data.id); } - reject(new ConnectionLostError()); + reject(new ConnectionLostError(url)); return; } let params; @@ -73,7 +78,7 @@ export function jsonrpc(env, rpcId, url, params, settings = {}) { if (!settings.silent) { bus.trigger("RPC:RESPONSE", data.id); } - return reject(new ConnectionLostError()); + return reject(new ConnectionLostError(url)); } const { error: responseError, result: responseResult } = params; if (!settings.silent) { @@ -90,7 +95,7 @@ export function jsonrpc(env, rpcId, url, params, settings = {}) { if (!settings.silent) { bus.trigger("RPC:RESPONSE", data.id); } - reject(new ConnectionLostError()); + reject(new ConnectionLostError(url)); }); // configure and send request request.open("POST", url); diff --git a/addons/web/static/tests/core/errors/error_service_tests.js b/addons/web/static/tests/core/errors/error_service_tests.js index ac62cee5278..74e0e121182 100644 --- a/addons/web/static/tests/core/errors/error_service_tests.js +++ b/addons/web/static/tests/core/errors/error_service_tests.js @@ -225,7 +225,7 @@ QUnit.test("handle CONNECTION_LOST_ERROR", async (assert) => { } }; await makeTestEnv({ mockRPC }); - const error = new ConnectionLostError(); + const error = new ConnectionLostError("/fake_url"); const errorEvent = new PromiseRejectionEvent("error", { reason: error, promise: null,