fix: Attachments not collapsing when using incoming webhook

[rezerov]

Proposed changes (including videos or screenshots)

Right now there has been a long-standing issue where attachments don't collapse even when "collapsed": true is specified. Collapse always initializes as null (doesn't consider the value from the incoming webhook).

Before: (regardless of specifying collapsed: true or collapsed: false)

After:

Untitled.video.-.Made.with.Clipchamp.9.mp4

Issue(s)

fix #29276

Steps to test or reproduce

1. Create webhook
2. Try to send a message containing an attachment
3. Specify "collapsed": true
4. The attachment should be collapsed but it's not.
5. Specify "collapsed": false
6. The attachment is not collapsed.

Further comments

curl -H "X-Auth-Token: my-token" \ -H "X-User-Id: my-id" \ -H "Content-type:application/json" \ https://localhost:3000/api/v1/chat.postMessage \ -d '{"channel":"#general","emoji":":smirk:","text":"Sample message","attachments":[{"collapsed":true,"color":"#abcdef","fields":[{"title":"test 1","value":"test 1 value"},{"title":"test 2","value":"test 2 value"}],"text":"test text","title":"test title","title_link":"https://youtube.com","ts":"2016-12-09T16:53:06.761Z"}]}'

This command can be used to test the modification

[changeset-bot]

🦋 Changeset detected

Latest commit: d000969

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages

Name	Type
@rocket.chat/meteor	Patch
@rocket.chat/core-typings	Patch
@rocket.chat/rest-typings	Patch
@rocket.chat/core-services	Patch
@rocket.chat/cron	Patch
@rocket.chat/gazzodown	Patch
@rocket.chat/livechat	Patch
@rocket.chat/model-typings	Patch
@rocket.chat/ui-contexts	Patch
@rocket.chat/account-service	Patch
@rocket.chat/authorization-service	Patch
@rocket.chat/ddp-streamer	Patch
@rocket.chat/omnichannel-transcript	Patch
@rocket.chat/presence-service	Patch
@rocket.chat/queue-worker	Patch
@rocket.chat/stream-hub-service	Patch
@rocket.chat/api-client	Patch
@rocket.chat/license	Patch
@rocket.chat/omnichannel-services	Patch
@rocket.chat/pdf-worker	Patch
@rocket.chat/presence	Patch
rocketchat-services	Patch
@rocket.chat/ddp-client	Patch
@rocket.chat/fuselage-ui-kit	Patch
@rocket.chat/models	Patch
@rocket.chat/ui-client	Patch
@rocket.chat/ui-video-conf	Patch
@rocket.chat/uikit-playground	Patch
@rocket.chat/web-ui-registration	Patch
@rocket.chat/instance-status	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4d473bb) 49.34% compared to head (d000969) 49.35%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #31318      +/-   ##
===========================================
+ Coverage    49.34%   49.35%   +0.01%     
===========================================
  Files         3306     3307       +1     
  Lines        81287    81300      +13     
  Branches     16650    16650              
===========================================
+ Hits         40111    40128      +17     
+ Misses       36481    36478       -3     
+ Partials      4695     4694       -1     

Flag	Coverage Δ
e2e	52.56% <100.00%> (+0.06%)	⬆️
e2e-api	40.66% <ø> (-0.02%)	⬇️
unit	76.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[debdutdeb]

hm this may be one fix .. but probably not the ideal one. IIRC when i looked at this, there was an inconsistency in how collapsed was passed at different stages vs how it was read. Just a comment for now

[rezerov]

Fair enough, I was sorta hesitant to use nullish operator but it seemed like the only option at the time.

I think the inconsistency came from collapsed being declared explicitly as a parameter when it wasn't really required. The collapsed value is taken as input in sendMessage.js as a sort of intrinsic value for every attachment (just like every other argument), however it is redeclared in the Attachments.tsx file for some reason. I tried to verify further effects of the same but it does seem like the redeclaration is redundant - it was always just assuming null value initially so all attachments were always uncollapsed.

Long story short, the type defined in Attachments.tsx need not redeclare collapsed. This should be the optimal solution imo, check it out whenever you have time @debdutdeb

PS. ignore the commit history :P

[hugocostadev]

I read your considerations and I agree with your statment...

You can remove the prop from the type declaration and also from the component that is passing the collapsed prop to

Quote component here: https://github.com/RocketChat/Rocket.Chat/blob/develop/apps/meteor/client/components/message/content/attachments/QuoteAttachment.tsx#L71

[rezerov]

Perfect :) I've made the changes @hugocostadev

[janainaCoelhoRocketchat]

tested