Skip to content

msw: continue handler migration - #1186

Open
deer-wmde wants to merge 23 commits into
mainfrom
de/msw-continue
Open

msw: continue handler migration#1186
deer-wmde wants to merge 23 commits into
mainfrom
de/msw-continue

Conversation

@deer-wmde

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

Copy link
Copy Markdown
Contributor

Note

needs a merge & rebase of #1184

  • 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

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.

2 participants