Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MBS-13896 / MBS-13897: User type link improvements #3446

Merged
merged 4 commits into from
Jan 10, 2025

Conversation

reosarevok
Copy link
Member

Fix MBS-13896 / Implement MBS-13897

Description

This fixes anchor links to editor types from editor pages (such as /user/reosarevok), which had broken when @Aerozol changed the headers on the doc from plural to singular. It also adds links to missing (now documented) types, "Bot", "Beginner" and "Account admin" (the last of which wasn't even indicated on the page yet for some reason, but really should be).

Testing

Manually, by checking my own page (all privs except beginner and bot) and creating a new account locally (beginner) and setting it as a bot.

The doc page was reorganized and improved, but that broke the links,
since headers were changed to singular.
There's no real reason why we wouldn't link to the docs from here.
There's no real reason why we wouldn't link to the docs from here.
I can't see any good reason why we don't already show this, which will
be even more relevant once admins are allowed to approve or reject
all edits (MBS-13770).
@reosarevok reosarevok added QoL Non-urgent quality of life improvements Bug Bugs that should be checked/fixed soonish labels Jan 9, 2025
@Aerozol
Copy link
Contributor

Aerozol commented Jan 9, 2025

THANKS AND SORRY XO

Copy link
Member

@mwiencek mwiencek left a comment

Choose a reason for hiding this comment

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

Tested locally, works for me.

if (isAccountAdmin(user)) {
typesList.push(
<a href="/doc/Editor#Account_admin" key="account-admin">
{lp('Account admin', 'user type')}
Copy link
Member

@mwiencek mwiencek Jan 10, 2025

Choose a reason for hiding this comment

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

Just curious, are these [privileges] listed in any particular order? They don't seem to be sorted by power nor alphabetically. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no idea why the existing order was what it is - my reasoning on the sorting of account admin was "auto-editor is the most relevant for a day-to-day user checking that, and after that account admin seems the most important due to the power it involves so I'll put it second" 🤷‍♂️

@reosarevok reosarevok merged commit 582f614 into metabrainz:master Jan 10, 2025
2 checks passed
@reosarevok reosarevok deleted the MBS-13896 branch January 10, 2025 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bugs that should be checked/fixed soonish QoL Non-urgent quality of life improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants