Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: Multiple file sharing in one message #32703
base: develop
Are you sure you want to change the base?
feat: Multiple file sharing in one message #32703
Changes from 125 commits
4ee6d2f
a1c0569
640ed9e
59d99a6
d84ce58
e6a4330
677f0ef
7c7eb29
a9add79
69f815e
ebbe64f
b43f57b
ec06eb3
f70d2e1
445c528
6daab1a
d14a0e0
fd5fcd2
2e1f78a
d307519
1e25638
574169f
7154756
222cbf3
aff80f3
de5f0f2
939940d
7fb9618
26e2eb8
3234c56
d894d30
aacf63d
ee3fe8a
956316e
13566ab
611f91e
48a97f7
446b1a7
35363b6
c258b21
989c61c
0a3cb4d
403ced3
ae670ff
de94f83
9da9640
8c7310f
5228bdb
9e83634
8f81edd
b9ff006
9a39914
20962c4
74cfee1
6508ebe
1a1788e
64a27cb
b5e1ba5
f108fd4
ae76481
18d0e87
e984ad2
c251e17
a96fa61
3f480b0
fa2456a
365cef8
bd90357
81dad69
8402305
503218e
09e3bb7
f6162af
b739659
a1f7848
74ed1f2
cbb41bb
c93da30
cfc177c
2e344a1
7f0b615
671fd31
8379c94
9a97c3d
62fbc56
f88cd26
f39042b
5a9f646
771314e
755c08b
f2bd552
39ff2a3
23eb009
e786f70
f4b60d5
1c0ddde
f5213b0
6f398b4
cdc0ff5
a1faa22
326a197
c078083
0e4d827
60a80f9
7ac5841
9f74219
9afeee6
97f743b
68f45dc
d7b92cc
21005c0
a0f6c83
de319d8
3ad5e63
72bc00d
fe2b428
b3db9fc
3aa8c3d
2eb9b64
11aac8e
9ff1b9f
2370b14
47b592e
9722194
84bfcae
335e886
a5598f7
7ea18d7
822e2c5
d1b6922
b13bdd2
a8cff84
0fe3aee
7347447
65c7be7
c4fb104
12abfc5
ed463c0
8c91c8c
7e5081b
e45e376
d21dd7f
8508954
ea689a2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check failure on line 73 in apps/meteor/app/file-upload/server/methods/sendFileMessage.ts
GitHub Actions / 🔎 Code Check / Code Lint
Check failure on line 75 in apps/meteor/app/file-upload/server/methods/sendFileMessage.ts
GitHub Actions / 🔎 Code Check / Code Lint
Check failure on line 88 in apps/meteor/app/file-upload/server/methods/sendFileMessage.ts
GitHub Actions / 🔎 Code Check / Code Lint
Check failure on line 16 in apps/meteor/app/lib/server/functions/sendMessage.ts
GitHub Actions / 🔎 Code Check / Code Lint
Check failure on line 16 in apps/meteor/app/lib/server/functions/sendMessage.ts
GitHub Actions / 🔎 Code Check / Code Lint
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 API should continue sending only a single file a time. The places where we send multiple files should call this API multiple times
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.
Thank you for the clarification @rodrigok
I have currently implemented it so that if there's a single file, it's converted to an array and sent using the API. For multiple files, I'm looping through each file and sending them one at a time using the API.
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.
@abhipatel0211 got it, but that is what is wrong. This API should only make the file upload. Should not even receive the message content to call the sendMessage inside it. The responsibility of this API should be to upload the file and return the file data to be stored in memory in the message composer. There you will call the sendMessage passing the list of files to be confirmed.
This code was like that because the flow to upload the file was coming from the modal and the old API was doing both things (upload and message send) together.
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.
Think that you don't need to upload the files only when the message is sent. You can upload each file when you are attaching it to the message box, so when you send the message later the files are already uploaded and you only confirm the uploads via ID. If the user forget the message in composing status the files will end up deleted after a while because they were not confirmed.
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.
@rodrigok sir so is it good to create an another uploadsFiles function which will be just used for the the fileUpload and return id. As this uploads function can be also used from uploadfiles and other places also (like live audio and video is sharing).
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.
Yes, you should extract the upload logic to another function
Check failure on line 35 in apps/meteor/client/lib/chats/uploads.ts
GitHub Actions / 🔎 Code Check / Code Lint
Check failure on line 35 in apps/meteor/client/lib/chats/uploads.ts
GitHub Actions / 🔎 Code Check / Code Lint
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.
What is this for?
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.
Currently, when sharing multiple files in one message, the preview always shows the very first image from the set, even if a different image is clicked. The preview should display the selected image correctly.
I've made adjustments to address this issue, ensuring that the correct image is displayed when clicked.
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.
Got it, we need to solve the root issue later, the issue happens here https://github.com/abhipatel0211/rocket.chat/blob/cdc0ff5ff9f82be59af8bdf4b4e70e478ebf129d/apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx#L73 since we pass only the first file id to the attachment elements. We, probably, have to save the file id inside the attachment to use it there and use the first id passed as fallback (for the old records)