Skip to content

feat: implement delete message feature in chat room #573

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

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

feat: implement delete message feature in chat room #573

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d5b69f3
feat: implement delete message feature in chat room
4555jan Oct 21, 2025
408686e
Restrict delete dialog to message sender only
4555jan Oct 25, 2025
82dfb67
Add delete message tests and removed notifications in deleted message…
4555jan Oct 26, 2025
c665b86
mistakenly changed existing code
4555jan Oct 26, 2025
1cc4c55
Restrict message editing to once and prevent editing/deleting of dele…
4555jan Oct 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions lib/controllers/room_chat_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,42 @@ class RoomChatController extends GetxController {
log(appwriteUpcommingRoom.toString());
}

/// delete method
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember to add tests for this function

Future<void> deleteMessage(String messageId) async {
try {
Message messageToDelete = messages.firstWhere(
(msg) => msg.messageId == messageId,
);
messageToDelete = messageToDelete.copyWith(content: '', isdeleted: true);

await databases.updateDocument(
databaseId: masterDatabaseId,
collectionId: chatMessagesCollectionId,
documentId: messageId,
data: messageToDelete.toJsonForUpload(),
);
if (appwriteUpcommingRoom != null) {
log('Sending notification for deleted message');
var body = json.encode({
'roomId': appwriteUpcommingRoom?.id,
'payload': {
'title': 'Message Deleted in ${appwriteUpcommingRoom?.name}',
'body': '${messageToDelete.creatorName} deleted a message',
},
});
var results = await functions.createExecution(
functionId: sendMessageNotificationFunctionID,
body: body.toString(),
);
log(results.status);
}
log('Message deleted successfully');
} catch (e) {
log('Error deleting message: $e');
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to send notifications on deleted messages, we can remove tis

}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Harden delete path: enforce ownership, handle not-found

Prevent deleting others’ messages client-side (in addition to backend ACLs) and avoid firstWhere throw.

   /// delete method
   Future<void> deleteMessage(String messageId) async {
     try {
-      Message messageToDelete = messages.firstWhere(
-        (msg) => msg.messageId == messageId,
-      );
-      messageToDelete = messageToDelete.copyWith(content: '', isdeleted: true);
+      final idx = messages.indexWhere((m) => m.messageId == messageId);
+      if (idx == -1) {
+        log('Message not found: $messageId');
+        return;
+      }
+      var messageToDelete = messages[idx];
+      if (messageToDelete.creatorId != auth.appwriteUser.$id) {
+        log('Forbidden: cannot delete others\' messages');
+        return;
+      }
+      messageToDelete =
+          messageToDelete.copyWith(content: '', isdeleted: true);
@@
-        data: messageToDelete.toJsonForUpload(),
+        data: {
+          // Update only the fields that change
+          'content': '',
+          'isdeleted': true,
+        },
       );

Confirm backend rules also restrict updates to message owner to avoid client bypass.

I'll verify the review comment by examining the codebase for the relevant concerns.


---

🏁 Script executed:

```shell
#!/bin/bash
# Find Message model definition
echo "=== Searching for Message model definition ==="
fd -e dart | xargs rg -n "class Message" -A 10 | head -50

Length of output: 549

🏁 Script executed:

#!/bin/bash
# Search for auth object and appwriteUser usage
echo "=== Searching for auth object and appwriteUser ==="
rg -n "auth\.appwriteUser" lib/controllers/ -C 2

Length of output: 496

🏁 Script executed:

#!/bin/bash
# Check the Message model fields, especially creatorId and creatorName
echo "=== Searching for creatorId field in Message ==="
rg -n "creatorId|creatorName" lib/ -C 1 | head -60

Length of output: 3646

🏁 Script executed:

#!/bin/bash
# Look for toJsonForUpload method to understand what data is being sent
echo "=== Searching for toJsonForUpload implementation ==="
rg -n "toJsonForUpload" lib/ -A 5 | head -50

Length of output: 1870

🏁 Script executed:

#!/bin/bash
# Check if there are any Appwrite ACL or permission configurations
echo "=== Searching for Appwrite permissions and ACL configuration ==="
rg -n "permissions|Permissions|ACL|acl" lib/ | grep -i "message\|delete\|document" | head -30

Length of output: 64


---

🏁 Script executed:

```shell
#!/bin/bash
# Search for serverless function implementations that might handle backend validation
echo "=== Searching for function handlers and backend validation ==="
rg -n "sendMessageNotification|Function|handler" lib/ -C 2 | head -50

Length of output: 3336

🏁 Script executed:

#!/bin/bash
# Look for any Appwrite Rules configuration files
echo "=== Searching for Appwrite config files ==="
fd -e json -e yaml -e yml | xargs rg -l "appwrite\|permission\|rule" 2>/dev/null | head -20

Length of output: 43

🏁 Script executed:

#!/bin/bash
# Check for any backend-related configuration or environment setup
echo "=== Searching for Appwrite database/collection initialization ==="
rg -n "chatMessagesCollectionId|createDocument|updateDocument" lib/ -B 2 -A 2 | head -80

Length of output: 5266

🏁 Script executed:

#!/bin/bash
# Look at the complete deleteMessage implementation including surrounding context
echo "=== Full deleteMessage method context ==="
rg -n "deleteMessage" lib/controllers/room_chat_controller.dart -B 2 -A 35

Length of output: 1374

Add ownership check and not-found handling to deleteMessage() method

The review identifies legitimate security and error-handling gaps in the current implementation.

Verified issues:

• Missing ownership validation: deleteMessage() lacks a client-side check despite the UI layer already enforcing it (room_chat_screen.dart lines 121-126). Calling the method directly bypasses this protection.

• Unhandled not-found case: firstWhere() throws StateError if the message doesn't exist, caught generically without distinction from other errors.

• Full object serialization: toJsonForUpload() sends all fields including creatorId—though the proposed change only updates content and isdeleted, which is the safer approach.

Backend validation note: No Appwrite ACL configuration is visible in the codebase; backend rules must be verified separately via Appwrite console or documentation to confirm message owner restrictions are enforced server-side.

The proposed changes are sound and should be applied.


Future<void> loadMessages() async {
messages.clear();
var queries = [
Expand Down Expand Up @@ -98,6 +134,7 @@ class RoomChatController extends GetxController {
isEdited: false,
content: content,
creationDateTime: DateTime.now(),
isdeleted: false,
);

await databases.createDocument(
Expand Down Expand Up @@ -148,6 +185,7 @@ class RoomChatController extends GetxController {
updatedMessage = updatedMessage.copyWith(
content: newContent,
isEdited: true,
isdeleted: false,
);

Comment on lines 171 to 176
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don’t resurrect deleted messages on edit

Editing currently forces isdeleted: false. Block edits for deleted messages and keep the flag unchanged.

   updatedMessage = updatedMessage.copyWith(
     content: newContent,
     isEdited: true,
-    isdeleted: false,
   );
+  if (updatedMessage.isdeleted) {
+    log('Editing a deleted message is not allowed');
+    return;
+  }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In lib/controllers/room_chat_controller.dart around lines 185-190, the copyWith
call forces isdeleted: false which resurrects deleted messages; change the logic
to first check the existing message's isdeleted flag and if it's true, block the
edit (return/throw or no-op) so deleted messages cannot be edited, otherwise
perform the copyWith but do not override isdeleted (preserve the original flag)
— remove the hardcoded isdeleted: false and ensure edits only proceed for
non-deleted messages.

try {
Expand Down Expand Up @@ -247,6 +285,7 @@ class RoomChatController extends GetxController {
messages[index] = messages[index].copyWith(
content: updatedMessage.content,
isEdited: updatedMessage.isEdited,
isdeleted: updatedMessage.isdeleted,
);
if (appwriteRoom != null) {
auth.flutterLocalNotificationsPlugin.show(
Expand Down
7 changes: 7 additions & 0 deletions lib/models/message.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class Message {
String creatorName;
String creatorImgUrl;
bool hasValidTag;
bool isdeleted;
int index;
bool isEdited;
String content;
Expand All @@ -25,6 +26,7 @@ class Message {
required this.creatorImgUrl,
required this.isEdited,
required this.content,
required this.isdeleted,
required this.creationDateTime,
this.replyTo,
});
Expand All @@ -41,6 +43,7 @@ class Message {
'index': index,
'isEdited': isEdited,
'content': content,
'isdeleted': isdeleted,
'creationDateTime': creationDateTime.toUtc().toIso8601String(),
'replyTo': replyTo?.toJson(),
};
Expand All @@ -58,6 +61,7 @@ class Message {
'index': index,
'isEdited': isEdited,
'content': content,
'isdeleted': isdeleted,
'creationDateTime': creationDateTime.toUtc().toIso8601String(),
};
}
Expand All @@ -74,6 +78,7 @@ class Message {
isEdited = json['isEdited'],
content = json['content'],
creationDateTime = DateTime.parse(json['creationDateTime']),
isdeleted = json['isdeleted'] ?? false,
replyTo = json['replyTo'] != null
? ReplyTo.fromJson(json['replyTo'])
: null;
Expand All @@ -87,6 +92,7 @@ class Message {
bool? hasValidTag,
int? index,
bool? isEdited,
bool? isdeleted,
String? content,
DateTime? creationDateTime,
ReplyTo? replyTo,
Expand All @@ -102,6 +108,7 @@ class Message {
index: index ?? this.index,
isEdited: isEdited ?? this.isEdited,
content: content ?? this.content,
isdeleted: isdeleted ?? this.isdeleted,
creationDateTime: creationDateTime ?? this.creationDateTime,
replyTo: replyTo ?? this.replyTo,
);
Expand Down
121 changes: 114 additions & 7 deletions lib/views/screens/room_chat_screen.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,18 @@ class _RoomChatScreenState extends State<RoomChatScreen> {
onEditMessage: (String newContent) async {
await updateMessage(index, newContent);
},
onDeleteMessage: (String messageId) async {
await chatController.deleteMessage(messageId);
},
replytoMessage: (Message message) =>
chatController.setReplyingTo(message),
canEdit:
auth.appwriteUser.$id ==
chatController.messages[index].creatorId &&
!chatController.messages[index].isEdited,
!chatController.messages[index].isdeleted,
Copy link
Contributor

Choose a reason for hiding this comment

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

messages cannot be edited twice, so the isEdited check also needs to be there

canDelete:
auth.appwriteUser.$id ==
chatController.messages[index].creatorId,
);
},
),
Expand All @@ -138,6 +144,8 @@ class ChatMessageItem extends StatefulWidget {
final void Function(int) onTapReply;
final void Function(String) onEditMessage;
final void Function(Message) replytoMessage;
final void Function(String) onDeleteMessage;
final bool canDelete;
final bool canEdit;

const ChatMessageItem({
Expand All @@ -147,6 +155,8 @@ class ChatMessageItem extends StatefulWidget {
required this.onEditMessage,
required this.replytoMessage,
required this.canEdit,
required this.onDeleteMessage,
required this.canDelete,
});

@override
Expand All @@ -156,6 +166,7 @@ class ChatMessageItem extends StatefulWidget {
class ChatMessageItemState extends State<ChatMessageItem> {
bool isEditing = false;
late TextEditingController _editingController;

double _dragOffset = 0.0;
@override
void initState() {
Expand Down Expand Up @@ -192,13 +203,76 @@ class ChatMessageItemState extends State<ChatMessageItem> {
_editingController.text = widget.message.content;
}

void _showMessageContextMenu(BuildContext context) {
showModalBottomSheet(
context: context,
backgroundColor: Colors.transparent,
builder: (context) {
return Container(
decoration: BoxDecoration(
color: Theme.of(context).colorScheme.surface,
borderRadius: const BorderRadius.vertical(top: Radius.circular(16)),
),
child: Wrap(
children: [
if (widget.canDelete && !widget.message.isdeleted)
///delete option
ListTile(
leading: const Icon(
Icons.delete,
color: Color.fromARGB(255, 199, 169, 166),
),
title: const Text('Delete'),
onTap: () {
Navigator.pop(context);
_confirmDelete(context);
},
),

///cancel option
ListTile(
leading: const Icon(Icons.close),
title: const Text('Cancel'),
onTap: () => Navigator.pop(context),
),
],
),
);
},
);
}

void _confirmDelete(BuildContext context) {
showDialog(
context: context,
builder: (context) => AlertDialog(
title: const Text('Delete Message'),
content: const Text('Are you sure you want to delete this message?'),
actions: [
TextButton(
onPressed: () => Navigator.pop(context),
child: const Text('Cancel'),
),
TextButton(
onPressed: () {
Navigator.pop(context);
widget.onDeleteMessage(widget.message.messageId);
},
child: const Text('Delete'),
),
],
),
);
}

@override
Widget build(BuildContext context) {
return LayoutBuilder(
builder: (context, constraints) {
return Stack(
children: [
GestureDetector(
onLongPress: () => _showMessageContextMenu(context),
onHorizontalDragUpdate: (details) {
if (_dragOffset + details.delta.dx > 0.0 &&
_dragOffset + details.delta.dx < 100) {
Expand Down Expand Up @@ -227,8 +301,11 @@ class ChatMessageItemState extends State<ChatMessageItem> {
child: Container(
width: double.infinity,
padding: const EdgeInsets.all(8.0),

decoration: BoxDecoration(
color: Theme.of(context).colorScheme.primary,
color: widget.message.isdeleted
? Colors.grey[300]
: Theme.of(context).colorScheme.primary,
borderRadius: BorderRadius.circular(8.0),
),
child: Column(
Expand All @@ -252,13 +329,20 @@ class ChatMessageItemState extends State<ChatMessageItem> {
widget.message.creatorName.capitalizeFirst!,
style: TextStyle(
fontWeight: FontWeight.bold,
color: Theme.of(
context,
).colorScheme.secondary,

color:
widget
.message
.isdeleted // turns grey after getting deleted
? Colors.grey[700]
: Theme.of(
context,
).colorScheme.secondary,
),
),
const SizedBox(height: 5),
if (widget.message.replyTo != null)
if (widget.message.replyTo != null &&
!widget.message.isdeleted)
GestureDetector(
onTap: () => widget.onTapReply(
widget.message.replyTo!.index,
Expand Down Expand Up @@ -297,7 +381,8 @@ class ChatMessageItemState extends State<ChatMessageItem> {
),
),
),
if (isEditing)
if (isEditing && !widget.message.isdeleted)
///the message to edit it AND the message is not deleted
Focus(
onKeyEvent: (node, event) {
if (event.logicalKey ==
Expand Down Expand Up @@ -326,6 +411,27 @@ class ChatMessageItemState extends State<ChatMessageItem> {
),
),
)
else if (widget.message.isdeleted)
///The message has been deleted (`isDeleted = true`)
Row(
children: [
Icon(
Icons.delete_outline,
color: Colors.grey[600],
size: 18,
),
Copy link
Contributor

Choose a reason for hiding this comment

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

we should reove the icon, the representation would be better with just text. subsequently, row can be removed as well

const SizedBox(width: 8),
Expanded(
child: Text(
'This message was deleted',
style: TextStyle(
color: Colors.grey[600],
fontStyle: FontStyle.italic,
),
),
),
],
)
else
Row(
children: [
Expand All @@ -350,6 +456,7 @@ class ChatMessageItemState extends State<ChatMessageItem> {
),
],
),

const SizedBox(height: 5),
Text(
widget.message.creationDateTime
Expand Down