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

confirm group membership ending #767

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

rouven0
Copy link
Contributor

@rouven0 rouven0 commented Dec 2, 2024

Currently, when you click on the 'End membership' button on a group (intended or accidentally), the group membership is ended without any confirmation, leading to possible unwanted situations.

This PR implements a simple confirmation form before performing the actual ending operation.

Copy link

codecov bot commented Dec 2, 2024

Codecov Report

Attention: Patch coverage is 25.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 87.69%. Comparing base (c44f543) to head (648b959).

Files with missing lines Patch % Lines
web/blueprints/user/__init__.py 25.00% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #767      +/-   ##
===========================================
+ Coverage    87.61%   87.69%   +0.07%     
===========================================
  Files          277      275       -2     
  Lines        17115    16954     -161     
===========================================
- Hits         14995    14867     -128     
+ Misses        2120     2087      -33     

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

@rouven0 rouven0 requested a review from lukasjuhrich December 3, 2024 17:48
Comment on lines +566 to 568
@bp.route("/<int:user_id>/end_membership/<int:membership_id>", methods=["GET", "POST"])
@access.require("groups_change_membership")
def end_membership(user_id: int, membership_id: int) -> ResponseReturnValue:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add frontend tests for this.
This is not hard, just look at the existing ones.
Hit me up if you have any questions!

Copy link
Collaborator

@lukasjuhrich lukasjuhrich left a comment

Choose a reason for hiding this comment

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

Great addition!

Did not test it live, but the changes are trivial enough that frontend tests should verify the correct behavior.

@rouven0 rouven0 force-pushed the confirm_member_ending branch 2 times, most recently from 1684e52 to 03c1b8e Compare December 10, 2024 22:36
@rouven0 rouven0 force-pushed the confirm_member_ending branch from 03c1b8e to 9b6316b Compare December 10, 2024 22:38
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