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 deleting channel members and changing role of existing members #495

Merged
merged 26 commits into from
Mar 11, 2022

Conversation

janjagusch
Copy link
Collaborator

@janjagusch janjagusch commented Mar 8, 2022

Closes #494

  • Changed post_channel_member function to work for new and already existing channel members.
  • Added delete_channel_member function and added to REST API.
  • Allow role attribute to be None in rest_models.Member class (this can otherwise cause nasty internal server errors when listing channel members with None as role).
  • Added test cases for the new API functionality.

CI running here. I believe the test failure is unrelated to these changes.

@janjagusch janjagusch changed the title Allow to delete channel members and change role of existing members Allow deleting channel members and changing role of existing members Mar 8, 2022
@janjagusch janjagusch marked this pull request as ready for review March 8, 2022 19:13
quetz/main.py Outdated Show resolved Hide resolved
quetz/main.py Outdated
status_code=status.HTTP_409_CONFLICT,
detail=f"Member {new_member.username} in {channel.name} exists",
status_code=status.HTTP_404_NOT_FOUND,
detail=f"channel member {username}/{channel.name} not found",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be clearer to write "User {username} does not exist or is not a member of {channel.name}" or something along those lines?

Copy link
Collaborator Author

@janjagusch janjagusch Mar 11, 2022

Choose a reason for hiding this comment

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

Sure! I'll go ahead and adjust it. In fact, I can add a separate check to determine whether the user exists.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should be good now

@wolfv wolfv added the enhancement New feature or request label Mar 11, 2022
@wolfv
Copy link
Member

wolfv commented Mar 11, 2022

This looks good to me.

Let me know what you think about my comment.

Are you OK if I squash the commits when merging? Or do you want to clean up the history?

@janjagusch
Copy link
Collaborator Author

This looks good to me.

Great!

Let me know what you think about my comment.

I've incorporated your comment.

Are you OK if I squash the commits when merging? Or do you want to clean up the history?

Totally fine with squashing. Sorry about the commit mess, I think I was debugging against CI at some point.

@wolfv
Copy link
Member

wolfv commented Mar 11, 2022

I'll merge after CI passes!

@janjagusch
Copy link
Collaborator Author

I'll merge after CI passes!

Nice! Apparently, there are some more CI issues, which also seem to occur on master: https://github.com/mamba-org/quetz/runs/5508578696?check_suite_focus=true

Should be unrelated to this PR then, right?

@wolfv wolfv merged commit fe8b7c9 into mamba-org:master Mar 11, 2022
@janjagusch janjagusch mentioned this pull request Mar 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow deleting channel members
2 participants