-
Notifications
You must be signed in to change notification settings - Fork 3
baseapp-chats [BA-2213]: delete messages #241
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
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
6554870 to
4ba2409
Compare
|
|
||
|
|
||
| class ChatRoomDeleteMessage(RelayMutation): | ||
| deleted_message = graphene.Field(MessageObjectType._meta.connection.Edge) |
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.
There is no need to return the deleted_message, you are required to return only its relay ID to make relay in the frontend remove the message from the store. This follows better the relay/graphql pattern.
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.
But we're doing a soft delete, so this is more like a message update than an actual deletion. We're just updating the deleted flag so we can replace the content of the message with the default deletion copy.
4ba2409 to
d68d5d3
Compare
| ) | ||
|
|
||
| if not info.context.user.has_perm( | ||
| "baseapp_chats.change_message", |
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 should be "baseapp_chats.delete_message", right?
tsl-ps2
left a comment
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.
Left one last minor comment which should take only a few seconds to fix.
d68d5d3 to
273cf7c
Compare
88287af to
537235b
Compare
537235b to
3d562ca
Compare
|


Acceptance Criteria
Guardrail
The snackbar will not have the action button for now
Design Link: https://www.figma.com/design/XRD6wSl1m8Kz6XUcAy5CLp/BaseApp---WEB?node-id=7763-127162&t=MOMbprMUFediIBZh-0
Note: The snackbar has been implemented in another PR
silverlogic/baseapp-frontend#207