Currently we limit file uploads per post to 5. This ticket is to increase the limit to 10 and update any associated help text.
With new file upload features like a photo picker on mobile we will want to increase the files per post that can be shared for better UX.
If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.
New contributors please see our Developer's Guide.
Greetings.
Can I move forward with this one?
@DigasNikas you got it, thanks!
Greetings, after a while I鈥檝e found what needed to change, but I must say I鈥檓 a bit confused with the logic. Any chance anyone here can clarify the idea behind?
I would argue it looks clearer to have POST_MAX_FILES = 5 instead of POST_FILEIDS_MAX_RUNES = 150 if we want to limit the number of files to 5.
And then:
if len(o.FileIds) > POST_MAX_FILES {
Instead of
if utf8.RuneCountInString(ArrayToJson(o.FileIds)) > POST_FILEIDS_MAX_RUNES {
I must say 1 month ago when I first got my eyes into this logic I quickly discarded it. I'd have bet it was trying to achieve something extra :smile:
@streamer45 @hmhealey Can you help with the question above? :point_up:
Yeah, I can explain why it's written that way. We have a bit of an odd setup where there's a two-way reference between the Post and FileInfo tables in the database. Each FileInfo has a PostId, and each Post has an array of FileIds that are written into the database as a json array stored in a string. All of our IDs are 26 characters long normally, so 5 file IDs plus the rest of the JSON is a little under 150 characters long in the database.
I like the idea of making it clearer by adding POST_MAX_FILES. I'll also add this as a comment on the PR, but since we still need to confirm that the FileIds value doesn't overflow the column, I think it might be best to have two constants and just check both.
@hmhealey thanks for the detailed explanation!
Followed your suggestion.
model.post.is_valid.file_ids.app_error is now used for POST_MAX_FILES and POST_FILEIDS_MAX_RUNES checks. Probably it would be nice to eventually split this error in two.
FYI, I created a separate Jira ticket to track adding this on mobile to ensure that gets done as well.
Most helpful comment
Greetings.
Can I move forward with this one?