Skip to content

msw: continue handler migration - #1186

Merged
rosalieper merged 15 commits into
mainfrom
de/msw-continue
Sep 15, 2026
Merged

rosalieper merged 15 commits into
mainfrom
de/msw-continue

Conversation

@deer-wmde

@deer-wmde deer-wmde commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor
  • sets onUnhandledRequest: 'error' so that not-implemented handlers error instead of bypass msw (docs)
  • migrates remaining existing handlers + some policy ones needed for interaction
    • POST /api/wiki/entityImport
    • GET /api/wiki/entityImport
    • POST /api/wiki/create
    • POST /api/wiki/delete
    • POST /api/wiki/logo/update
    • POST /\/api\/wiki\/setting\/.*?\/update$/
    • POST /api/wiki/details
    • GET /api/wiki
    • GET /api/v1/policies/missing
    • GET /api/v1/policies/current
  • fixes removeWiki: splice() mutates, not copies (docs)
  • fixes user login handle, not always test@local anymore
    • do we actually want this or are we now relying on it? :D

https://phabricator.wikimedia.org/T436530

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

Deployment previews on netlify for branch refs/pull/1186/merge will be at the following locations (when build is done):

@deer-wmde
deer-wmde marked this pull request as ready for review September 3, 2026 15:50

@dati18 dati18 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a singleton state is created at import time (including myWikis, lastWikiId, user and getEntityImportCalledTimes). For me it's a bit concerning because the state is glocal and not reset between tests or re-renderings, and a test that creates/deletes wikis can affect later tests.
I think a getter/setter pattern is more suitable than module-level constants. For example:

  • const getMyWikis = () => { //some code }
  • const setMyWiki = (items) => { //some code }
    And recompute lastWikiId from the current array when needed, rather than keeping one stale global value.

Not a blocker for a trivial mock, but I would treat it as a test-flakiness issue. If this file is used across multiple UI tests, it might cause some order-dependent failures.

@deer-wmde

Copy link
Copy Markdown
Contributor Author

a singleton state is created at import time (including myWikis, lastWikiId, user and getEntityImportCalledTimes). For me it's a bit concerning because the state is glocal and not reset between tests or re-renderings, and a test that creates/deletes wikis can affect later tests. I think a getter/setter pattern is more suitable than module-level constants. For example:

* `const getMyWikis = () => { //some code }`

* `const setMyWiki = (items) => { //some code }`
  And recompute `lastWikiId` from the current array when needed, rather than keeping one stale global value.

Not a blocker for a trivial mock, but I would treat it as a test-flakiness issue. If this file is used across multiple UI tests, it might cause some order-dependent failures.

I agree for unit tests this logic is not suitable. Using a (shared) logic for unit tests I think should come after these initial fixes. As mentioned in the other PR, the scope for these was primarily the migration and touchup of the old broken mock service

dati18
dati18 previously requested changes Sep 10, 2026
Comment thread src/backend/mocks/default_handlers.js
Comment thread src/backend/mocks/default_handlers.js
Comment thread src/backend/mocks/default_handlers.js

@rosalieper rosalieper left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We(Tom Dat and I) agreed in the Daily that this could me merged and deploy as it is good enough as it is and @dati18 concerns will be addressed in a follow up ticket.

@rosalieper
rosalieper dismissed dati18’s stale review September 15, 2026 14:44

see comment added to the approve

@rosalieper
rosalieper merged commit 8a119c3 into main Sep 15, 2026
7 checks passed
@rosalieper
rosalieper deleted the de/msw-continue branch September 15, 2026 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants