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

feat(user): fully support the Visible Role field #3087

Merged
merged 7 commits into from
Jan 23, 2025

Conversation

wescopeland
Copy link
Member

@wescopeland wescopeland commented Jan 21, 2025

This PR adds full support for the Visible Role field in a user's settings. A migration has been included which adds a UserAccounts.visible_role_id field.

How it works:
Given a user has 2 or more roles attached, they may select which one is publicly visible from their settings:
Screenshot 2025-01-20 at 7 57 23 PM

Once a visible_role_id is set, it is respected on the user profile, forum author box, and user card tooltips:
Screenshot 2025-01-20 at 7 57 35 PM

Screenshot 2025-01-20 at 7 57 45 PM

Screenshot 2025-01-20 at 7 57 58 PM

Implementation details:
The meat of this PR is ActsAsCommunityMember::getVisibleRoleAttribute().

visible_role_id is nullable. By default, getVisibleRoleAttribute() is going to return the user's first display-ordered role. For the great majority of folks who have roles, this will either be their Developer or Junior Developer roles.

If the user has a defined visible_role_id value, but they've since lost the role, then the attribute falls back to the first display-ordered role they have attached, or just null.

We'll obviously need to do some kind of mass assignment of team roles after deploy. I can figure out those details later.

@wescopeland wescopeland added the deployment/sql Includes SQL that needs to be run before/after deployment label Jan 21, 2025
@wescopeland wescopeland requested a review from a team January 21, 2025 01:03
@Jamiras
Copy link
Member

Jamiras commented Jan 22, 2025

I also believe we should set the Administrator role to visible

I'm not convinced of that. How would we describe the Administrator role to users? Administrator is similar to Root. It's a super user with access to pretty much anything, while not really being a News Manager, or Event Manager, etc.

Conversely, my roles right now are all hidden except Developer. People know I have more power than that, but Administrator is more of a Title than a Role.

As a user, when would I want to message an Administrator over someone with a more dedicated role?

@wescopeland
Copy link
Member Author

Good point - probably never.

I'm pretty sure each admin should have more specific roles under the current role offerings we have. For example, you and I (and anyone else with write access to our GitHub org) should probably have the "Engineer" role.

@wescopeland wescopeland removed the deployment/sql Includes SQL that needs to be run before/after deployment label Jan 22, 2025
@wescopeland wescopeland merged commit ea7a001 into RetroAchievements:master Jan 23, 2025
8 checks passed
@wescopeland wescopeland deleted the user-visible-roles branch January 23, 2025 00:47
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