test: [DHIS2-22025] Cypress tests for working list view sharing on update - #4706
Open
karolinelien wants to merge 17 commits into
Open
test: [DHIS2-22025] Cypress tests for working list view sharing on update#4706karolinelien wants to merge 17 commits into
karolinelien wants to merge 17 commits into
Conversation
…user from paylaod
…pdate The existing scenario "Ensure sharing settings are preserved when updating a working list" passed on broken code and has done so for over a year: it asked the sharing dialog whether the access survived, and the dialog answers from stale client state. Verified against 2.43.1 by instrumenting the run — the app PUT returned 200, GET /api/sharing reported userAccesses: [] immediately after, and the scenario still went green. It is green on master's own push CI today (run 32000477547) while the bug reproduces on 2.41 through 2.44. Replaces it with a dedicated area covering all three affected list types: - assertions read sharing from the server, never from the dialog, and check publicAccess as well as the user entry (the update resets public access too) - owner scenarios grant access through the real Share view dialog, covering the dialog -> redux -> payload wiring - non-owner scenarios (@user:trackerAutoTestRestricted) seed the view by API as the owner, so the sharing comes from the templates GET instead, and assert the update is not rejected with 409 (DHIS2-13020) and that Update view is offered at all, which was that issue's original symptom - the non-owner seeding asserts the view is not owned by the logged-in user, so the scenario fails loudly rather than passing vacuously if the seeding were ever to run as the wrong user - teardown by API in an After hook, plus a sweep before each scenario, so a failed or killed run does not leave views behind AI Assisted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI on the dev instance failed that one scenario on a missing table header, four attempts, while the other four passed. Cause is access, not the test: Malaria case registration is shared as public rw------, which is metadata only, and trackerAutoTestRestricted belongs to no user groups, so it sees no events there and the event list renders no sortable table. The owner scenario keeps Malaria case registration. The non-owner one moves to XX MAL RDT - Case Registration, which is public rwrw---- and assigned to Ngelehun on both 2.43 and dev, so that user can read its events. Same resource and same update code path, so the coverage is unchanged. AI Assisted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first CI run reported missing access as "Expected to find element dhis2-uicore-tableheadercellaction, but never found it", which took a round trip plus reading the program's sharing to translate into "that account cannot see these events". The seeding step now checks the program up front, as the logged-in user, and fails with what is actually wrong, e.g. "trackerAutoTestRestricted can read the events of Malaria case registration: expected false to equal true". Metadata read is required for every list type. Data read is additionally required for the event list, which renders its table only once events resolve, while the tracker list builds its columns from the template — asserting data read there would fail scenarios that legitimately pass today. Both checks read the session, so they run before any request carrying basic auth, which would otherwise replace the session cookie with the owner's. AI Assisted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
karolinelien
marked this pull request as ready for review
August 20, 2026 11:10
karolinelien
marked this pull request as draft
August 20, 2026 11:10
karolinelien
marked this pull request as ready for review
August 20, 2026 13:16
henrikmv
changed the base branch from
master
to
hv/fix/DHIS2-13020_error-on-update-view-when-view-edit-access
August 20, 2026 16:00
Base automatically changed from
hv/fix/DHIS2-13020_error-on-update-view-when-view-edit-access
to
master
August 21, 2026 07:49
henrikmv
changed the title
- seed program stage working lists with a program stage (ANTENATAL_STAGE_ID), which that resource requires on create, added via a spread so the other list types are untouched - add the sixth scenario: non-owner update of a program stage view - drop basic auth entirely in favour of switching session, so every request runs as a real logged-in user: switchToOwner/switchToRestrictedUser, teardown switches to the owner first, and deleteViewsNamed and assertStillSharedWith run against the current session - scope the Share view interactions to [data-test="sharing-dialog"] Two deviations from the review, both forced by what the runtime does: Session switching goes through loginUser, now exported from support/tagUtils/login.js, instead of a local cy.session call with the same key format. A session id must be declared with the same setup and validate function every time, so redefining `user<name>` in the step file fails with "this session already exists" as soon as a scenario in the same spec has used the harness login. Exporting the helper gives one definition per user for the whole suite, which is what the review was after. The user search results and the access-level menu render in poppers outside the dialog element, so those two clicks cannot be scoped to it: scoping them fails with "Expected to find content 'English Bombali' within <aside>". Everything else in that step is scoped. Also reordered the owner scenarios so the "no longer has unsaved changes" UI check runs before the server assertion: assertStillSharedWith now switches session, and cy.session clears the page, which would leave nothing to inspect. Verified against play stable-2-43-1 with a second real account: all six pass. AI Assisted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The master merge kept the older POT-Creation-Date, which left a two-line timestamp diff in a test-only PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaced three constructs that appeared nowhere else in cypress/:
- cross-step state via cy.wrap(value).as(alias): the suite uses .as() only for
intercept aliases and otherwise re-derives what it needs, so the list type is
now a step-text parameter (event|tracker|program stage) and the view is looked
up by name, the way cleanUpWorkingListIfApplicable does. That also removed the
viewId, myId, otherUserId and listType aliases, and collapsed the two
"still shared with" steps into one
- failOnStatusCode: false: teardown now uses cy.request('DELETE', url), the same
call the other cleanup helpers make
- a $body.find() conditional to detect an open menu: the menu is closed again
with a forced click, and .click({ force: true }) is used throughout the suite
Kept, with reasons:
- expect(value, 'message'): the same chai call the suite already makes, with a
message. The message is the whole point of the precondition assertion — it
names the account and the program instead of failing as an opaque timeout
- the LIST_TYPES table, session switching through loginUser, and the
...(programStageId && ...) spread all came from the previous review round
Verified against play stable-2-43-1 with a second real account: all six pass.
AI Assisted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A view cannot be updated without the server accepting it, so a separate "Then the update is accepted by the server" step described the same fact twice and read as optional. The status assertion now sits in the update step itself, where the intercept is already awaited, and the DHIS2-13020 note about the 409 moved with it. Six Gherkin lines removed, coverage unchanged. Verified against play stable-2-43-1 with a second real account: all six pass. AI Assisted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
henrikmv
marked this pull request as draft
August 25, 2026 07:35
henrikmv
added
e2e-tests
e2e record
henrikmv
added
e2e record
henrikmv
added
e2e-tests
and removed
e2e-tests
e2e record
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
henrikmv
marked this pull request as ready for review
August 27, 2026 14:21
Contributor
|
@karolinelien Great initiative and a good draft created. Thank you! I made some cleanup now and put the PR ready for review. Well done improving our testing! |
henrikmv
changed the title
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
2 participants
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.
DHIS2-22025
Cypress test for DHIS2-21871 and DHIS2-13020
Replaces a broken Cypress test for working list view sharing with a new, comprehensive test suite under
WorkingListsSharing/.Why the old test was broken: it asserted sharing via the Share dialog UI, which renders stale client state. After a
PUTthat silently droppeduserAccesses, the dialog still showed the old access — so the test passed on broken code.What changes:
"Ensure sharing settings are preserved"scenario and its step definitions are deleted fromEventWorkingListsUser.WorkingListsSharingsuite adds 6 scenarios covering event, tracker, and program-stage working lists, in both owner and non-owner roles.GET /api/<resource>/<id>?fields=sharing) instead of the dialog, checking bothuserAccessesandpublicAccess.@user:trackerAutoTestRestricted. AnAfterhook + pre-scenario sweep ensures cleanup even on failure.