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.

Sign up for GitHub

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

Jump to bottom

Fix: deleting user that authored some comments #913

Merged
merged 5 commits into from
Feb 21, 2024
Merged

Fix: deleting user that authored some comments #913

merged 5 commits into from
Feb 21, 2024

Conversation

psrok1
Copy link
Member

@psrok1 psrok1 commented Feb 19, 2024

Your checklist for this pull request

  • I've read the contributing guideline.
  • I've tested my changes by building and running the project, and testing changed functionality (if applicable)
  • I've added automated tests for my change (if applicable, optional)
  • I've updated documentation to reflect my change (if applicable)

What is the current behaviour?

  • Deleting user that is an author of object comment results in ISE 500

What is the new behaviour?

  • We can delete users with comments and author field for these comments is just set to NULL.

Test plan

  1. Create user
  2. Log in to that user, upload file, comment it
  3. Log in back to admin account and try to delete a sample

Closing issues

@psrok1 psrok1 added the type:bug Something isn't working label Feb 19, 2024
@psrok1
Copy link
Member Author

psrok1 commented Feb 19, 2024

Hmm I see that author field is missing after serialization instead of being null.

msm-cert
msm-cert reviewed Feb 20, 2024
View reviewed changes
mwdb/schema/comment.py
@@ -18,5 +18,5 @@ class CommentRequestSchema(CommentSchemaBase):

class CommentItemResponseSchema(CommentSchemaBase):
id = fields.Int(required=True, allow_none=False)
author = fields.Str(required=True, allow_none=False, attribute="author_login")
author = fields.Str(required=True, default=None, attribute="author_login")
Copy link
Member

Choose a reason for hiding this comment

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

So we won't remove comments when the user is deleted? How does it look in the mwdb interface?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we don't want to lose information that is potentially valuable.
image

@psrok1 psrok1 requested a review from msm-cert February 20, 2024 13:23
@psrok1 psrok1 merged commit 49b6ae5 into master Feb 21, 2024
12 checks passed
@psrok1 psrok1 deleted the fix/delete-user-with-comments branch February 21, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msm-cert msm-cert msm-cert approved these changes

Assignees
No one assigned
Labels
type:bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@psrok1 @msm-cert