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

Allow project users to manage collections #2898

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

midigofrank
Copy link
Collaborator

Description

This PR allows project users to view, create, edit and delete collections.
Only users with edit access can create, edit and delete collections

This PR also removes the interface from the superadmin side

Closes #2838

Validation steps

  1. Login to the application as a project editor, admin or owner
  2. Go to the project settings page, you'll notice a collections tab just after credentials
  3. Try creating, editing and deleting the collections

When you login as a viewer, all the action buttons are greyed out

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our Responsible AI Policy

Pre-submission checklist

  • I have performed a self-review of my code.
  • I have implemented and tested all related authorization policies. (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

@midigofrank midigofrank self-assigned this Feb 4, 2025
Copy link

codecov bot commented Feb 4, 2025

Codecov Report

Attention: Patch coverage is 97.54098% with 3 lines in your changes missing coverage. Please review.

Project coverage is 91.47%. Comparing base (bb5d716) to head (75708bb).

Files with missing lines Patch % Lines
...ing_web/live/project_live/collections_component.ex 97.34% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2898      +/-   ##
==========================================
+ Coverage   91.45%   91.47%   +0.01%     
==========================================
  Files         343      341       -2     
  Lines       12371    12360      -11     
==========================================
- Hits        11314    11306       -8     
+ Misses       1057     1054       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@midigofrank midigofrank marked this pull request as ready for review February 4, 2025 08:35
Copy link
Member

@jyeshe jyeshe left a comment

Choose a reason for hiding this comment

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

Nicely done Frank! I have left one request that would help users on knowing the naming requirements to prevent a try-and-error repetition situation.

defp validate_changeset(changeset) do
changeset
|> validate_format(:name, ~r/^[a-z0-9]+([\-_.][a-z0-9]+)*$/,
message: "Collection name must be URL safe"
Copy link
Member

Choose a reason for hiding this comment

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

It would be helpful for the users to know the exact rule. Something like only digits, letters and the separating symbols '-', '_' or '.'

Comment on lines +5212 to +5213
for {conn, _user} <-
setup_project_users(conn, project, [:owner, :admin, :editor]) do
Copy link
Member

@jyeshe jyeshe Feb 4, 2025

Choose a reason for hiding this comment

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

This is a nice coverage but would you mind moving to a separate test case for the different groups of users? One test, the current one would be left for the viewers, and another one would be created for different expected result contemplating this second group of users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

Enable project users to manage collections
2 participants