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

fix: Ignore soft-deleted memberships when querying if user belongs to project #874

Merged
merged 2 commits into from
Oct 12, 2023

Conversation

david-code
Copy link
Contributor

@david-code david-code commented Oct 6, 2023

Description

This seems to be caused by #870 . When I was debugging this, it seems that the problem came from trying to match the fields created by the ManyToManyField (so here the members on membership_list, i.e. collaboration_memberships__membership_list). The results don't seem to take into account the deleted_at field that marks if a membership has been soft deleted, and should be ignored from the query.

For example, when I debug the test, if I retrieve the membership list for the project I'm trying to query, and then do memb_list.members.all(), the membership that has been deleted still shows up. So it seems to not be a problem with the filter itself, but perhaps the django-safedelete configuration of the models.

The fix here is just to make a subquery that actually works. It would be nice to fix the underlying problem with the zombie memberships later too.

Checklist

  • Corresponding issue has been opened
  • New tests added

Related Issues

Fixes #....

Verification steps

@david-code david-code marked this pull request as ready for review October 10, 2023 14:23
@david-code david-code requested review from shrouxm and josebui October 11, 2023 14:11
@david-code david-code merged commit 8b799ff into main Oct 12, 2023
9 checks passed
@david-code david-code deleted the fix/dont-return-deleted-memberships branch October 12, 2023 15:14
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