Skip to content

Added Show users on datasets/scan reports - #1165

Merged
brian-kim31 merged 7 commits into
masterfrom
show_users
Aug 1, 2025
Merged

Added Show users on datasets/scan reports#1165
brian-kim31 merged 7 commits into
masterfrom
show_users

Conversation

@brian-kim31

@brian-kim31 brian-kim31 commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

✨ Feature

PR Description

This PR implements member display functionality across datasets and scan reports by creating dedicated API functions (getDatasetMembers() and getScanReportMembers()) that aggregate and deduplicate users from multiple permission levels (viewers, editors, and admins for datasets; viewers and editors for scan reports). The implementation integrates with existing layout components to show all users with access to each resource using the AvatarList component, which displays member avatars with tooltips and a count indicator. This provides users with immediate visibility into who has access to datasets and scan reports, improving collaboration and access management while maintaining the existing permission structure and ensuring backward compatibility.

Related Issues or other material

Related #1073
Closes #1073

Screenshots, example outputs/behaviour etc.

Screenshot 2025-08-01 at 12 35 45 Screenshot 2025-08-01 at 12 35 45

@brian-kim31
brian-kim31 marked this pull request as ready for review July 30, 2025 11:55
@brian-kim31 brian-kim31 self-assigned this Jul 30, 2025
@AndyRae

AndyRae commented Jul 30, 2025

Copy link
Copy Markdown
Member

I can see why this looks like a small issue to tackle, and why you'd take this approach given the existing codebase and how it fetches / resolves this pattern generally. But this is really an iceberg issue - looks small, but there's a bunch of debt it's building on.

The ideal pattern for this is to mirror the existing Projects type / API, when fetching a Project the API contains the list of members (as user objects), this makes it easy for the frontend to render it.

Unfortunately - the pattern is not the case for Datasets and `Scan Reports, where as you've seen only serialize the user ids on their endpoints, so therefore you need to go and fetch the users again, and we're back to adding more code on top of a bad pattern.

So what do to?

  1. Merge this (layer on top of the bad pattern and make it harder to fix in future)
  2. Fix the pattern and rework this slightly
  3. Leave it for now - this is a P3 issue and we have bigger fish to fry! This is my current thinking but happy for thoughts @AndrewThien @brian-kim31

Comment thread app/next-client-app/app/(protected)/datasets/[id]/layout.tsx Outdated
@brian-kim31
brian-kim31 requested a review from AndyRae August 1, 2025 11:37
AndyRae
AndyRae previously approved these changes Aug 1, 2025
@brian-kim31
brian-kim31 merged commit 44b30bc into master Aug 1, 2025
16 checks passed
@brian-kim31
brian-kim31 deleted the show_users branch August 1, 2025 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show users on datasets/scan reports

2 participants