feat: e2e playwright validation#1450
Open
tankerkiller125 wants to merge 15 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRemoves WebKit body text-selection/touch-callout restrictions, adds an Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Security Recommendations
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
⬆️ enhancement
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/pages/collection/index/entity-types.vue (1)
65-65:⚠️ Potential issue | 🔴 Critical🛡️ Fix the source of the 500 error —
defaultTemplateId: ""crashes the backend.Lines 65 and 115 fallback to an empty string:
defaultTemplateId: createTemplate.value?.id || "",The E2E test suite (lines 120-122 of
collection-settings-entity-types.spec.ts) documents this bug: the backend's*uuid.UUIDJSON decoder rejects empty strings and returns a 500 error. The test patches outgoing requests to rewrite""→null, but real users hit this directly whenever they create/update an entity type without selecting a default template.Fix it at the source:
Proposed fix
- defaultTemplateId: createTemplate.value?.id || "", + defaultTemplateId: createTemplate.value?.id || null,- defaultTemplateId: updateTemplate.value?.id || "", + defaultTemplateId: updateTemplate.value?.id || null,Update the type contract in
frontend/lib/api/types/data-contracts.tsfromdefaultTemplateId: stringtodefaultTemplateId: string | nullfor bothEntityTypeCreateandEntityTypeUpdateto match the backend's nullable*uuid.UUIDdefinition.Also applies to: 115-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/collection/index/entity-types.vue` at line 65, The backend rejects empty-string UUIDs causing 500s when frontend sends defaultTemplateId: "" (seen in entity-types.vue where defaultTemplateId: createTemplate.value?.id || ""); update the data contract and calls so the field is nullable: change defaultTemplateId: string to defaultTemplateId: string | null in EntityTypeCreate and EntityTypeUpdate in frontend/lib/api/types/data-contracts.ts, and update usage sites (e.g., entity-types.vue's defaultTemplateId assignment and any form submit that currently falls back to "") to pass null instead of an empty string when no template is selected.frontend/pages/item/[id]/index/edit.vue (1)
711-773:⚠️ Potential issue | 🟡 MinorPer-row action test-ids are not unique across rows.
attachment-type,attachment-view,attachment-delete, andattachment-editare emitted verbatim for every attachment, sopage.getByTestId('attachment-delete')will match N elements the moment a second attachment exists and trip Playwright's strict-mode. The intended scoping is presumably via the outerattachment-row-${attachment.title}, but:
- That row id is keyed on
attachment.title, which is user-editable and not guaranteed unique.- Any spec that reaches for the action button without first locating inside the row will silently pass on single-attachment fixtures and then break as soon as the suite seeds a second file.
Using
attachment.id(stable, unique) for the row — and either leaving the action buttons un-id'd (scope via the row) or suffixing them with the id — would remove both footguns.🔧 Suggested direction
- :data-testid="`attachment-row-${attachment.title}`" + :data-testid="`attachment-row-${attachment.id}`"Then in specs, scope actions inside the row:
const row = page.getByTestId(`attachment-row-${attachmentId}`); await row.getByTestId('attachment-delete').click();🔐 Security aside while we're here:
attachment.titleoriginates from user input, and although Vue'sv-bindescapes attribute values so this isn't an XSS vector, embedding untrusted text in test-id strings also means any CSS attribute-selectors ([data-testid="..."]) written against them could be tripped up by quotes in titles. One more reason to prefer the id.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/item/`[id]/index/edit.vue around lines 711 - 773, Row-level and per-action test IDs are not unique because attachment-row uses attachment.title and action buttons reuse static IDs; change the row data-testid to use the stable unique id (e.g., :data-testid="`attachment-row-${attachment.id}`") and either remove the per-button data-testid attributes (scope via the row in tests) or append the id to them (e.g., data-testid="`attachment-delete-${attachment.id}`", data-testid="`attachment-edit-${attachment.id}`", data-testid="`attachment-view-${attachment.id}`"); update any tests that call deleteAttachment, openAttachmentEditDialog or openDialog(DialogID.ItemImage, ...) to scope via page.getByTestId(`attachment-row-${attachmentId}`) before targeting the button.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/Form/Checkbox.vue`:
- Around line 8-13: The inline branch renders a Label unconditionally causing an
empty, clickable label when only ariaLabel is provided; update the inline branch
to conditionally render the <Label> only when the label prop is present (same
guard used in the non-inline branch) and ensure the Checkbox still receives
:aria-label="ariaLabel || undefined" so screen-readers get the ariaLabel without
creating an empty <Label>; look for the inline branch that renders Label and
Checkbox and add v-if="label" to the Label element (keeping the Checkbox id,
v-model binding, and aria-label behavior unchanged).
In `@frontend/test/e2e/collection-notifiers.spec.ts`:
- Around line 24-34: The test uses fragile positional selection
article.locator("[data-button]").nth(1) in openEditDialog (and the similar
delete selection at lines 115-124); replace these positional selectors with
stable data-testid attributes by updating the notifier row buttons to include
data-testid="notifier-row-edit" and data-testid="notifier-row-delete" and change
the test to locate and click
article.locator('[data-testid="notifier-row-edit"]') (and the delete equivalent)
so the tests target the specific edit/delete buttons directly.
- Around line 17-22: Test currently works around a real i18n bug where the
Notifier dialog title always reads "Edit Notifier"; open a follow-up or fix the
NotifierDialog component so the title uses the correct i18n key based on the
dialog mode (e.g., check the isEdit/mode prop and return either
t('notifier.createTitle') or t('notifier.editTitle') instead of always using
"Edit Notifier"), update the component's conditional that sets the title string,
and then restore the test in collection-notifiers.spec.ts to assert the actual
title (using getByRole('heading' or getByText) rather than bypassing it);
include a brief issue/PR description referencing the failing i18n title and add
a unit/e2e test to cover both create and edit modes.
- Around line 104-112: The checkbox click is ambiguous because
dialog.getByRole("checkbox") can match any checkbox; update both occurrences
(the ones after openEditDialog(...) and before submitAndWaitClose(...)) to
target the checkbox by its accessible name, e.g. dialog.getByRole("checkbox", {
name: /active|enabled/i }).click(), so the test deterministically toggles the
notifier's Active flag; keep openEditDialog and submitAndWaitClose calls
unchanged.
In `@frontend/test/e2e/collection-settings-entity-types.spec.ts`:
- Around line 120-141: This test-side workaround (installEntityTypeCreateFix)
masks a real bug by rewriting "" to null for defaultTemplateId; once the
frontend/component or backend is fixed, remove the installEntityTypeCreateFix
function and its two call sites so the E2E tests exercise real wire format;
while this workaround remains, tighten the route match to a glob like
**/api/v1/entity-types* and add a cheap content-type guard
(req.headers()["content-type"]?.includes("application/json")) before calling
req.postDataJSON() to avoid parsing non-JSON bodies.
In `@frontend/test/e2e/collection-tools.spec.ts`:
- Around line 51-57: The route handlers calling route.fulfill(...) must await or
return the promise so rejections surface; update the page.route(IMPORT_API,
route => { ... }) callback (and the two other similar handlers around the other
mocks) to be async and either await route.fulfill(...) or return
route.fulfill(...), ensuring any rejection from route.fulfill is propagated
instead of becoming an unhandled promise.
In `@frontend/test/e2e/items-advanced-fields.spec.ts`:
- Around line 122-134: The test "warranty: expires-on date without lifetime
checkbox" currently only asserts "Lifetime Warranty" is "No" but never verifies
the actual expires-on date persisted; after calling fillDatePicker(warrantyCard,
page, "06/20/2026") and saveAndReturn(page, itemId) add an assertion that the
detail view shows the warranty expiry (use detailRow(page, "Warranty Expires")
or a similar row key) and assert it contains the expected formatted date string
(match the app's display format e.g., "Jun 20, 2026" or "06/20/2026"); keep the
existing use of setupItemAndOpenEdit, ensureSwitchOn, cardBySectionTitle,
fillDatePicker and saveAndReturn and only append the new detailRow assertion to
validate persistence of the date.
- Around line 77-82: The helper fillDatePicker currently returns silently when
the date input is not found, allowing tests to false-pass; update
fillDatePicker(Locator scope, Page page, string value) so that when (await
input.count()) === 0 it fails loudly (throw an Error or use an assertion) unless
an explicit option like allowMissing=true is passed; reference the existing
function name fillDatePicker and the input locator "input[aria-label='Select
Date']" so the change is localized and callers that intentionally expect a
missing picker can opt in via the new parameter.
In `@frontend/test/e2e/items-attachments.spec.ts`:
- Around line 144-184: The e2e test "set primary image for a photo attachment"
bypasses the UI by PUTting to /api/v1/entities/... instead of exercising the
CheckboxRoot click in the attachment edit dialog, so either rename the test to
truthfully describe that it verifies the API round-trip or add a FIXME and
follow-up issue to restore the UI-driven path: update the test name string in
the test(...) call in frontend/test/e2e/items-attachments.spec.ts to "primary
flag round-trips via the API" (or wrap the current test body with test.fixme and
add a new focused test that clicks
attachmentRow(...).getByTestId('attachment-edit') → dialog.locator('#primary')
to exercise the CheckboxRoot when the portal bug is fixed), and create an issue
referencing the CheckboxRoot/portal aria-checked regression (mentioning
CheckboxRoot, attachment-edit dialog, and the test file) so someone can
re-enable the real click flow and replace the PUT-based assertion later.
In `@frontend/test/e2e/items-crud.spec.ts`:
- Around line 189-191: The test is using a fragile locator that matches the
missing i18n key "global.more_actions" or the fallback "More actions" (the
selector 'button[aria-label="global.more_actions"], button[aria-label="More
actions"]'). Fix by either (A) adding the missing translation for the
"global.more_actions" key in the English locale so the UI uses the proper label
and then simplify the test to a single aria-label locator, or (B) add a stable
attribute to the more-actions trigger (e.g., data-testid="more-actions") in the
component and update the test to use 'button[data-testid="more-actions"]'
instead of the current aria-label-based selector; reference the more-actions
trigger locator in the spec when making the update.
In `@frontend/test/e2e/navigation.spec.ts`:
- Around line 43-49: The test iterates COLLECTION_LINKS and uses
nav.locator(...).last() to ensure it clicks the collapsible sub-menu item rather
than a primary menu duplicate; add a brief inline comment above the await
nav.locator(`a[href="${href}"]`).last().click() line explaining that .last()
targets the collapsible sub-menu entries (e.g., /collection/members appears
twice) so refactors won't accidentally change the selector behavior in the
collection sub-link test (`collection sub-link ${href} navigates correctly`).
In `@frontend/test/e2e/profile.spec.ts`:
- Around line 153-163: The test "legacy header toggle button updates its label"
uses legacyBtn.textContent() which can be null and makes the subsequent
not.toHaveText(initialLabel ?? "") vacuously pass; fix by first
waiting/asserting the initial label is non-empty (use await
expect(legacyBtn).toHaveText(/.+/ or a concrete expected label via
expect(legacyBtn).toHaveText(...) to ensure hydration) or capture it with await
expect(legacyBtn).toHaveText(..., { timeout })) then click and assert the new
label differs from that captured value (use legacyBtn from page.getByRole and
keep the final toggle-back click to avoid polluting shared state).
- Around line 108-114: The current read of each switch's initial state using
locator.getAttribute("data-state") can race with UI hydration; update the
switchesWithInitial gathering to first wait for a real attribute value by using
dialog.locator(...) together with an explicit
expect(locator).toHaveAttribute("data-state", /* non-empty or specific expected
value */) (or expect.not.toHaveAttribute("data-state", "") ) to retry until the
switch is rendered, then call locator.getAttribute("data-state") to capture the
stable initial value; modify the code around switchIds and switchesWithInitial
to perform that wait-before-read sequence so initial is never set to the empty
fallback prematurely.
In `@frontend/test/e2e/quick-menu-label-generator.spec.ts`:
- Around line 26-41: The test uses expect(opened).toBeTruthy() which hard-fails
when openQuickMenuViaKeyboard(browserName) returns false, while other tests call
test.skip(!opened, ...); change this test to use test.skip(!opened, "hotkey not
dispatchable in this browser") immediately after obtaining opened (i.e., call
test.skip(!opened, ...) instead of expect(opened).toBeTruthy()), and make the
handling consistent for other tests in this file that call
openQuickMenuViaKeyboard (replace any remaining expect(opened).toBeTruthy()
checks with the same test.skip(!opened, ...) pattern so all three tests skip
when the hotkey is not dispatchable).
- Around line 104-119: Replace fragile class-based selectors and substring style
assertions in the "updating label width and height recalculates and renders the
preview" test: update the rendered preview element to include a stable attribute
data-testid="label-preview-card" and change the test to locate the card via
page.locator('[data-testid="label-preview-card"]').first() instead of "section
.flex.border-2"; after generating the page (using the existing
"#input-cardWidth", "#input-cardHeight" and "Generate Page" interactions),
assert dimensions using Playwright's CSS checks (e.g.,
expect(card).toHaveCSS("width", /^4(?:in)?$/) and
expect(card).toHaveCSS("height", /^2(?:in)?$/)) or by evaluating
getComputedStyle on the element to parse numeric values, rather than using
style.toContain("width: 4in").
In `@frontend/test/e2e/tags-crud.spec.ts`:
- Around line 121-138: Add an assertion that the original tag name was not
changed after the rejected update: after submitUpdate(updateDialog) and the
existing visibility/URL checks, assert that page.getByRole("heading", { name:
tagName }) (or equivalent selector used elsewhere to show the tag title) is
still visible, and also assert that any success toast like /Tag updated/i is not
present (e.g., ensure no visible toast element with that text). This uses the
existing variables and helpers (tagName, createTagWithName, fillTagName,
submitUpdate, page.getByRole) so the test fails if the backend silently accepts
an empty name or a client-side no-op occurs.
In `@frontend/test/e2e/tags-tree.spec.ts`:
- Around line 181-186: The test "empty tree shows create-tag call-to-action" is
currently grabbing the first "Create" button globally via
page.getByRole(...).first(), which can match header or toolbar buttons; update
the selector to target the empty-state specifically by either scoping to the
empty-state container (e.g.,
locate('[data-testid="tags-empty-state"]').getByRole('button', { name: /create/i
})) or assert on text that only appears in the empty state (anchor to the
empty-state copy string) so that the variable emptyCta or the selector in the
test reliably references the empty-state CTA rather than any Create button on
the page.
- Around line 174-178: The current assertions use global locators
(page.locator("svg").first() and page.getByText(/created/i).first()) which are
too broad; change them to be scoped to the tag detail header by locating the
unique parent (e.g., the tag detail header test id or an ancestor that contains
the heading) and then query within it (for example, replace the global svg
locator with the header-scoped locator and replace the global
getByText(/created/i).first() with the same header-scoped getByText) so the icon
and "Created" metadata assertions only match elements inside the tag detail
header.
In `@frontend/test/e2e/templates.spec.ts`:
- Around line 175-191: The test relies on pressing Escape to close itemDialog
after defaults are applied which will hang if a future "unsaved changes" guard
appears; change the teardown to close the Create Item dialog via its explicit
Cancel/Close button (e.g., use itemDialog.getByRole("button", { name:
/Cancel|Close/i }) and click) so it always dismisses regardless of keyboard
handlers, and add a brief inline comment near the close action mentioning
restoreLastTemplate() / handleTemplateSelected() and the assumption being
enforced so future UX changes don't silently break the test; ensure assertions
still reference templateName, locationName, defaultItemName and defaultDesc
after reopening.
---
Outside diff comments:
In `@frontend/pages/collection/index/entity-types.vue`:
- Line 65: The backend rejects empty-string UUIDs causing 500s when frontend
sends defaultTemplateId: "" (seen in entity-types.vue where defaultTemplateId:
createTemplate.value?.id || ""); update the data contract and calls so the field
is nullable: change defaultTemplateId: string to defaultTemplateId: string |
null in EntityTypeCreate and EntityTypeUpdate in
frontend/lib/api/types/data-contracts.ts, and update usage sites (e.g.,
entity-types.vue's defaultTemplateId assignment and any form submit that
currently falls back to "") to pass null instead of an empty string when no
template is selected.
In `@frontend/pages/item/`[id]/index/edit.vue:
- Around line 711-773: Row-level and per-action test IDs are not unique because
attachment-row uses attachment.title and action buttons reuse static IDs; change
the row data-testid to use the stable unique id (e.g.,
:data-testid="`attachment-row-${attachment.id}`") and either remove the
per-button data-testid attributes (scope via the row in tests) or append the id
to them (e.g., data-testid="`attachment-delete-${attachment.id}`",
data-testid="`attachment-edit-${attachment.id}`",
data-testid="`attachment-view-${attachment.id}`"); update any tests that call
deleteAttachment, openAttachmentEditDialog or openDialog(DialogID.ItemImage,
...) to scope via page.getByTestId(`attachment-row-${attachmentId}`) before
targeting the button.
---
Nitpick comments:
In `@frontend/components/Form/Checkbox.vue`:
- Around line 20-37: The props are currently declared inline with
defineProps({...}) in Checkbox.vue; change to the TypeScript interface form by
creating a Props interface describing modelValue, inline, label, and ariaLabel
types/defaults and replace defineProps({...}) with defineProps<Props>() (and
update usages of props if needed); ensure the interface matches existing
defaults and types so the component behavior and any existing emits remain
unchanged.
In `@frontend/components/Item/CreateModal.vue`:
- Line 669: The current fallback uses || which treats empty string as falsy and
could unintentionally use form.location.id; change the expression in the
CreateModal component where parentId is set (look for parentId: form.parentId ||
(form.location.id as string)) to use nullish coalescing so it only falls back
when form.parentId is null/undefined (e.g., replace with form.parentId ??
(form.location.id as string)), keeping the parentheses for readability and
preserving the intended string typing.
In `@frontend/components/Location/Tree/Node.vue`:
- Around line 58-86: The testids use item.name which can collide for duplicate
names; change the data-testid values in Node.vue to include the unique item.id
(e.g., `location-tree-node-${item.id}-${item.name}` or at minimum append
`${item.id}`) for the three places referencing
`location-tree-node-${item.name}`, `location-tree-toggle-${item.name}`, and
`location-tree-link-${item.name}` so each DOM node is uniquely addressable;
update any tests that query those testids to use the new id-based testid format.
In `@frontend/components/Template/Card.vue`:
- Line 86: Change the Card's test id to use the stable unique identifier instead
of the mutable name: replace the use of template.name in the Card prop (the Card
component instance where data-testid is set) with template.id, and add a
separate attribute (e.g., data-template-name) that preserves the visible
template name for tests that need to match by text; then update e2e tests
(frontend/test/e2e/templates.spec.ts) to either query by the new data-testid
using template.id or locate cards by visible title text or the new
data-template-name attribute.
In `@frontend/components/Template/Selector.vue`:
- Line 7: Add a symmetric stable test id to the full-mode trigger Button in
Selector.vue: locate the full-mode Button (the non-compact trigger in the
Template Selector component) and add a data-testid attribute (e.g.,
data-testid="template-selector") to it so tests can target the non-compact
variant the same way the compact variant uses
data-testid="template-selector-compact".
In `@frontend/pages/location/`[id]/index.vue:
- Around line 1-14: Replace the two script blocks with a single <script setup
lang="ts">: remove the empty <script setup lang="ts"> and the separate <script
lang="ts">, move the call to definePageMeta(...) into the single script-setup
block, and drop the manual import of BaseContainer since it's auto-imported;
keep using NuxtPage and BaseContainer in the template as-is. Ensure the final
file contains only one <script setup lang="ts"> with definePageMeta and no
redundant imports or empty script tags.
In `@frontend/test/e2e/auth.spec.ts`:
- Around line 53-78: The helper firstRegister declared in the test "duplicate
email registration shows error toast" has an unused parameter and a misleading
name; remove the dead parameter from its declaration, rename the helper to
something clearer (e.g., waitForRegisterResponse or waitForRegisterRequest), and
update both call sites (the two places currently passing "first" and "second")
to call the new helper without arguments; ensure you keep the predicate that
matches POST requests to "/api/v1/users/register" and preserve awaiting the
returned Response used by firstResp/secondResp (rename those variables if you
rename the helper) so the existing status assertions still work.
- Around line 19-32: The test "invalid credentials surface an error toast"
currently uses fixed sleeps (page.waitForTimeout(500)) and a brittle
class-substring selector ("div[class*='login-error']"); remove the
waitForTimeout calls and replace the locator with a stable test hook (e.g.
page.locator('[data-testid="login-error"]') or similar) and use Playwright's
auto-retrying assertions (await expect(...).toBeVisible() and then await
expect(...).toHaveText("Invalid email or password")) so the test waits reliably
and doesn't depend on generated CSS class names.
In `@frontend/test/e2e/collection-members-invites.spec.ts`:
- Around line 42-46: Replace brittle CSS/positional locators in the e2e test
with stable data-testid hooks: change the locator that sets tokenCell (currently
using "span.font-mono" and .first()) to query by a new data-testid like
"invite-token", read its text into the token variable as before, and replace the
"first button in the row" copy-button locator with a data-testid like
"invite-token-copy" (also update the other similar occurrences mentioned around
lines 78-80). Update the test to expect the same token format and visibility
timeouts but use page.locator('[data-testid="invite-token"]') and
page.locator('[data-testid="invite-token-copy"]') to make the test robust to
structural/CSS changes.
In `@frontend/test/e2e/collection-notifiers.spec.ts`:
- Line 5: VALID_URL uses a non-networking Apprise scheme but lacks a comment
explaining that intent; add a concise inline comment next to the VALID_URL
constant stating that "generic://example.com/webhook?template=json" is
intentionally chosen to satisfy the Apprise URL parser without performing real
outbound network calls or hitting third-party hosts (and warn maintainers not to
replace it with https://example.com or other real schemes), so future edits
don't accidentally enable external HTTP calls—update the constant declaration
for VALID_URL accordingly.
In `@frontend/test/e2e/collection-settings-entity-types.spec.ts`:
- Around line 25-34: The isVisible().catch(() => false) check on the
headerCreate locator is racy; replace that pattern in the helper that returns
the dialog (where headerCreate is defined) with a deterministic check such as
using headerCreate.count() > 0 or using an OR locator
(page.getByRole("main").getByRole("button", { name: "Create", exact: true
}).or(page.getByRole("button", { name: "Create Entity Type" }))) and then click
the visible/first match, or explicitly waitFor the button to be attached/visible
before clicking; update the logic around headerCreate and the fallback click so
clicks only occur after a reliable existence/visibility check.
- Around line 143-201: Add three new E2E tests to cover missing negative and
alternative paths: 1) a test that uses openCreateEntityTypeDialog + inputByLabel
to create an entity type while selecting a non-empty default template (exercise
non-empty defaultTemplateId path), 2) a test that attempts to submit an empty
name via fillNameAndSubmit (or directly clicking the Create button) and asserts
the "Name is required" toast appears, and 3) a test that creates/marks an entity
referencing a type and then calls deleteEntityType to assert the "Failed to
delete… Make sure no entities are using it." toast is shown; use
cardForEntityType and deleteEntityType to locate and delete the type in each
flow and reuse registerAndLogin/gotoEntityTypes/setup helpers from the spec.
- Around line 54-59: The deletion test's final assertion in deleteEntityType
currently queries the whole page (expect(page.getByText(name, { exact: true
})).toHaveCount(0)), which can falsely pass/fail if the name appears elsewhere
(e.g., toast); instead scope the assertion to the entity-type cards container or
the specific card locator returned by cardForEntityType/deleteButton: after
clicking confirm, wait for the card locator (cardForEntityType(page, name)) to
be detached/hidden or assert that the cards container (the parent locator that
holds all entity-type cards) no longer contains the text by calling getByText on
that container and expecting count 0—use the existing cardForEntityType and
deleteButton symbols to locate the correct element to assert against.
- Around line 36-52: The selectors are brittle (relying on Tailwind classes) —
update cardForEntityType, editButton, and deleteButton to use stable data-testid
attributes instead: have cardForEntityType locate the entity card via a
data-testid (e.g., locator('[data-testid="entity-type-card"]').filter({ has:
page.getByText(name, { exact: true }) }).first()), and have editButton and
deleteButton find buttons inside that card via specific data-testids (e.g.,
'[data-testid="entity-type-edit"]' and '[data-testid="entity-type-delete"]').
Ensure the corresponding data-testid attributes are added to
frontend/pages/collection/index/entity-types.vue so the new selectors match the
DOM.
In `@frontend/test/e2e/collection-tools.spec.ts`:
- Around line 12-16: The explicit await page.waitForLoadState("networkidle") in
openTools should be removed because SSE/long-poll can block it; instead rely on
the existing readiness assertion await expect(page.getByRole("heading", { name:
"Import Inventory" })).toBeVisible() after navigating to TOOLS_ROUTE. Edit the
openTools function to drop the waitForLoadState call and keep the navigation and
the visible-heading assertion (use the same TOOLS_ROUTE and the heading
selector) so the test is faster and resilient to persistent connections.
- Around line 99-104: The test uses a fragile selector page.getByRole("button",
{ name: "Confirm", exact: true }).last() which relies on DOM order; instead
scope the lookup to the confirmation alert dialog (use
page.getByRole("alertdialog") or locate the dialog container) then call
getByRole("button", { name: "Confirm", exact: true }) within that scope before
clicking and waiting for ENSURE_IDS_API; apply the same scoping change to the
other occurrences referenced (around lines 118-123) so the Confirm button is
selected from the dialog context rather than the entire page.
- Around line 50-77: Add a negative assertion after the mocked 422 response to
ensure no success side-effects occur: after awaiting the request
(requestPromise) and asserting the error toast (page.getByText("Import failed.
Please try again later.")), also assert that the success toast string is not
visible (e.g., page.getByText("Import successful") or the actual success message
you use) and that no navigation or list mutation happened (e.g., verify URL
unchanged via page.url() or that the inventory list/table does not contain the
new row). Reference the existing selectors used in this test
(page.getByRole("button", { name: "Submit" }), page.getByText(...),
page.locator("input[type='file']")) and add the negative checks immediately
after the current error-toast assertion.
In `@frontend/test/e2e/helpers/auth.ts`:
- Around line 17-32: Update the error thrown when fetching entity types so it
includes the response body like the later create-location error: when the
request stored in etRes (the GET to "/api/v1/entity-types") is not ok, throw a
new Error that contains both etRes.status() and await etRes.text() to make
failures symmetric and easier to triage.
- Line 4: The constant STRONG_PASSWORD should not be hardcoded for shared or
demo environments; change the declaration for STRONG_PASSWORD to read from an
environment variable with a safe fallback (e.g.,
process.env.HOMEBOX_E2E_PASSWORD ?? "ThisIsAStrongDemoPass") so CI/staging can
override it, and ensure the value is not copied into demo/docker-compose seeds
or public example docs (remove or replace any occurrences used for seeding or
examples). Keep the symbol STRONG_PASSWORD but make it environment-configurable
and audit/remove uses in any seeding or example files to prevent accidental
copy-paste into real deployments.
In `@frontend/test/e2e/home-dashboard.spec.ts`:
- Around line 62-69: The test currently relies on ordering via values.nth(1)
which couples it to statistics.ts ordering; update the StatCard component (e.g.,
StatCard root) to accept or compute a semantic data-testid (for example
"stat-card-total-items") and set that on the Total Items card in statistics.ts,
then change the test to select page.getByTestId("stat-card-total-items") (or
page.getByTestId("stat-card").filter/with that id) and assert it contains "0";
also remove the ordering comment that explains the nth() dependency.
- Around line 99-100: The assertions using page.getByText(..., { exact: false
}).first() are too loose and can match unrelated UI text; instead scope the
lookup to the recent-items panel or row (e.g., use a data-testid on the
recent-items container/row) and assert against that scoped locator, or use
exact: true with the exact faker-generated itemName/locationName to ensure the
text is an exact match; update the assertions that reference page.getByText,
itemName, and locationName to use page.locator('[data-testid="recent-items"]')
(or the specific row testid) and then .getByText(..., { exact: true }) or assert
against the row locator directly.
- Around line 103-122: The test title test("navigates to items, locations, and
tags routes from the dashboard", ...) is misleading because the test uses
page.goto(...) rather than interacting with the dashboard; rename the test title
to reflect what it actually asserts (for example "items/locations/tags routes
are reachable" or similar) in the home-dashboard.spec.ts test block so the name
matches the behavior of the test and update any nearby comment if desired.
In `@frontend/test/e2e/items-advanced-fields.spec.ts`:
- Around line 85-102: The test flakes because saveAndReturn only waits for the
URL change and the server PATCH may still be in-flight; update the test (or
modify saveAndReturn) to wait for the authoritative server response before
asserting persistence — for example, after triggering Save in saveAndReturn,
await a page.waitForResponse that matches /\/api\/v1\/entities\/[^/]+$/ (or the
app's PATCH endpoint) in addition to the navigation wait so the assertions in
"purchase details (price, purchased from, date) persist and render" run only
after the server update has completed.
- Around line 85-239: The assertions using
detailRow(...).toContainText("Yes"/"No") are substring matches and can
false-pass; update those assertions (e.g., in tests "warranty: lifetime checkbox
persists and shows Yes on detail", "warranty: expires-on date without lifetime
checkbox", "insurance checkbox toggles to Yes on detail view", and "archive
checkbox persists, then unarchive restores No") to assert exact text by either
scoping to the specific <dd> element returned by detailRow and using
toHaveText("Yes")/toHaveText("No") or use a regex like toHaveText(/^\s*Yes\s*$/)
and toHaveText(/^\s*No\s*$/) so the check matches the whole cell rather than
substrings.
- Around line 62-66: The helper cardBySectionTitle currently anchors to a
Tailwind class via ancestor::div[contains(`@class`,'rounded-lg')], which is
fragile; update tests and components so each Card root gets a stable data-testid
(e.g. data-testid="card-purchase-details" or "card-warranty-details"), then
change cardBySectionTitle to locate the heading (getByRole("heading", { name:
title, level: 3 })) and scope it to the nearest card by that data-testid (use a
test helper or Locator.locator('[data-testid="..."]') or getByTestId) instead of
relying on the rounded-lg class so the selector is stable across style
refactors.
In `@frontend/test/e2e/items-attachments.spec.ts`:
- Around line 68-85: In uploadAttachment, remove the redundant await
page.waitForLoadState("networkidle") call and rely on the existing awaited
uploadResponse (the waitForResponse matching POST to
/api/v1/entities/*/attachments) followed by the
expect(...attachment-row-<name>).toBeVisible; ensure uploadResponse is still
awaited before polling for the row so the POST response and subsequent UI update
are the only synchronization points.
- Line 3: The import line imports STRONG_PASSWORD but it isn't used; update the
import to only bring in registerAndLogin (remove STRONG_PASSWORD) so the file's
import surface is minimal and linter warnings are removed—locate the import
statement that currently reads `import { registerAndLogin, STRONG_PASSWORD }
from "./helpers/auth";` and change it to import only `registerAndLogin`.
In `@frontend/test/e2e/items-bulk-ops.spec.ts`:
- Around line 170-177: The code fetches entity types and assigns destType but
never uses it; remove the unused block that GETs "/api/v1/entity-types" and the
destType lookup (including the expect(destType).toBeDefined()) OR, if you
intended to use the entity type id, replace the later name-based location
resolution with a query that uses destType.id; locate the fetch call and the
destType variable in frontend/test/e2e/items-bulk-ops.spec.ts and either delete
that entire block (request.get("/api/v1/entity-types") + destType find + expect)
or wire destType.id into the subsequent entity query logic.
- Around line 13-46: These tests define duplicate helpers (createLocation,
createTag, createItem) that duplicate existing/shared helpers; remove these
local helpers and import the centralized versions instead (use ApiEntity and
apiCreateLocation from helpers/auth.ts if present, or add shared helpers to a
new helpers/api.ts and export apiCreateTag/apiCreateItem alongside ApiEntity);
update this spec to import and call the shared functions (or add thin adapter
functions here that call the shared helpers) so payload shape and entityTypeId
logic are maintained in one place and referenced by the test via the shared
function names.
- Around line 48-53: Remove the brittle waitForLoadState("networkidle") calls
and replace them with explicit UI waits: in gotoItemsTableView() drop await
page.waitForLoadState("networkidle") and rely on the await
page.getByRole("button", { name: "Table" }).click() to wait for the button to be
actionable (or add a targeted expect like expect(page.getByRole("button", {
name: "Table" })).toBeVisible() before clicking); similarly remove the redundant
waitForLoadState("networkidle") in items-attachments.spec.ts (and the other
tests mentioned: wipe-inventory.browser.spec.ts, tags-tree.spec.ts,
collection-tools.spec.ts) where a following
expect(...).toBeVisible()/toBeEnabled() already provides the concrete UI signal.
Ensure you only remove the networkidle waits and, where necessary, add a small
explicit expect on the specific element used in the test to guarantee the UI is
ready.
In `@frontend/test/e2e/items-search-filter.spec.ts`:
- Around line 28-44: The test helpers leave itemType plumbing unused: update
createItem to forward params.itemTypeId into the POST body (include itemTypeId
when present) and/or remove itemTypeId from createItem's param type; also update
pickEntityTypeIds usages so tests either destructure and pass the returned
itemType (e.g., pass itemType.id as itemTypeId to createItem) or stop
returning/using itemType entirely. Locate the functions createItem and
pickEntityTypeIds and all test call-sites that destructure { itemType } (and
those passing itemTypeId: "") and either wire the itemType.id through to
createItem's POST payload or remove the dead parameter and corresponding
returns/usages across the suite.
In `@frontend/test/e2e/locations-crud.spec.ts`:
- Line 77: The tests use toHaveText(new RegExp(name)) which is unanchored and
can fail or throw for regex metacharacters; update each assertion that uses new
RegExp(name) (e.g., the calls on page.getByTestId("location-detail-name"),
page.getByTestId("...") at the other occurrences) to use toContainText(name)
instead (or, if you need an exact match, use an anchored/escaped regex) so the
checks match the literal generated name safely and deterministically.
- Line 3: Remove the dead helpers: delete the unused import STRONG_PASSWORD from
the top of the file and remove the unused helper functions expandAdvanced and
fillLocationNotes from this spec; if you choose to keep the helpers instead,
wire them into the existing tests by replacing the manual xpath walk in the
"notes" flow with a call to fillLocationNotes and use STRONG_PASSWORD where a
strong password is needed in registerAndLogin. Ensure you update any references
so no unused symbols (STRONG_PASSWORD, expandAdvanced, fillLocationNotes)
remain.
In `@frontend/test/e2e/locations-tree.spec.ts`:
- Around line 61-79: The test "show-items toggle button can be clicked without
breaking the tree" assumes a fixed initial showItems value; instead read the
current URL before clicking and assert that the URL flips to the opposite value
after each click (use page.url() or page.locator to check for /showItems=true/
vs /showItems=false/), e.g. capture initialHasTrue =
page.url().includes("showItems=true") then after toggle.click() assert the URL
contains the negation, repeat for the second click; update references to the
toggle locator (getByTestId("location-tree-toggle-items")) and the test block
name accordingly so the assertions are robust to default-state changes.
In `@frontend/test/e2e/maintenance.spec.ts`:
- Around line 26-33: The test creates an `option` locator but never uses it,
relying on `search.press("Enter")`; change it to explicitly click the located
option to make selection deterministic: after `await
expect(option).toBeVisible();` add `await option.click();` (using the existing
`option` locator from `page.getByRole("option", { name: locationName, exact:
true }).first()`), and then keep the final assertion that the `dialog` combobox
contains `locationName`; remove or replace the `search.press("Enter")` call so
selection is performed by `option.click()` instead.
- Around line 144-152: The confirm-button selector in the test uses a loose
regex (/Confirm/i) which can match unintended labels; update the dialog
interaction so the Confirm button is selected with an exact name matcher instead
(use confirmDialog.getByRole("button", { name: "Confirm", exact: true }) ),
keeping the rest of the file's selectors consistent with exact match semantics;
the change should be applied where confirmDialog is defined and used in the
deletion flow alongside maintenanceCard, showAllMaintenance, and page.getByRole
calls.
- Around line 74-83: The test eagerly consumes the response body with await
res.text() when constructing the failure message; change the assertion to first
check res.status() and only read the body on failure so successful paths don't
wastefully consume the stream. Concretely, update the block around
page.request.post / res to assert status (e.g., if (res.status() >= 400) { const
body = await res.text(); throw new Error(`maintenance create failed: ${body}`);
}) or wrap the expect so that await res.text() is executed only inside the
failure branch; reference the res variable and the expect(res.status(), ...)
assertion to locate the code to change. Also keep in mind that
opts.cost/opts.notes are forwarded directly by page.request.post if you later
reuse this helper in untrusted contexts.
- Around line 51-62: The maintenanceCard helper is brittle and its JSDoc is out
of sync: update the function maintenanceCard to select the card by a stable
data-testid (e.g. data-testid="maintenance-card") instead of the Tailwind
selector "div.shadow-xl", and adjust the JSDoc to describe that it locates the
maintenance card via that test-id (or, if you keep the Delete-button anchor
behavior, implement the documented approach by locating the Delete button and
walking up to the card root). Ensure the selector uses
page.locator('[data-testid="maintenance-card"]').filter({ hasText: entryName })
(or the equivalent Delete-button-based traversal) so tests are resilient.
In `@frontend/test/e2e/navigation.spec.ts`:
- Around line 17-20: The tests call registerAndLogin in test.beforeEach which
creates a fresh user per test; change to a per-worker or shared storage-state
approach to avoid repeated registration: create authentication once (e.g., in a
worker-scoped fixture or a test.beforeAll for this file) that runs
registerAndLogin or performs a single login and saves Playwright storageState,
then have each test reuse that saved storageState instead of calling
registerAndLogin in test.beforeEach; update references to registerAndLogin and
test.beforeEach/test.beforeAll accordingly and remove the per-test registration
to reuse the shared auth.
- Around line 51-60: The test uses a brittle selector; add a stable data-testid
to the Selector component by setting data-testid="collection-selector" in
components/ui/sidebar/.../Selector.vue, then update the "collection selector
dropdown opens" test to locate the element via
page.locator('[data-testid="collection-selector"]') (replace the current
selector variable) and keep the same assertions; optionally, after opening the
dropdown call page.keyboard.press('Escape') if you want to close it for chained
assertions.
- Around line 34-49: The URL regex used in the assertions for PRIMARY_LINKS and
COLLECTION_LINKS is not anchored at the start, which risks false positives;
update the expect(page).toHaveURL calls (inside the tests that use
page.locator(...).click()) to either assert the exact path (compare the page URL
string or use toHaveURL with an exact string) or use a start-anchored, escaped
regex that matches the href and optional query only (i.e., prepend ^ and escape
slashes in href before adding the optional query part); apply the same change to
both loops that reference PRIMARY_LINKS and COLLECTION_LINKS so the navigation
tests use the new anchored/escaped match.
In `@frontend/test/e2e/profile.spec.ts`:
- Line 2: Remove the unused import of the symbol "faker" from the imports in
this test file (profile.spec.ts); the import statement "import { faker } from
\"@faker-js/faker\";" should be deleted so the file no longer references an
unused symbol and the ESLint warning is resolved.
- Around line 45-64: The "change password: matching current password succeeds"
test is permanently skipped via test.skip which hides missing positive-path
coverage; replace test.skip with test.fixme (so CI will fail once the underlying
issue is resolved) or, if skip must remain, add a one-line blocker comment
referencing the issue/PR that explains why the test is skipped; update the test
declaration (the symbol test.skip in profile.spec.ts) accordingly and ensure the
intent is clear to future maintainers.
- Around line 87-99: The test "language switcher changes the active locale"
(involving gotoProfile and the combobox/listbox interactions) leaves the app
locale mutated; update the test to either assert the exact expected locale
("de") instead of only not "en" (use await
expect(page.locator("html")).toHaveAttribute("lang", "de")) or restore English
at the end of the test by selecting the English option before finishing to avoid
leaking state to other tests.
In `@frontend/test/e2e/quick-menu-label-generator.spec.ts`:
- Around line 144-151: The test "entering oversized dimensions surfaces a toast"
uses hard-coded "50" which may fall below future validation thresholds; update
the values filled into `#input-cardWidth` and `#input-cardHeight` to a clearly
oversized number (e.g., 999) or derive them from the app's threshold constant if
available (referencing the threshold constant or config), and add a short
comment linking to the validation threshold to ensure the test continues to
exercise the oversized path; keep the locator usage
(page.locator("#input-cardWidth"), page.locator("#input-cardHeight"), and the
Generate Page button) unchanged.
- Around line 121-142: The tests assume checkbox getAttribute("data-state") is
non-null; before clicking assert the initial value is present and valid (e.g.,
not null and one of the expected values like "true" or "false") to guard against
Reka-UI not yet mounting; update both tests "bordered labels checkbox can be
toggled" (locator "#borderedLabels") and "print location row checkbox can be
toggled" (locator "#printLocationRow") to check the initial attribute is not
null and is an allowed value before performing the click and subsequent polling.
In `@frontend/test/e2e/tags-crud.spec.ts`:
- Around line 5-46: Extract the duplicated tag-dialog helper functions into a
shared helper module and replace the duplicates in both specs: create a new file
./helpers/tags.ts that exports the functions getDialog, openCreateTagDialog,
fillTagName, selectIcon, randomizeColor, submitCreateAndExpectNavigation,
submitUpdate, and createTagWithName (matching the existing signatures), then
update frontend/test/e2e/tags-crud.spec.ts and
frontend/test/e2e/tags-tree.spec.ts to import these helpers instead of defining
them locally so future changes to the dialog title or hotkey are made in one
place.
In `@frontend/test/e2e/tags-tree.spec.ts`:
- Around line 46-48: Add a stable data-testid to the tag tree node component
(modify Tag/Tree/Node.vue to include an attribute like
data-testid="tag-tree-node" and optionally data-testid-name="{name}" or
data-testid="tag-tree-node-{id}") and update the test helper treeNodeLink to
select nodes by that data-testid instead of the href selector; locate the test
function treeNodeLink(Page, name) in frontend/test/e2e/tags-tree.spec.ts and
replace the href^="/tag/" filter with a locator that targets
data-testid="tag-tree-node" and filters by the node name (or uses the per-node
data-testid if you add id/name-specific testids).
In `@frontend/test/e2e/templates.spec.ts`:
- Around line 5-15: The test is duplicating hotkey logic in createLocation;
extract the hotkey behavior into a single helper module (e.g., create
helpers/hotkeys.ts) that exports functions like openCreateLocation(page),
openCreateTag(page), and openCreateItem(page), then update createLocation to
call openCreateLocation(page) (and replace similar hotkey sequences in
tags-*.spec.ts and templates/item specs to call openCreateTag/page and
openCreateItem/page respectively) so the shortcut mapping is centralized and
maintained in one place.
- Around line 143-146: The test uses a generic defaultDesc for "Item
Description" (dialog.getByLabel("Item Description") and later assertions) while
earlier tests use templateDescription, causing potential confusion; rename the
local variable(s) to clearly reflect intent (e.g., defaultItemDescription for
the third test and templateDescription for the first) and update all usages of
the variable in the third test (the fill call via dialog.getByLabel("Item
Description", ...), any subsequent assertions, and any helper call like
selectLocationInDialog(dialog, locationName) if it references the same variable)
so the names match their purpose and avoid mixing "Template Description" vs
"Item Description".
- Around line 31-44: The repeated "type-to-filter + Enter" workaround should be
extracted into a shared helper: add a function selectComboboxOption(combo:
Locator, label: string) in ./helpers/reka.ts that clicks the combobox, finds the
CommandInput/search input (e.g., page.getByPlaceholder(/search/i).last()), fills
it with label, awaits the option (page.getByRole("option", { name: label, exact:
true }).first()) to be visible, presses Enter, and then asserts the combo
contains the label; export it and replace the inline code in
selectLocationInDialog (and the similar block in tags-tree.spec.ts) to simply
call selectComboboxOption(combo, locationName) so the workaround lives in one
place.
- Around line 107-125: Add a new e2e test "duplicate a template from the
templates page card": reuse registerAndLogin, gotoTemplatesPage and
openCreateTemplateModal to create a template (use faker for unique name), then
locate the created card via page.getByTestId(`template-card-${templateName}`),
click its template-card-duplicate testid button, confirm any duplication dialog
if present, and assert that there are now two matching cards
(expect(page.getByTestId(`template-card-${templateName}`)).toHaveCount(2) or
equivalent visibility checks) to ensure duplication created an independent copy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 59ba1912-8ced-4536-9a30-ca4da368a804
📒 Files selected for processing (42)
frontend/assets/css/main.cssfrontend/components/Form/Checkbox.vuefrontend/components/Item/CreateModal.vuefrontend/components/Location/CreateModal.vuefrontend/components/Location/Tree/Node.vuefrontend/components/Scanner/ARControls.vuefrontend/components/Scanner/AROverlayCard.vuefrontend/components/Template/Card.vuefrontend/components/Template/CreateModal.vuefrontend/components/Template/Selector.vuefrontend/components/global/StatCard/StatCard.vuefrontend/lib/api/classes/items.tsfrontend/pages/collection/index/entity-types.vuefrontend/pages/item/[id]/index/edit.vuefrontend/pages/location/[id]/index.vuefrontend/pages/location/[id]/index/index.vuefrontend/pages/locations.vuefrontend/pages/tags.vuefrontend/pages/template/[id].vuefrontend/pages/templates.vuefrontend/test/e2e/auth.spec.tsfrontend/test/e2e/collection-members-invites.spec.tsfrontend/test/e2e/collection-notifiers.spec.tsfrontend/test/e2e/collection-settings-entity-types.spec.tsfrontend/test/e2e/collection-tools.spec.tsfrontend/test/e2e/helpers/auth.tsfrontend/test/e2e/home-dashboard.spec.tsfrontend/test/e2e/items-advanced-fields.spec.tsfrontend/test/e2e/items-attachments.spec.tsfrontend/test/e2e/items-bulk-ops.spec.tsfrontend/test/e2e/items-crud.spec.tsfrontend/test/e2e/items-search-filter.spec.tsfrontend/test/e2e/locations-crud.spec.tsfrontend/test/e2e/locations-tree.spec.tsfrontend/test/e2e/login.browser.spec.tsfrontend/test/e2e/maintenance.spec.tsfrontend/test/e2e/navigation.spec.tsfrontend/test/e2e/profile.spec.tsfrontend/test/e2e/quick-menu-label-generator.spec.tsfrontend/test/e2e/tags-crud.spec.tsfrontend/test/e2e/tags-tree.spec.tsfrontend/test/e2e/templates.spec.ts
💤 Files with no reviewable changes (2)
- frontend/assets/css/main.css
- frontend/test/e2e/login.browser.spec.ts
Comment on lines
+120
to
+141
| // Workaround: the entity-types page serializes `defaultTemplateId: ""` which the backend | ||
| // rejects with a 500 (the *uuid.UUID JSON decoder can't parse an empty string). | ||
| // Rewrite outgoing create/update requests to send `null` instead until the component is fixed. | ||
| async function installEntityTypeCreateFix(page: Page) { | ||
| await page.route(/\/api\/v1\/entity-types/, async route => { | ||
| const req = route.request(); | ||
| const method = req.method(); | ||
| if (method === "POST" || method === "PUT") { | ||
| try { | ||
| const data = req.postDataJSON() as Record<string, unknown> | null; | ||
| if (data && data.defaultTemplateId === "") { | ||
| data.defaultTemplateId = null; | ||
| await route.continue({ postData: JSON.stringify(data) }); | ||
| return; | ||
| } | ||
| } catch { | ||
| // fall through to default continue | ||
| } | ||
| } | ||
| await route.continue(); | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
🛡️ Test-side workaround masks a real production bug — track and remove.
The comment is admirably honest — the backend's *uuid.UUID decoder 500s on "", and users hit it whenever they create/update an entity type without a default template. See my comment on frontend/pages/collection/index/entity-types.vue lines 65/115 for the root cause.
Once the component (or backend) is patched:
- Delete
installEntityTypeCreateFixand its two call sites. - Keep the E2E tests — they'll now genuinely exercise the real wire format.
Minor concerns while this stays:
- The regex
/\/api\/v1\/entity-types/also matchesGETto listing/detail endpoints; themethodfilter short-circuits correctly, but pin it tighter (e.g.**/api/v1/entity-types*glob + method guard) if you add more entity-types endpoints later. postDataJSON()will throw on multipart/non-JSON bodies; thetry/catchhandles it, but a body-type check (req.headers()["content-type"]?.includes("application/json")) is slightly cheaper and less surprising.
Want me to open a follow-up issue to track removing this workaround once the defaultTemplateId nullability fix lands?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/test/e2e/collection-settings-entity-types.spec.ts` around lines 120
- 141, This test-side workaround (installEntityTypeCreateFix) masks a real bug
by rewriting "" to null for defaultTemplateId; once the frontend/component or
backend is fixed, remove the installEntityTypeCreateFix function and its two
call sites so the E2E tests exercise real wire format; while this workaround
remains, tighten the route match to a glob like **/api/v1/entity-types* and add
a cheap content-type guard
(req.headers()["content-type"]?.includes("application/json")) before calling
req.postDataJSON() to avoid parsing non-JSON bodies.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/test/e2e/collection-members-invites.spec.ts (1)
42-46: Fragile selector:span.font-monoleans on Tailwind styling as a contract 🎯This PR's stated theme is "injects stable
data-testidattributes across many UI templates", but the token cell is located via a styling class (span.font-mono) plus positional.first(). Any future refactor that changes the font utility — or introduces another mono-styled span earlier on the page (e.g., an ID column, a code snippet) — will silently grab the wrong element, andtoMatch(/^[A-Z0-9]{26}$/)will then fail with a confusing message.Prefer a
data-testidon the invite token cell (e.g.,data-testid="invite-token") to match the selector strategy used elsewhere in this PR. Same reasoning applies torow.getByRole("button").first()on line 81 — a dedicated testid on the copy button would be more robust than "first button in row". As per coding guidelines:frontend/test/e2e/**/*.spec.ts : Write E2E tests using Playwright for user workflows and page interactions— durable selectors make those workflows less flappy in CI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/test/e2e/collection-members-invites.spec.ts` around lines 42 - 46, Replace the fragile style-based selectors with durable test IDs: change the lookup of the invite token (currently using the selector "span.font-mono" assigned to tokenCell and variable token) to use a data-testid like data-testid="invite-token" and update the test to locate that element; likewise replace row.getByRole("button").first() (the copy button lookup) with a dedicated data-testid such as data-testid="invite-copy-btn". Update the page templates/components to add these attributes and adjust the spec to use page.getByTestId("invite-token") and row.getByTestId("invite-copy-btn") so the tests no longer depend on styling or element order.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/test/e2e/collection-members-invites.spec.ts`:
- Around line 101-110: The test uses faker.internet.email() for inviteeEmail and
a loose assertion on regRes which risks flakey 409 collisions and leaking
request bodies; update inviteeEmail to append a unique suffix (e.g., Date.now()
or crypto.randomUUID()) to avoid collisions, tighten the assertion to
expect(regRes.ok()).toBeTruthy() (i.e., require 2xx) instead of status() < 400,
and remove regRes.text() from the failure message to avoid potentially logging
sensitive fields (PASSWORD) — references: inviteeEmail, regRes, PASSWORD, token.
- Around line 62-90: The test declares it "copies" but never verifies clipboard
contents; after calling createInvite and clicking the copy button (refer to the
test "creates, displays, copies, and deletes", the copyButton locator and
createInvite helper), add a Chromium-only clipboard assertion: ensure
context.grantPermissions(...) is already called for
"clipboard-read"/"clipboard-write" when browserName === "chromium", then read
the clipboard (e.g., via page.evaluate(() => navigator.clipboard.readText()) or
equivalent) and assert the result equals the token variable so the copy behavior
is actually tested; keep this check gated on browserName === "chromium" so
Firefox/WebKit still skip it.
---
Nitpick comments:
In `@frontend/test/e2e/collection-members-invites.spec.ts`:
- Around line 42-46: Replace the fragile style-based selectors with durable test
IDs: change the lookup of the invite token (currently using the selector
"span.font-mono" assigned to tokenCell and variable token) to use a data-testid
like data-testid="invite-token" and update the test to locate that element;
likewise replace row.getByRole("button").first() (the copy button lookup) with a
dedicated data-testid such as data-testid="invite-copy-btn". Update the page
templates/components to add these attributes and adjust the spec to use
page.getByTestId("invite-token") and row.getByTestId("invite-copy-btn") so the
tests no longer depend on styling or element order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f2ec9be8-1cd3-420e-8e5e-e44968950e75
📒 Files selected for processing (1)
frontend/test/e2e/collection-members-invites.spec.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Taskfile.yml (1)
91-91: Considererrorinstead offatalto keep a breadcrumb trail when CI fails. 🔦Setting
HBOX_LOG_LEVEL: fatalsilencesdebug,info,warn, anderrorlogs on the backend duringgo:ci. That certainly tames the "crazy logs" the commit message mentions, but when a Playwright run fails in CI you'll also lose the backenderrorlogs that are often the single most useful clue for diagnosing the failure — leaving you to re-run locally just to see what blew up.A slightly less aggressive level would give you the quiet CI output you want while still preserving genuine failure signals:
🔧 Suggested tweak
go:ci: env: HBOX_DEMO: true - HBOX_LOG_LEVEL: fatal + HBOX_LOG_LEVEL: error desc: Runs all go test and lint related tasksSecurity side-note while we're here 🔐:
fatalalso hides auth/permission-relatedwarn/errorlogs (failed logins, unauthorized API hits, etc.) during e2e runs. Not a vulnerability on its own — these tests run against an ephemeral CI instance — but worth keeping in mind if these logs ever feed into security regression checks or anomaly dashboards down the line.errorpreserves the auth-failure signal without bringing back the noise.Not a blocker — feel free to keep
fatalif the noise reduction is more valuable than post-mortem error visibility for your workflow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Taskfile.yml` at line 91, Change the HBOX_LOG_LEVEL from "fatal" to "error" so backend error logs are preserved during CI runs; locate the HBOX_LOG_LEVEL entry (symbol: HBOX_LOG_LEVEL) in Taskfile.yml and update its value to "error" to keep error-level logs while still silencing debug/info/warn noise, then run a CI or local go:ci to verify desired log output.backend/app/api/handlers/v1/v1_ctrl_qrcode.go (1)
73-84: Good pattern for streaming endpoints — consider adding debug-level logging.Suppressing
EPIPE/context.Canceled/context.DeadlineExceededafter headers flush is correct: the 200 status and response headers are already sent, so propagating the error would only produce spurious 500 logs and risk downstream middleware attempting to write to a half-streamed response. ✓One minor improvement worth considering:
Debug logging for observability. A genuine
context.DeadlineExceededcaused by slow QR encoding (rather than a client disconnect) will now go undetected. Add a debug-level log beforereturn nilto preserve the "no noisy 500s" property while keeping a breadcrumb for diagnosing performance issues:if isClientDisconnect(err) { log.Debug().Err(err).Msg("qrcode: client disconnected during write") return nil }🔐 Security note: Suppressing write errors on an already-authenticated, streaming endpoint is safe. Just ensure any future logging here does not echo the raw
q.Dataquery string at info level — users sometimes embed internal URLs or object IDs in QR payloads, and these would then land in log aggregation systems.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/handlers/v1/v1_ctrl_qrcode.go` around lines 73 - 84, The handler currently swallows write errors from qrc.Save (written to qrwriter) for EPIPE, context.Canceled, and context.DeadlineExceeded but does not log them; add a debug-level log immediately before returning nil for these client-disconnect/timeout cases (preserving the existing errors.Is checks around syscall.EPIPE, context.Canceled, context.DeadlineExceeded) so you get a breadcrumb for performance/timeout diagnosis without turning them into noisy 500s; ensure the log call uses a debug-level logger (e.g., log.Debug()/logger.Debug()) and does not include any sensitive payload like q.Data.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/app/api/handlers/v1/v1_ctrl_qrcode.go`:
- Around line 73-84: The handler currently swallows write errors from qrc.Save
(written to qrwriter) for EPIPE, context.Canceled, and context.DeadlineExceeded
but does not log them; add a debug-level log immediately before returning nil
for these client-disconnect/timeout cases (preserving the existing errors.Is
checks around syscall.EPIPE, context.Canceled, context.DeadlineExceeded) so you
get a breadcrumb for performance/timeout diagnosis without turning them into
noisy 500s; ensure the log call uses a debug-level logger (e.g.,
log.Debug()/logger.Debug()) and does not include any sensitive payload like
q.Data.
In `@Taskfile.yml`:
- Line 91: Change the HBOX_LOG_LEVEL from "fatal" to "error" so backend error
logs are preserved during CI runs; locate the HBOX_LOG_LEVEL entry (symbol:
HBOX_LOG_LEVEL) in Taskfile.yml and update its value to "error" to keep
error-level logs while still silencing debug/info/warn noise, then run a CI or
local go:ci to verify desired log output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b84685f1-2a20-4cff-b16d-24f7572009e7
📒 Files selected for processing (3)
Taskfile.ymlbackend/app/api/handlers/v1/v1_ctrl_qrcode.gofrontend/pages/reports/label-generator.vue
✅ Files skipped from review due to trivial changes (1)
- frontend/pages/reports/label-generator.vue
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
frontend/test/e2e/items-advanced-fields.spec.ts (2)
77-82:⚠️ Potential issue | 🟡 Minor
fillDatePickerstill silently swallows a missing picker — tests can false-pass.If the selector drifts,
count() === 0triggers a silent early return and the "warranty: expires-on date without lifetime checkbox" case (lines 122-134) degrades to asserting only the defaultLifetime Warranty: Nostate — passing without ever testing the date at all. Fail loudly here.🛡️ Proposed fix — require the picker to be present
async function fillDatePicker(scope: Locator, page: Page, value: string) { const input = scope.locator("input[aria-label='Select Date']").first(); - if ((await input.count()) === 0) return; + await expect(input, "expected a Date picker inside scope").toBeVisible(); await input.fill(value); await page.keyboard.press("Enter"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/test/e2e/items-advanced-fields.spec.ts` around lines 77 - 82, fillDatePicker currently returns silently when the date input is missing, allowing tests to false-pass; update the function (fillDatePicker) to require the picker be present and fail loudly by checking the locator and throwing or asserting if count() === 0 (or using Playwright's expect toBeVisible) before attempting fill/press, so any selector drift surfaces as a test failure rather than an early return.
122-134:⚠️ Potential issue | 🟡 MinorThe warranty-expires test barely verifies what its name promises.
Lifetime Warranty → Nois the default for any freshly-created item, so this test would still pass if the date picker never populated. Add an assertion thatdetailRow(page, "Warranty Expires")contains the rendered date so the test actually covers its stated behavior. Combined with thefillDatePickerfix above, you'll get a meaningful signal when the date UI regresses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/test/e2e/items-advanced-fields.spec.ts` around lines 122 - 134, The test "warranty: expires-on date without lifetime checkbox" currently only asserts "Lifetime Warranty → No" which is the default; update the test (after calling fillDatePicker, saveAndReturn, and reopening the details) to also assert that detailRow(page, "Warranty Expires") contains the date you filled (e.g., "06/20/2026") so the spec verifies the date picker actually persisted and rendered the expiry date; use the existing helpers (fillDatePicker, saveAndReturn, detailRow, cardBySectionTitle) to locate the Warranty Expires row and add an expect(...).toContainText(...) for the expected date string.frontend/test/e2e/items-search-filter.spec.ts (1)
156-156:⚠️ Potential issue | 🟡 MinorDrop the unused
itemTypefrom destructuring across all five tests — CI lint ceiling is 1.
pickEntityTypeIdsreturns{ locType, itemType }, but these tests only uselocType. The bot previously flagged this for a couple of lines; re-flagging here because the same binding is still unused on lines 156, 188, 218, 245, and 278, and the frontend CI enforces a max of 1 lint warning (pnpm run lint:ci). Leaving fiveno-unused-varswill likely blow past that ceiling and red-CI this PR.🧹 Proposed fix — apply at all five sites
- const { locType, itemType } = await pickEntityTypeIds(page); + const { locType } = await pickEntityTypeIds(page);As per coding guidelines: "Frontend code must pass linting with a maximum of 1 warning in CI (
pnpm run lint:ci)".Also applies to: 188-188, 218-218, 245-245, 278-278
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/test/e2e/items-search-filter.spec.ts` at line 156, The tests destructure both locType and itemType from pickEntityTypeIds(page) but never use itemType; remove the unused binding in each affected test by changing the destructuring to only extract locType (i.e., replace occurrences like "const { locType, itemType } = await pickEntityTypeIds(page);" with a single-binding destructure of locType) in the tests that call pickEntityTypeIds so lint no-unused-vars warnings are eliminated across the five sites.frontend/test/e2e/items-attachments.spec.ts (1)
144-184:⚠️ Potential issue | 🟡 Minor"Set primary image" test still advertises a UI flow it doesn't exercise.
The name claims a user-facing "set primary" workflow, but the body bypasses the CheckboxRoot entirely (per the inline comment on lines 160-165) and PUTs
/entities/<id>/attachments/<id>directly. Net effect: a regression in the dialog'supdateAttachmentwiring or checkboxv-modelwould not fail this test, and the underlying reka-ui portalaria-checkedbug is being silently tolerated.Either rename to something honest (e.g. "primary flag round-trips via the API") and file a follow-up for the real UI path behind a
test.fixme, or fix the CheckboxRoot interaction and swap the PUT back out for a click.Want me to open a tracking issue for the reka-ui CheckboxRoot/portal
aria-checkedregression and stub atest.fixmeplaceholder for the click-driven flow?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/test/e2e/items-attachments.spec.ts` around lines 144 - 184, Rename the test "set primary image for a photo attachment" to an honest name like "primary flag round-trips via the API" and update its description to reflect that it exercises the backend PUT on /api/v1/entities/:id/attachments/:id (the code using setupItemAndGotoEdit, uploadAttachment, attachmentRow and the direct page.request.put call). Also add a test.fixme marker and a TODO comment referencing the reka-ui CheckboxRoot/portal aria-checked regression so a follow-up test can be implemented that clicks the attachment-edit checkbox (replacing the direct PUT) once updateAttachment/CheckboxRoot is fixed; optionally open a tracking issue for that bug and reference its ID in the TODO.
🧹 Nitpick comments (3)
frontend/test/e2e/locations-crud.spec.ts (2)
164-164: Minor: tighten accessible-name match for the heading assertion.
getByRole("heading", { name, level: 1 })treatsnameas a substring match. With an 8-char random suffix the collision risk is negligible, but passing{ name, level: 1, exact: true }makes this consistent with the stricter checks used elsewhere in the file (e.g. line 57 uses the same form — so this is really a suggestion to make line 164 match that pattern). Safe to defer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/test/e2e/locations-crud.spec.ts` at line 164, The heading assertion uses page.getByRole("heading", { name, level: 1 }) which performs a substring match; update the call to include exact: true so it becomes page.getByRole("heading", { name, level: 1, exact: true }) to enforce an exact accessible-name match and make it consistent with the stricter checks used elsewhere (see the other getByRole calls).
77-77: Security note (false-positive, but worth addressing): replacenew RegExp(name)with substring assertions.ast-grep is flagging
new RegExp(<variable>)as a potential ReDoS sink on lines 76, 101, 118, and 151. In practice the input comes fromfaker.string.alphanumeric(...).toLowerCase()with a literal prefix, so the character set is[a-z0-9-]and there is no regex-metacharacter injection or catastrophic backtracking risk here. That said, you can dodge the warning and make the assertions more idiomatic by using Playwright's substring form oftoHaveText/toContainText, which accepts a plain string. This also keeps things future-proof if someone later swaps the generator for one that can emit regex metacharacters.♻️ Proposed refactor
- await expect(page.getByTestId("location-detail-name")).toHaveText(new RegExp(name)); + await expect(page.getByTestId("location-detail-name")).toContainText(name); ... - await expect(page.getByTestId("location-detail-name")).toHaveText(new RegExp(name)); + await expect(page.getByTestId("location-detail-name")).toContainText(name); ... - await expect(page.getByTestId("location-detail-name")).toHaveText(new RegExp(originalName)); + await expect(page.getByTestId("location-detail-name")).toContainText(originalName); ... - await expect(page.getByTestId("location-detail-name")).toHaveText(new RegExp(renamedName)); + await expect(page.getByTestId("location-detail-name")).toContainText(renamedName);Based on learnings: Applies to
frontend/test/e2e/**/*.spec.ts: Write E2E tests using Playwright for user workflows and page interactions.Also applies to: 101-102, 118-119, 151-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/test/e2e/locations-crud.spec.ts` at line 77, Replace uses of new RegExp(name) in the Playwright assertions with plain-string substring assertions to avoid constructing regexes from variable input; specifically change calls like await expect(page.getByTestId("location-detail-name")).toHaveText(new RegExp(name)) (and the similar calls around the other occurrences) to use the string form such as await expect(page.getByTestId("location-detail-name")).toContainText(name) or await expect(...).toHaveText(name) so the matcher consumes a plain string instead of a RegExp.frontend/test/e2e/items-search-filter.spec.ts (1)
60-68: Optional: simplifypickEntityTypeIdsto only return what's consumed.No caller in this file uses the
itemTypefield, and computing it viatypes.find(t => !t.isLocation)is wasted cycles. Returning just the location type (or inlining the lookup) keeps the helper honest and kills the unused-destructure smell at the source.♻️ Proposed refactor
-async function pickEntityTypeIds(page: Page) { - const types = await getEntityTypes(page); - const locType = types.find(t => t.isLocation); - if (!locType) { - throw new Error("Expected default location entity type to exist"); - } - const itemType = types.find(t => !t.isLocation); - return { locType, itemType }; -} +async function pickLocationType(page: Page): Promise<EntityTypeSummary> { + const types = await getEntityTypes(page); + const locType = types.find(t => t.isLocation); + if (!locType) { + throw new Error("Expected default location entity type to exist"); + } + return locType; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/test/e2e/items-search-filter.spec.ts` around lines 60 - 68, The helper pickEntityTypeIds currently finds both locType and itemType but callers only use the location type; remove the unused computation by changing pickEntityTypeIds to only locate and return locType (or rename to pickLocationEntityType) and drop the types.find(t => !t.isLocation) logic, then update call sites to destructure the single returned value accordingly so there is no unused itemType.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@frontend/test/e2e/items-advanced-fields.spec.ts`:
- Around line 77-82: fillDatePicker currently returns silently when the date
input is missing, allowing tests to false-pass; update the function
(fillDatePicker) to require the picker be present and fail loudly by checking
the locator and throwing or asserting if count() === 0 (or using Playwright's
expect toBeVisible) before attempting fill/press, so any selector drift surfaces
as a test failure rather than an early return.
- Around line 122-134: The test "warranty: expires-on date without lifetime
checkbox" currently only asserts "Lifetime Warranty → No" which is the default;
update the test (after calling fillDatePicker, saveAndReturn, and reopening the
details) to also assert that detailRow(page, "Warranty Expires") contains the
date you filled (e.g., "06/20/2026") so the spec verifies the date picker
actually persisted and rendered the expiry date; use the existing helpers
(fillDatePicker, saveAndReturn, detailRow, cardBySectionTitle) to locate the
Warranty Expires row and add an expect(...).toContainText(...) for the expected
date string.
In `@frontend/test/e2e/items-attachments.spec.ts`:
- Around line 144-184: Rename the test "set primary image for a photo
attachment" to an honest name like "primary flag round-trips via the API" and
update its description to reflect that it exercises the backend PUT on
/api/v1/entities/:id/attachments/:id (the code using setupItemAndGotoEdit,
uploadAttachment, attachmentRow and the direct page.request.put call). Also add
a test.fixme marker and a TODO comment referencing the reka-ui
CheckboxRoot/portal aria-checked regression so a follow-up test can be
implemented that clicks the attachment-edit checkbox (replacing the direct PUT)
once updateAttachment/CheckboxRoot is fixed; optionally open a tracking issue
for that bug and reference its ID in the TODO.
In `@frontend/test/e2e/items-search-filter.spec.ts`:
- Line 156: The tests destructure both locType and itemType from
pickEntityTypeIds(page) but never use itemType; remove the unused binding in
each affected test by changing the destructuring to only extract locType (i.e.,
replace occurrences like "const { locType, itemType } = await
pickEntityTypeIds(page);" with a single-binding destructure of locType) in the
tests that call pickEntityTypeIds so lint no-unused-vars warnings are eliminated
across the five sites.
---
Nitpick comments:
In `@frontend/test/e2e/items-search-filter.spec.ts`:
- Around line 60-68: The helper pickEntityTypeIds currently finds both locType
and itemType but callers only use the location type; remove the unused
computation by changing pickEntityTypeIds to only locate and return locType (or
rename to pickLocationEntityType) and drop the types.find(t => !t.isLocation)
logic, then update call sites to destructure the single returned value
accordingly so there is no unused itemType.
In `@frontend/test/e2e/locations-crud.spec.ts`:
- Line 164: The heading assertion uses page.getByRole("heading", { name, level:
1 }) which performs a substring match; update the call to include exact: true so
it becomes page.getByRole("heading", { name, level: 1, exact: true }) to enforce
an exact accessible-name match and make it consistent with the stricter checks
used elsewhere (see the other getByRole calls).
- Line 77: Replace uses of new RegExp(name) in the Playwright assertions with
plain-string substring assertions to avoid constructing regexes from variable
input; specifically change calls like await
expect(page.getByTestId("location-detail-name")).toHaveText(new RegExp(name))
(and the similar calls around the other occurrences) to use the string form such
as await expect(page.getByTestId("location-detail-name")).toContainText(name) or
await expect(...).toHaveText(name) so the matcher consumes a plain string
instead of a RegExp.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2a32a4c2-84aa-4102-b481-5228ddab5d34
📒 Files selected for processing (6)
frontend/test/e2e/collection-tools.spec.tsfrontend/test/e2e/home-dashboard.spec.tsfrontend/test/e2e/items-advanced-fields.spec.tsfrontend/test/e2e/items-attachments.spec.tsfrontend/test/e2e/items-search-filter.spec.tsfrontend/test/e2e/locations-crud.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/test/e2e/collection-tools.spec.ts
- frontend/test/e2e/home-dashboard.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a ton of playwright tests across the board to try to add significant automated validation across the 3 major browser engines.
Summary by CodeRabbit
New Features
Bug Fixes
Tests