Merge pull request 'fix(recipes): corrige la numérotation des étapes faussée par le popover de correction' (#31) from fix/step-counter-scoped-to-direct-children into main
This commit is contained in:
commit
1ec846ed3d
4 changed files with 165 additions and 20 deletions
|
|
@ -53,21 +53,33 @@ describe("Admin monitoring", () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
// The page route `/admin/monitoring` and the API path `GET /admin/monitoring`
|
// The page route `/admin/monitoring` and the API path `GET /admin/monitoring`
|
||||||
// are identical — a plain `**/admin/monitoring` glob would also match the
|
// are identical — a plain `**/admin/monitoring` glob also matches the
|
||||||
// `cy.visit()` document request (and whether that's same-origin or not
|
// `cy.visit()` document request (and whether that's same-origin or not
|
||||||
// depends on `VITE_API_URL`, which isn't set in CI). `resourceType: "fetch"`
|
// depends on `VITE_API_URL`, which isn't set in CI).
|
||||||
// pins the intercept to the `AdminApiClient` XHR only.
|
//
|
||||||
const monitoringApi = {
|
// This used to be disambiguated with a declarative `resourceType: "fetch"`
|
||||||
method: "GET",
|
// route matcher, but that stopped working (Cypress 13.17.0 on this
|
||||||
url: "**/admin/monitoring",
|
// machine's Chrome 152): the *declarative* `resourceType` matcher no
|
||||||
resourceType: "fetch",
|
// longer catches this request at all — confirmed by logging every request
|
||||||
} as const;
|
// Cypress hands to a `{resourceType: "fetch"}` intercept, which only ever
|
||||||
|
// saw `/admin/auth/me`, never `/admin/monitoring` — even though the very
|
||||||
|
// same request's own `req.resourceType`, read *inside* a handler matched
|
||||||
|
// by url/method alone, reliably reads back `"fetch"`. So the request *is*
|
||||||
|
// correctly typed; only the pre-filtering by that type is broken. Reading
|
||||||
|
// `req.resourceType` inside the handler and branching manually (as below)
|
||||||
|
// isn't affected by that bug and reproduced correctly in every check.
|
||||||
|
function interceptMonitoringApi(response: { statusCode: number; body: unknown }) {
|
||||||
|
return cy.intercept("GET", "**/admin/monitoring", (req) => {
|
||||||
|
if (req.resourceType === "document") {
|
||||||
|
req.continue();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
req.reply(response);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
it("renders one card per service with its status and details", () => {
|
it("renders one card per service with its status and details", () => {
|
||||||
cy.intercept(monitoringApi, {
|
interceptMonitoringApi({ statusCode: 200, body: monitoringFixture() }).as("getMonitoring");
|
||||||
statusCode: 200,
|
|
||||||
body: monitoringFixture(),
|
|
||||||
}).as("getMonitoring");
|
|
||||||
cy.visit("/admin/monitoring");
|
cy.visit("/admin/monitoring");
|
||||||
cy.wait("@getMonitoring");
|
cy.wait("@getMonitoring");
|
||||||
|
|
||||||
|
|
@ -85,11 +97,17 @@ describe("Admin monitoring", () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
it("shows an error state when the request fails", () => {
|
it("shows an error state when the request fails", () => {
|
||||||
cy.intercept(monitoringApi, {
|
// Aliased and waited on (unlike the other tests' plain intercepts) so
|
||||||
statusCode: 500,
|
// this genuinely proves the 500 came from the mock — without a live API
|
||||||
body: { code: 5000, message: "x" },
|
// server, an unmatched request fails at the network level too, which
|
||||||
});
|
// `MonitoringPage`'s `.catch()` can't tell apart from a real 500 (see
|
||||||
|
// the comment above `interceptMonitoringApi`, this is exactly the class
|
||||||
|
// of bug that let this test pass for the wrong reason before).
|
||||||
|
interceptMonitoringApi({ statusCode: 500, body: { code: 5000, message: "x" } }).as(
|
||||||
|
"getMonitoringError",
|
||||||
|
);
|
||||||
cy.visit("/admin/monitoring");
|
cy.visit("/admin/monitoring");
|
||||||
|
cy.wait("@getMonitoringError");
|
||||||
cy.contains("Impossible de charger").should("be.visible");
|
cy.contains("Impossible de charger").should("be.visible");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -69,6 +69,64 @@ const omeletteDetail = {
|
||||||
],
|
],
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A 3-step recipe whose middle step already has an existing technique match
|
||||||
|
* carrying attached ingredient/utensil metadata — for the step-numbering
|
||||||
|
* regression test below, which needs a correction popover with several
|
||||||
|
* `<li>` rows of its own open at once (see that test's own comment).
|
||||||
|
*/
|
||||||
|
const tarteDetail = {
|
||||||
|
id: 5,
|
||||||
|
name: "Tarte",
|
||||||
|
description: null,
|
||||||
|
picture: null,
|
||||||
|
portions: 4,
|
||||||
|
authorId: 1,
|
||||||
|
visibility: "PERSONAL",
|
||||||
|
allergens: [],
|
||||||
|
diets: [],
|
||||||
|
isFavorite: false,
|
||||||
|
ingredients: [],
|
||||||
|
steps: [
|
||||||
|
{ id: 1, description: "Préchauffer le four.", picture: null, order: 1, techSteps: [] },
|
||||||
|
{
|
||||||
|
id: 2,
|
||||||
|
description: "Faire revenir les oignons avec une poêle.",
|
||||||
|
picture: null,
|
||||||
|
order: 2,
|
||||||
|
techSteps: [
|
||||||
|
{
|
||||||
|
techStep: { id: 2, key: "panFry" },
|
||||||
|
start: 0,
|
||||||
|
end: 13,
|
||||||
|
source: "auto",
|
||||||
|
ingredients: [
|
||||||
|
{
|
||||||
|
ingredient: {
|
||||||
|
id: 10,
|
||||||
|
key: "onion",
|
||||||
|
icon: "VEGETABLE",
|
||||||
|
category: "freshProduce",
|
||||||
|
subcategory: "vegetables",
|
||||||
|
allergens: [],
|
||||||
|
diets: [],
|
||||||
|
},
|
||||||
|
quantity: 2,
|
||||||
|
unit: { id: 2, key: "piece", type: "COUNT", toBaseFactor: 1 },
|
||||||
|
start: 18,
|
||||||
|
end: 25,
|
||||||
|
source: "auto",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
utensils: [{ utensil: { id: 20, key: "pan" }, start: 31, end: 40, source: "auto" }],
|
||||||
|
temperatures: [],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
{ id: 3, description: "Servir chaud.", picture: null, order: 3, techSteps: [] },
|
||||||
|
],
|
||||||
|
};
|
||||||
|
|
||||||
function interceptAuth() {
|
function interceptAuth() {
|
||||||
cy.intercept("GET", "**/auth/me", { statusCode: 200, body: authenticatedProfile });
|
cy.intercept("GET", "**/auth/me", { statusCode: 200, body: authenticatedProfile });
|
||||||
cy.intercept("GET", "**/profile/disliked-ingredients", { statusCode: 200, body: [] });
|
cy.intercept("GET", "**/profile/disliked-ingredients", { statusCode: 200, body: [] });
|
||||||
|
|
@ -187,6 +245,54 @@ describe("Recipe catalog", () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Regression: `.recipe-detail-panel__steps`'s CSS step counter
|
||||||
|
// (recipes.scss) used a bare descendant `li` selector, matching *any*
|
||||||
|
// `<li>` nested inside a step — including ones several levels down
|
||||||
|
// inside `TechStepCorrectionPopover` (each attached ingredient/utensil/
|
||||||
|
// temperature row, and the technique picker's own `<li>` list). Opening
|
||||||
|
// a correction with attached metadata inflated the counter, throwing off
|
||||||
|
// every *later* step's own displayed number — exactly the "les étapes
|
||||||
|
// affichées sont numérotées n'importe comment" bug report this covers.
|
||||||
|
it("keeps every step's own number correct while a correction popover with attached metadata is open", () => {
|
||||||
|
cy.viewport(1600, 900);
|
||||||
|
cy.intercept("GET", /\/recipes\?/, { statusCode: 200, body: [] });
|
||||||
|
cy.intercept("GET", "**/recipes/5", { statusCode: 200, body: tarteDetail });
|
||||||
|
cy.intercept("GET", "**/reference/tech-steps", {
|
||||||
|
statusCode: 200,
|
||||||
|
body: [{ id: 2, key: "panFry" }],
|
||||||
|
});
|
||||||
|
|
||||||
|
cy.visit("/recettes/5");
|
||||||
|
cy.get(".recipe-detail-panel__steps > li").should("have.length", 3);
|
||||||
|
|
||||||
|
// Open the middle step's correction — pre-seeded with one ingredient
|
||||||
|
// and one utensil, so its popover renders two `.tech-step-correction-popover__row`
|
||||||
|
// `<li>`s of its own (plus the technique picker's own `<li>` list once
|
||||||
|
// "Changer" is clicked) sitting *inside* the second step's own `<li>`.
|
||||||
|
cy.contains(".step-tech-step", "Faire revenir").click();
|
||||||
|
cy.get(".tech-step-correction-popover__row").should("have.length", 2);
|
||||||
|
cy.contains(".tech-step-correction-popover__technique-change", "Changer").click();
|
||||||
|
cy.get(".catalog-search-picker__list li").should("have.length.greaterThan", 0);
|
||||||
|
|
||||||
|
// None of the popover's own nested `<li>`s join the step counter chain
|
||||||
|
// — `getComputedStyle(...).content` isn't reliable for reading a
|
||||||
|
// *resolved* counter() value across browsers (it can return the raw
|
||||||
|
// `counter(step) ". "` declaration instead), so this checks the actual
|
||||||
|
// `counter-increment` computed property instead: a real, directly
|
||||||
|
// testable signal for exactly what `> li` (recipes.scss) does and
|
||||||
|
// doesn't scope to.
|
||||||
|
cy.get(".tech-step-correction-popover__row").each(($row) => {
|
||||||
|
expect(window.getComputedStyle($row[0]).counterIncrement).to.equal("none");
|
||||||
|
});
|
||||||
|
cy.get(".catalog-search-picker__list li").each(($item) => {
|
||||||
|
expect(window.getComputedStyle($item[0]).counterIncrement).to.equal("none");
|
||||||
|
});
|
||||||
|
// The step `<li>`s themselves are still correctly in the chain.
|
||||||
|
cy.get(".recipe-detail-panel__steps > li").each(($step) => {
|
||||||
|
expect(window.getComputedStyle($step[0]).counterIncrement).to.equal("step 1");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it("shows a not-found message for a selected id the API rejects", () => {
|
it("shows a not-found message for a selected id the API rejects", () => {
|
||||||
cy.intercept("GET", /\/recipes\?/, { statusCode: 200, body: [] });
|
cy.intercept("GET", /\/recipes\?/, { statusCode: 200, body: [] });
|
||||||
cy.intercept("GET", "**/recipes/999", {
|
cy.intercept("GET", "**/recipes/999", {
|
||||||
|
|
|
||||||
|
|
@ -180,7 +180,12 @@ Then("the ingredient {string} should be visible in the picker", (name: string) =
|
||||||
});
|
});
|
||||||
|
|
||||||
Then("the recipe should include the ingredient {string}", (name: string) => {
|
Then("the recipe should include the ingredient {string}", (name: string) => {
|
||||||
cy.contains(".ingredient-row__name", name).should("be.visible");
|
// Same reasoning as "I should see the highlighted technique" above: on the
|
||||||
|
// recipe-edit form the ingredients list can sit below the fold of
|
||||||
|
// `.app-content`'s own scroll (Nom/Description/Photo/Portions/Visibilité/
|
||||||
|
// Régimes all come first), and a bare `.should("be.visible")` doesn't
|
||||||
|
// auto-scroll — only interaction commands like `.click()`/`.check()` do.
|
||||||
|
cy.contains(".ingredient-row__name", name).scrollIntoView().should("be.visible");
|
||||||
});
|
});
|
||||||
|
|
||||||
Then("there should be {int} ingredient rows", (count: number) => {
|
Then("there should be {int} ingredient rows", (count: number) => {
|
||||||
|
|
|
||||||
|
|
@ -630,7 +630,17 @@
|
||||||
gap: var(--space-md);
|
gap: var(--space-md);
|
||||||
counter-reset: step;
|
counter-reset: step;
|
||||||
|
|
||||||
li {
|
// `> li`, not a bare descendant `li` — this used to match *any* `<li>`
|
||||||
|
// nested anywhere inside a step, not just the step's own direct one.
|
||||||
|
// Harmless while `StepDescription`'s editable mode only ever added
|
||||||
|
// plain text/buttons, but `TechStepCorrectionPopover` renders its own
|
||||||
|
// `<li>`s a few levels down (each attached ingredient/utensil/
|
||||||
|
// temperature row, `.tech-step-correction-popover__row`; the technique
|
||||||
|
// picker's `.catalog-search-picker__list`, up to ~74 entries) —
|
||||||
|
// every one of those was incrementing this *same* counter, throwing
|
||||||
|
// every step's own number off as soon as any correction popover with
|
||||||
|
// attached metadata (or its technique picker) was open.
|
||||||
|
> li {
|
||||||
counter-increment: step;
|
counter-increment: step;
|
||||||
|
|
||||||
&::before {
|
&::before {
|
||||||
|
|
@ -639,14 +649,20 @@
|
||||||
color: var(--color-primary);
|
color: var(--color-primary);
|
||||||
}
|
}
|
||||||
|
|
||||||
img {
|
// Same descendant-selector fix as `> li` above, for the same reason:
|
||||||
|
// both the step's own picture and its `StepDescription`-rendered `<p>`
|
||||||
|
// are direct children of this `<li>` — `TechStepCorrectionPopover`'s
|
||||||
|
// own `<p>`s (the selection line, hints, `.field-error`) sit deeper
|
||||||
|
// inside it and must not inherit "inline, no margin" meant only for
|
||||||
|
// the step's own paragraph.
|
||||||
|
> img {
|
||||||
display: block;
|
display: block;
|
||||||
max-width: 100%;
|
max-width: 100%;
|
||||||
border-radius: var(--radius-base);
|
border-radius: var(--radius-base);
|
||||||
margin-bottom: var(--space-xs);
|
margin-bottom: var(--space-xs);
|
||||||
}
|
}
|
||||||
|
|
||||||
p {
|
> p {
|
||||||
display: inline;
|
display: inline;
|
||||||
margin: 0;
|
margin: 0;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue