Split deleting own account off to its own path

[yvanzo]

Problem

So far deleting your own account had to be accessed from /admin/user/delete/<editorname> which is the same as for admin deleting someone else account.

Solution

This patch splits account removal into admin-only/non-admin files (thus simplifies implementing MBS-13117) and makes deleting own account accessible through /account/delete instead.

Additionally fix an error page in 930addd.

Checklist for author

• Manually test that
  • “Delete” tab is hidden or shown and links to the URL as expected:
    • non-admin viewing other account: no tab
    • non-admin viewing own account: tab links to /account/delete
    • admin viewing other account: tab links to /admin/user/delete/<username>
    • admin viewing own account: tab links to /account/delete
  • /admin/user/delete/<username> returns a 403 for non-admin
  • /admin/user/delete/<username> still works for admin
  • /account/delete works
• Update CI tests accordingly

Action

1. Rebase MBS-13117: Prevent localizing admin-only messages #3091 after merging

[yvanzo]

Queried locally, with sample data and a logged-in user, /account/delete currently returns:

TypeError: _components_mjs__WEBPACK_IMPORTED_MODULE_5__.default.main/error/404 is not a function
    at getResponse (/musicbrainz-server/root/static/build/server.js:652121:94)
    at Socket.receiveData (/musicbrainz-server/root/static/build/server.js:650353:69)
    at Socket.emit (node:events:514:28)
    at Socket.emit (node:domain:488:12)
    at addChunk (node:internal/streams/readable:376:12)
    at readableAddChunk (node:internal/streams/readable:349:9)
    at Readable.push (node:internal/streams/readable:286:10)
    at Pipe.onStreamRead (node:internal/stream_base_commons:190:23)

Edit: Fixed in 930addd.
Note: Selenium failure is random and unrelated.

[reosarevok]

Tested, mostly works perfectly fine, but it introduces an ISE if the editor is completely empty :)

[reosarevok]

Why not just pass user like the other page does?

[yvanzo]

It shouldn’t be needed. In comparison root/account/ChangePassword.js just tests user before using it.

[reosarevok]

That form tests user because it can also be used without one (for mandatory reset while logged out IIRC) so it has a valid return if user does not exist. This seems a bit different, but I'm not too worried about using the $c one - maybe it should then also be tested though rather than adding a $FlowIgnore.

[yvanzo]

Fair enough. Actually I didn’t know what to return in the opposite case. I just found the solution of throwing an error instead; See https://github.com/metabrainz/musicbrainz-server/compare/392edbe98c29d5f507c227e7fe40000ccced0fb1..66024ca025cc4b308a8b8543ee2ab9b06609acd7.

[mwiencek]

We also have a couple helpers you could use in the future (both throw errors): invariant(user) or const user = expect($c.user). invariant will be auto-imported if you use it, but expect currently has to be imported manually from invariant.js.

[yvanzo]

Thanks for the reminder. I changed my code for using invariant as it allows to return a custom message; See https://github.com/metabrainz/musicbrainz-server/compare/66024ca025cc4b308a8b8543ee2ab9b06609acd7..0ecba92e46bd5a86d0ff62ba94940b7ee8da83fa

I retested it all.

[reosarevok]

Can we lowercase new strings in preparation for #3083 ?

[yvanzo]

Let’s not mix changes :)

[reosarevok]

Ok, I guess whenever we're ready to merge that other PR we can look through all strings again and fix any new ones in a separate commit at the end.

[reosarevok]

Seems good now - I'd still slightly prefer a better solution than a $FlowIgnore but I'm ok with merging it that way if you like that most.

[yvanzo]

Thanks for your review! Account deletion is a danger zone, so I will also wait for @mwiencek's double-review.

[mwiencek]

Tested locally & reviewed the controller bits, looks good to me.