-
Notifications
You must be signed in to change notification settings - Fork 42
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
bug
Delete Chat Replies on Delete
#1064
Conversation
Your Testserver will be ready at https://1064.test.live.mm.rbg.tum.de in a few minutes. Logins
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked locally with DataGrip, looks great! 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Do we ever need to delete a chat without deleting it's replies? If not we can also merge these methods into one.
Probably not. Good idea! 👍🏻 |
Please ignore the |
dao/chat.go
Outdated
return errors.Join( | ||
DB.Model(&model.Chat{}).Delete(&model.Chat{}, id).Error, | ||
DB.Delete(&model.Chat{}, "reply_to = ?", id).Error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems a little cleaner
return errors.Join( | |
DB.Model(&model.Chat{}).Delete(&model.Chat{}, id).Error, | |
DB.Delete(&model.Chat{}, "reply_to = ?", id).Error) | |
return DB.Where("id = ? OR reply_to = ?", id, id).Delete(&model.Chat{}).Error |
# Conflicts: # go.work.sum
* Add DeleteReplies * Fix tests * Merge functions * Update dao function
Motivation and Context
Currently if a message with replies is deleted only the "base" message is deleted, i.e. replies are kept in the database. In other words
deleted_at
is only not-null for base messages. As a consequence, each time we fetch messages deleted "ghost" replies are transmitted also.This PR fixes this issue.
Description
Add
DeleteReplies
toChatDao
and call in delete handler.Steps for Testing