msw: continue handler migration

[deer-wmde]

• 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]

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

• UI with mock store: https://c58112324a-mock--wbstack-ui.netlify.app/
• UI pointing to prod: https://c58112324a-prod--wbstack-ui.netlify.app/

[dati18]

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]

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]

I think you need to register a handler for /api/user/register, the frontend calls that endpoint in api.js:19
The frontend expects registration to work, the backend has a route for it, but the mock leaves it out

[dati18]

I'm still a bit confused by this part. The mock’s GET /api/auth/login handler can reset the user with a fresh user by calling makeUser().
Then POST /api/auth/login uses the submitted email to create the session user in line 146, but the next auth check at line 134 throws that away and recreates the default user?

[dati18]

I already mentioned it in this comment

• The mock is a bit odd because it returns a string, api.js returns an object
• the frontend expects a different message text than the backend actually sends (a "!" difference)