fix(e2e): target the nav settings entry by test id, not by translated label - #540
Merged
Merged
Conversation
… label
With the support-dialog fix in, due-date-reminder-settings.spec.ts stopped
failing on an intercepted click and started failing on a locator that resolves
to nothing:
waiting for locator('#app-navigation-vue').locator('a[title="Settings"]')
There is no such element. CnAppNav auto-prepends the entry that opens the app's
NcAppSettingsDialog and names it "Personal settings", and that name is
translated, so a title selector is a language assertion this suite has no
business making. It is the same defect class as the vault unlock button, which
matched /^(Unlock|Set up vault)$/ and found nothing on a Dutch instance.
The library carries stable ids for exactly this: data-testid="cn-nav-settings"
on the foldout and data-testid="cn-nav-personal-settings" on the entry. Both
are present in the installed bundle (2.27.2), verified rather than assumed.
🔴 The click targets the <a> INSIDE the entry, not the test id itself.
data-testid lands on NcAppNavigationItem's ROOT, which is an <li>; clicking
that is a silent no-op and the failure then surfaces several lines later on
whatever the dialog was supposed to show.
planninq declares no section:"settings" menu entries, so the foldout exists
solely because includePersonalSettings defaults true. That is why the entry is
the auto-prepended one rather than an app-declared item.
Contributor
Quality Report — ConductionNL/planninq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 536/536 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-01 05:14 UTC
Download the full PDF report from the workflow artifacts.
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
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.
With the support-dialog fix (#536) in,
due-date-reminder-settings.spec.tsstopped failing on an intercepted click and started failing on a locator that resolves to nothing:There is no such element.
CnAppNavauto-prepends the entry that opens the app'sNcAppSettingsDialogand names it "Personal settings" — and that name is translated, so a title selector is a language assertion this suite has no business making.Same defect class as the vault unlock button earlier today, which matched
/^(Unlock|Set up vault)$/and found nothing on a Dutch instance.The fix
The library carries stable ids for exactly this:
cn-nav-settingscn-nav-personal-settingsBoth verified present in the installed bundle (2.27.2), not just library source.
The click targets the anchor inside, not the test id
data-testidlands onNcAppNavigationItem's root, which is an<li>. Clicking that is a silent no-op, and the failure then surfaces several lines later on whatever the dialog was supposed to show — accusing the dialog rather than the click.Why the entry is the auto-prepended one
planninq declares no
section: "settings"menu entries (humaniq has 2, dossiq has 9). Its foldout exists solely becauseincludePersonalSettingsdefaults true, so the only thing in it is the auto-prepended entry.That is worth knowing separately: if planninq is meant to have app-declared settings entries, they are missing. This PR does not add them — it makes the test target what the app actually renders.