fix: Moderators with Bypass Time limits permission cannot edit messages

[KevLehman]

Proposed changes (including videos or screenshots)

Issue(s)

https://rocketchat.atlassian.net/browse/SUP-552

Steps to test or reproduce

Further comments

Fixes a problem with how the permission was being checked on the UI. Since Moderator is a role scoped to subscriptions, the call to hasPermission should have had the scope (which for a subscription, is the room).

Since this value was not being provided, it returned always false

[dionisio-bot]

Looks like this PR is not ready to merge, because of the following issues:

• This PR is missing the 'stat: QA assured' label
• This PR is targeting the wrong base branch. It should target 6.10.0, but it targets 6.9.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

[changeset-bot]

🦋 Changeset detected

Latest commit: 9864dd6

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

This PR includes changesets to release 32 packages

Name	Type
@rocket.chat/meteor	Patch
@rocket.chat/core-typings	Patch
@rocket.chat/rest-typings	Patch
@rocket.chat/apps	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-avatar	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

[KevLehman]

Note: the bypass actions are minimum 1min, so i decided to not to add UI tests on this one.

If you know a way i can do them, lmk 😄

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.58%. Comparing base (7d5bdde) to head (9864dd6).
Report is 32 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #32376      +/-   ##
===========================================
+ Coverage    55.55%   55.58%   +0.03%     
===========================================
  Files         2406     2403       -3     
  Lines        52882    52898      +16     
  Branches     10861    10866       +5     
===========================================
+ Hits         29376    29404      +28     
+ Misses       20900    20892       -8     
+ Partials      2606     2602       -4     

Flag	Coverage Δ
e2e	54.90% <100.00%> (+0.11%)	⬆️
unit	73.38% <ø> (-0.19%)	⬇️

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

[MarcosSpessatto]

We have more usage of this permission check, can we verify if the same fix needs to be applicable for the other cases?

https://github.com/RocketChat/Rocket.Chat/blob/develop/apps/meteor/client/methods/updateMessage.ts#L53
https://github.com/RocketChat/Rocket.Chat/blob/develop/apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useMessageDeletionIsAllowed.ts#L12

We also have the same check on the server side.
https://github.com/RocketChat/Rocket.Chat/blob/develop/apps/meteor/app/authorization/server/functions/canDeleteMessage.ts#L37
https://github.com/RocketChat/Rocket.Chat/blob/develop/apps/meteor/app/lib/server/methods/updateMessage.ts#L56

Shouldn't this be fixed there as well?

Finally, can we add some UI tests to ensure this behavior? And the same for API if there are no tests yet?

[KevLehman]

Hey, fixed the other places, thanks 🤗 i didn't notice we had that much usages of this permission.

On the tests part, i placed a comment just before yours 😬 about testability. Basically, for "testing" that the feature works, the minimum time allowed for this setting is 1min, and honestly I don't want to "wait" for one minute on the CI for testing this.

Was thinking on some Units, but since none of the files have Units, if we go this way, I'd prefer to have them as a separate task, wdyt?