Skip to content

Commit

Permalink
[REF] web_tour: prepare to change the default action in tour_compiler
Browse files Browse the repository at this point in the history
Currently, in tour_compiler, to check that an element is actually
present in the DOM in a step, there are 2 possibilities.
- `isCheck: true` => Checks that the element is in the DOM.
The latter can be disabled.
- `run() {}` => Checks that the element is in the DOM (because this
is what is done by default). However, if the element is disabled,
the step will be aborted.

In the codebase, we can see a lot of `run() {} //it's a check` and
`isCheck:true`. However, the behavior is not exactly the same.
What's more, there are 2 ways to do "the same thing".
In order to clarify the turns API, it was decided to no longer put an
action (step.run) by default. (Previously, the default action was
the "click" action).
From then on, it is no longer necessary to stipulate
`step.isCheck: true` nor `run() {}`.
The corollary is that now, you must explicitly write `run: click`, if
you want the click action to be triggered on the target.

This commit sets the stage for making this change.

closes odoo#167743

Related: odoo/design-themes#801
Related: odoo/enterprise#63711
Signed-off-by: Julien Mougenot (jum) <[email protected]>
  • Loading branch information
pipu-odoo committed Jun 5, 2024
1 parent 8e8e9d4 commit 4cc6ca1
Show file tree
Hide file tree
Showing 16 changed files with 58 additions and 22 deletions.
1 change: 1 addition & 0 deletions addons/account/static/src/js/tours/account.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ registry.category("web_tour.tours").add('account_tour', {
trigger: "button[name=document_layout_save]",
extra_trigger: "div.modal-dialog",
content: _t("Configure document layout."),
run: "click",
},
{
trigger: "div[name=partner_missing_email] a",
Expand Down
4 changes: 4 additions & 0 deletions addons/mail/static/tests/tours/discuss_configuration_tour.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,22 @@ registry.category("web_tour.tours").add("discuss_configuration_tour", {
{
trigger: "button:contains('All Messages')",
content: _t("Server notification settings"),
run: "click",
},
{
trigger: "button:contains('Mentions Only')",
content: _t("Server notification settings"),
run: "click",
},
{
trigger: "button:contains('Nothing')",
content: _t("Server notification settings"),
run: "click",
},
{
trigger: ".modal-header button[aria-label='Close']",
content: _t("Click to close"),
run: "click",
},
],
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ registry.category("web_tour.tours").add("point_of_sale_tour", {
width: 215,
position: "right",
edition: "community",
run: "click",
},
{
trigger: '.o_app[data-menu-xmlid="point_of_sale.menu_point_root"]',
content: markup(_t("Ready to launch your <b>point of sale</b>?")),
width: 215,
position: "bottom",
edition: "enterprise",
run: "click",
},
{
trigger: ".o_pos_kanban button.oe_kanban_action_button",
Expand All @@ -31,6 +33,7 @@ registry.category("web_tour.tours").add("point_of_sale_tour", {
)
),
position: "bottom",
run: "click",
},
],
});
2 changes: 2 additions & 0 deletions addons/pos_hr/static/tests/tours/utils/pos_hr_helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export function clickLoginButton() {
{
content: "click login button",
trigger: ".login-overlay .login-button.select-cashier",
run: "click",
},
];
}
Expand All @@ -15,6 +16,7 @@ export function clickCashierName() {
{
content: "click cashier name",
trigger: ".oe_status .cashier-name",
run: "click",
},
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,12 @@ registry.category("web_tour.tours").add("MergeTableTour", {
{
content: `select linked table`,
trigger: 'div.isLinked div.label:contains("4")',
run: "click",
},
{
content: `unlink in edit plan if unlink possible`,
trigger: '.edit-buttons button:contains("Unlink")',
run: "click",
},
Chrome.clickMenuOption("Edit Plan"),
...MergeTable.checkMergeTableIsCancelHelpers(),
Expand All @@ -308,10 +310,12 @@ registry.category("web_tour.tours").add("MergeTableTour", {
{
content: `select linked table`,
trigger: 'div.isLinked div.label:contains("4")',
run: "click",
},
{
content: `unlink in edit plan if unlink possible`,
trigger: '.edit-buttons button:contains("Unlink")',
run: "click",
},
Chrome.clickMenuOption("Edit Plan"),
...MergeTable.checkMergeTableIsCancelHelpers(),
Expand All @@ -321,6 +325,7 @@ registry.category("web_tour.tours").add("MergeTableTour", {
{
content: `link in edit plan if link possible`,
trigger: '.edit-buttons button:contains("Link")',
run: "click",
},
Chrome.clickMenuOption("Edit Plan"),
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export function clickFloor(name) {
{
content: `click '${name}' floor`,
trigger: `.floor-selector .button-floor:contains("${name}")`,
run: "click",
},
];
}
Expand All @@ -44,6 +45,7 @@ export function clickEditButton(button) {
{
content: "add table",
trigger: `.edit-buttons i[aria-label=${button}]`,
run: "click",
},
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export function tableNameShown(table_name) {
{
content: "Table name is shown",
trigger: `.table-name:contains(${table_name})`,
run: "click",
},
];
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export function clickPercentTip(percent) {
return [
{
trigger: `.tip-screen .percentage:contains("${percent}")`,
run: "click",
},
];
}
Expand All @@ -17,6 +18,7 @@ export function clickSettle() {
return [
{
trigger: `.button.highlight.next`,
run: "click",
},
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export function clickBack() {
return {
content: `Click on back button`,
trigger: `.btn.btn-back`,
run: "click",
};
}

Expand All @@ -15,6 +16,7 @@ export function selectTable(table) {
{
content: `Click on 'Confirm' button`,
trigger: `.self_order_popup_table .btn:contains('Confirm')`,
run: "click",
},
];
}
Expand All @@ -23,6 +25,7 @@ export function checkProduct(name, price, quantity) {
return {
content: `Check product card with ${name} and ${price}`,
trigger: `.product-card-item:has(strong:contains("${name}")):has(div:contains("${quantity}")):has(div .o-so-tabular-nums:contains("${price}"))`,
run: "click",
};
}

Expand All @@ -41,6 +44,7 @@ export function checkAttribute(productName, attributes) {
return {
content: `Check product card with ${productName} and ${attributeStringReadable}`,
trigger: `.product-card-item div:contains("${productName}") + div ${attributeString}`,
run: "click",
};
}

Expand All @@ -59,6 +63,7 @@ export function checkCombo(comboName, products) {
steps.push({
content: `Check combo ${comboName}`,
trigger: step,
run: "click",
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export function selectLocation(locationName) {
return {
content: `Click on location '${locationName}'`,
trigger: `.o_kiosk_eating_location_box h3:contains('${locationName}')`,
run: "click",
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,5 +162,6 @@ registry.category("web_tour.tours").add('project_update_tour', {
trigger: '.o_back_button',
content: 'Go back to the kanban view the project',
extra_trigger: '.o_list_view',
run: "click",
},
]});
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ registry.category("web_tour.tours").add('purchase_matrix_tour', {
auto: true,
run: "click",
}, {
trigger: "a:contains('Add a product')"
trigger: "a:contains('Add a product')",
run: "click",
}, {
trigger: 'div[name="product_template_id"] input',
run: "edit Matrix",
Expand Down
43 changes: 24 additions & 19 deletions addons/web_tour/static/src/tour_service/tour_compilers.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { browser } from "@web/core/browser/browser";
import { debounce } from "@web/core/utils/timing";
import { _legacyIsVisible, isVisible } from "@web/core/utils/ui";
import { omit, pick } from "@web/core/utils/objects";
import { omit } from "@web/core/utils/objects";
import { tourState } from "./tour_state";
import * as hoot from "@odoo/hoot-dom";
import { setupEventActions } from "@web/../lib/hoot-dom/helpers/events";
Expand Down Expand Up @@ -74,13 +74,17 @@ function tryFindTrigger(tour, step, elKey) {
function findStepTriggers(tour, step) {
const triggerEl = tryFindTrigger(tour, step, "trigger");
const altEl = tryFindTrigger(tour, step, "alt_trigger");
const extraEl = tryFindTrigger(tour, step, "extra_trigger");
const skipEl = tryFindTrigger(tour, step, "skip_trigger");
step.state = step.state || {};
step.state.triggerFound = !!triggerEl;
step.state.altTriggerFound = !!altEl;
step.state.extraTriggerFound = step.extra_trigger ? !!extraEl : true;
step.state.skipTriggerFound = !!skipEl;
// `extraTriggerOkay` should be true when `step.extra_trigger` is undefined.
// No need for it to be in the modal.
const extraTriggerOkay = step.extra_trigger
? tryFindTrigger(tour, step, "extra_trigger")
: true;
return { triggerEl, altEl, extraTriggerOkay, skipEl };
const extraTriggerOkay = step.state.extraTriggerFound;
return { triggerEl, altEl, extraEl, skipEl, extraTriggerOkay };
}

/**
Expand All @@ -99,18 +103,22 @@ function describeFailedStepSimple(tour, step) {

function describeWhyStepFailed(step) {
const stepState = step.state || {};
if (!stepState.stepElFound) {
return `The error appears to be that one or more elements in the following list cannot be found in DOM.\n ${JSON.stringify(
pick(step, "trigger", "extra_trigger", "alt_trigger", "skip_trigger")
)}`;
if (step.skip_trigger && !stepState.skipTriggerFound) {
return `The cause is that skip trigger (${step.skip_trigger}) element cannot be found in DOM.`;
} else if (step.extra_trigger && !stepState.extraTriggerFound) {
return `The cause is that extra trigger (${step.extra_trigger}) element cannot be found in DOM.`;
} else if (!stepState.triggerFound) {
return `The cause is that trigger (${step.trigger}) element cannot be found in DOM.`;
} else if (step.alt_trigger && !stepState.altTriggerFound) {
return `The cause is that alt(ernative) trigger (${step.alt_trigger}) element cannot be found in DOM.`;
} else if (!stepState.isVisible) {
return "Element has been found but isn't displayed. (Use 'step.allowInvisible: true,' if you want to skip this check)";
} else if (!stepState.isEnabled) {
return "Element has been found but is disabled. (If this step does not run action so that you only want to check that element is visible, you can use 'step.isCheck: true,')";
} else if (stepState.isBlocked) {
return "Element has been found but DOM is blocked by UI.";
} else if (!stepState.hasRun) {
return `Element has been found. The error seems to be in run()`;
return `Element has been found. The error seems to be with step.run`;
}
return "";
}
Expand Down Expand Up @@ -182,7 +190,6 @@ function getAnchorEl(el, consumeEvent) {
function canContinue(el, step) {
step.state = step.state || {};
const state = step.state;
state.stepElFound = true;
const rootNode = el.getRootNode();
state.isInDoc =
rootNode instanceof ShadowRoot
Expand Down Expand Up @@ -403,7 +410,8 @@ export function compileStepAuto(stepIndex, step, options) {

if (skipEl) {
stepEl = skipEl;
step.isCheck = true;
step.run = () => {};
step.allowDisabled = true;
}
// "isCheck: true" = no action to run and accept disabled elements.
if (step.isCheck) {
Expand All @@ -427,6 +435,10 @@ export function compileStepAuto(stepIndex, step, options) {
pointer.hide();
}

if (!("run" in step)) {
throwError(tour, step, ["The step must have a run key `run() {}`."]);
}

// TODO: Delegate the following routine to the `ACTION_HELPERS` in the macro module.
const actionHelper = new RunningTourActionHelper(stepEl);

Expand All @@ -448,13 +460,6 @@ export function compileStepAuto(stepIndex, step, options) {
actionHelper[m.groups?.action](m.groups?.arguments)
);
}
} else {
if (stepIndex === tour.steps.length - 1) {
console.warn("Tour %s: ignoring action (auto) of last step", tour.name);
step.state.hasRun = true;
} else {
await tryToDoAction(() => actionHelper.click());
}
}
return result;
},
Expand Down
4 changes: 2 additions & 2 deletions addons/web_tour/static/tests/tour_service_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ QUnit.module("Tour service", (hooks) => {
"run": "click"
},`;
const expectedError = [
`error: Tour tour1 failed at step content (trigger: .wrong_selector). The error appears to be that one or more elements in the following list cannot be found in DOM.\n {"trigger":".wrong_selector"}`,
`error: Tour tour1 failed at step content (trigger: .wrong_selector). The cause is that trigger (.wrong_selector) element cannot be found in DOM.`,
];
assert.verifySteps([expectedWarning, ...expectedError]);
});
Expand Down Expand Up @@ -826,7 +826,7 @@ QUnit.module("Tour service", (hooks) => {
await mock.advanceTime(750);

const expectedError = [
"error: Tour tour2 failed at step .button1. Element has been found. The error seems to be in run()",
"error: Tour tour2 failed at step .button1. Element has been found. The error seems to be with step.run",
"error: Cannot read properties of null (reading 'click')",
"error: tour not succeeded",
];
Expand Down
2 changes: 2 additions & 0 deletions addons/website_links/static/tests/tours/website_links.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ function fillSelect2(inputID, search) {
{
content: "Click select2 form item",
trigger: `.o_website_links_utm_forms div.select2-container#s2id_${inputID} > .select2-choice`,
run: "click",
},
{
content: "Enter select2 search query",
Expand All @@ -17,6 +18,7 @@ function fillSelect2(inputID, search) {
{
content: "Select found select2 item",
trigger: `.select2-drop li:only-child .select2-match:contains("/^${search}$/")`,
run: "click",
},
{
content: "Check that select2 is properly filled",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ stepUtils.autoExpandMoreButtons('.o_form_saved'),
trigger: '.o_field_widget[name="type"] input[data-value="service"]',
content: _t('Set to service'),
position: 'left',
run: "click",
}, {
mobile: false,
trigger: ".o_field_widget[name=taxes_id] input",
Expand Down

0 comments on commit 4cc6ca1

Please sign in to comment.