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 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: a3efdfe

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/fuselage-ui-kit	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/uikit-playground	Patch
@rocket.chat/models	Patch
@rocket.chat/ui-avatar	Patch
@rocket.chat/ui-client	Patch
@rocket.chat/ui-video-conf	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 56.03%. Comparing base (a3e4c06) to head (a3efdfe).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #32376      +/-   ##
===========================================
- Coverage    56.37%   56.03%   -0.35%     
===========================================
  Files         2435     2428       -7     
  Lines        53728    53601     -127     
  Branches     11068    11047      -21     
===========================================
- Hits         30289    30034     -255     
- Misses       20800    20932     +132     
+ Partials      2639     2635       -4     

Flag	Coverage Δ
e2e	56.02% <100.00%> (-0.10%)	⬇️
unit	72.08% <ø> (-0.02%)	⬇️

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?