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

add profile links from other communities #1409

Merged
merged 10 commits into from
Sep 20, 2024

Conversation

cellio
Copy link
Member

@cellio cellio commented Sep 17, 2024

Minimally addresses #284 by adding an "all communities" tab to the user profile to hold links to that user's profile on all other network communities where a profile exists.

Here's a moderator viewing its own tab:

tab with table: community profile link, mod status where applicable, number of posts, rep

And here's how someone else's profile looks:

same table columns, fewer tabs

The guts of the profile tab are separated from the "container" (network.html.erb renders _network.html.erb) to make it easier to pick this up and move it to a truly global place later if we can work out how to do that. (See discussion on the linked issue.) Right now, your network profile is part of your user page on each specific community, just like your inbox is, even though these things are really "higher up".

To support this, I added several "accessor" functions to the user model to go fetch information on specific communities. I followed the pattern of the pre-existing has_ability_on function.

Currently this table shows the number of (undeleted) posts on each community. I was going to separate that out by post type, but post types can be defined in configuration, so that means walking the list of all post types to get their names and counts on each community (some of which might not be enabled on some communities), and I decided to defer that until we decide we need it. If the consensus from user feedback is that we should have that, we can do it as followup work.

This initial implementation does nothing special with sorting, and I believe the order will automatically be the same as the site switcher and the dashboard. This implementation also does not give the user a way to change the sort order. A full implementation would be user-customizable, which requires storing settings somewhere. Also, we need to work out the UI for custom ordering.

This implementation does not yet have any user-private information. I once proposed a design that would show you things like where you have new inbox items or flag responses. I'd still like to do that, but even without it, I think a basic list is better than what we have today. As a moderator I have sometimes needed to look at a user's activity on other communities, and neither URL editing nor navigating the users list is satisfactory.

I learned a lot about Ruby/Rails while doing this and probably failed to learn other things. I suspect there are stylistic anomalies in my code ("no don't do it that way!"), which I hope reviewers will point out so I can fix them.

@cellio cellio requested a review from a team September 17, 2024 20:58
@ws909
Copy link
Contributor

ws909 commented Sep 17, 2024

For bookkeeping, I’d suggest renaming this to «Listing profile information from across the network», since it isn’t actually a network-profile.

Copy link
Member

@Oaphi Oaphi left a comment

Choose a reason for hiding this comment

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

Other than a single redo item (the rest are optional suggestions), LGTM at a quick glance (at least from the initial implementation standpoint)!

app/helpers/users_helper.rb Outdated Show resolved Hide resolved
app/models/user.rb Outdated Show resolved Hide resolved
app/models/user.rb Outdated Show resolved Hide resolved
app/models/user.rb Outdated Show resolved Hide resolved
app/models/user.rb Outdated Show resolved Hide resolved
@cellio
Copy link
Member Author

cellio commented Sep 17, 2024

For bookkeeping, I’d suggest renaming this to «Listing profile information from across the network», since it isn’t actually a network-profile.

When you say "renaming", where do you mean?

@ws909
Copy link
Contributor

ws909 commented Sep 17, 2024

When you say "renaming", where do you mean?

PR title; currently named "basic network profile".

@cellio cellio changed the title basic network profile add profile links from other communities Sep 17, 2024
@cellio
Copy link
Member Author

cellio commented Sep 17, 2024

Other than a single redo item (the rest are optional suggestions), LGTM at a quick glance (at least from the initial implementation standpoint)!

I addressed your comments -- with luck, in the ways you intended. But I see rubocop is unhappy again, so we might have to undo one of those.

Copy link
Member

@Oaphi Oaphi left a comment

Choose a reason for hiding this comment

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

Leaving for others to take a quick look too, but otherwise - LGTM

@@ -24,6 +24,11 @@ def suspended?
false
end

# All undeleted posts on this community by this user.
def post_count
Post.unscoped.where(community_id: community_id).where(user: user).undeleted.count
Copy link
Member

Choose a reason for hiding this comment

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

This is going to be expensive when run for multiple users and/or communities on one page. We're probably alright at current scale, but we should ultimately cache this on the CommunityUser model for each community.

Copy link
Member Author

Choose a reason for hiding this comment

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

It occurs to me that the community-specific post count shows up other places, including on the profile, where we call @user.posts.undeleted.count. But I'm not quite sure how to do that for a non-local user -- any advice? We're calling this from a user on site A and asking about the user on site B, C, D... .

Copy link
Member Author

Choose a reason for hiding this comment

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

Filed #1411 after discussion with @ArtOfCode- .

@cellio cellio merged commit a0922b3 into develop Sep 20, 2024
7 checks passed
@cellio cellio deleted the cellio/284-basic-network-profile branch September 20, 2024 03:19
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.

4 participants