Reuse server strings when possible in stats #3442
Conversation
This adds two special vote types ("Admin approval" and "Admin rejection")
that will be used in a subsequent commit for a feature that allows admins
to immediately accept or reject any edit. These are added first in
a separate commit so that it can be released on production while
the main part of the new feature is released on beta,
avoiding any temporary production ISEs when encountering the new vote types.
"Admin approval" is treated as an approval for icons and headers, while
"Admin rejection" is treated as a No vote. The new vote types are searchable
in edit searches, their stats can be seen on admin profiles, etc.
There's plenty of cases where a removal of a spam release for example sits open for days, and closing it earlier requires asking for more eyes on it which does seem like the opposite of what you want to do with spam. This allows account admins to use the same accept/reject mechanism we already used in test servers to immediately accept or reject any open edit (including their own). When doing so, a vote is added to the edit (of the newly added types "Admin approval" or "Admin reject", added in a previous commit so they can be visible in production), so it is as clear as possible why the edit applied or failed. I kept the mechanisms separate, still keeping a /test/ endpoint for the existing testing mechanism while adding a new /admin/ one for the new feature. This is mostly for testing purposes (it's very hard to test for the feature otherwise since the test setup has the buttons on by default because of its DB_STAGING_TESTING_FEATURES setting) but also to avoid setting the new votes every time we use the feature in testing, since that might not be desirable.
We seemingly were never hitting this, because as soon as we did during my testing, it died horribly. Unsurprisingly, since it was trying to access $_ outside the any call. Unsure how useful this is given we never hit it, but for now, this at least actually should do what the old code tried to.
We were making people translate stuff twice a lot more than strictly needed, and mwiencek suggested we don't. I'm sure I've missed some reused strings, but this is most of them.
|
We actually have a test ensuring we don't do this, so I'm assuming it is intentional? see https://app.circleci.com/pipelines/github/metabrainz/musicbrainz-server/12638/workflows/b2485960-7220-46b2-9416-38fecdf44aa9/jobs/20969 @yvanzo, can you remind us why? |
|
See #3139 and #3141, particularly #3141 (comment). I think the gist is:
I'm not qualified to speak to the second point, though. |
|
Ah, ok. Well, I do feel that there's some cases where this makes sense, but I guess if we wanted to do it it could be made more explicit (something like aliasing it to |
|
Adding an explicit |
Description
We were making people translate stuff twice a lot more than strictly needed, and @mwiencek suggested we don't. I'm sure I've missed some reused strings, but this is most of them.
On top of #3392 because that's where this started and this has strings from there.