fix: Overriding Retention Policy not working

[dougfabris]

Proposed changes (including videos or screenshots)

Issue(s)

Closes #30810
Closes #31667
Closes #19730
Closes #20348
Closes #20186
Closes #24894
Closes #30567

Steps to test or reproduce

Further comments

SUP-563

[dionisio-bot]

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

[changeset-bot]

🦋 Changeset detected

Latest commit: df95565

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

[codecov]

Codecov Report

Attention: Patch coverage is 71.05263% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 56.03%. Comparing base (f83bd56) to head (df95565).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #32454      +/-   ##
===========================================
+ Coverage    55.85%   56.03%   +0.18%     
===========================================
  Files         2432     2431       -1     
  Lines        53568    53557      -11     
  Branches     11028    11030       +2     
===========================================
+ Hits         29919    30011      +92     
+ Misses       21013    20901     -112     
- Partials      2636     2645       +9     

Flag	Coverage Δ
e2e	55.46% <71.05%> (+0.29%)	⬆️
e2e-api	41.27% <ø> (+0.11%)	⬆️
unit	72.30% <ø> (-0.02%)	⬇️

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

[Gummikavalier]

@dougfabris I had a go with retention time fix with 6.9.0 rc1. When setting the test scenario up I noticed this issue:

This happens when trying to change the channel type from private to public with channel owner privileges.

The two days retention in the picture is a global default for all channels.

The user owning this channel does not have pruning rights aka Clean Channel History privilege (the default). I did not touch anything else on this newly created channel, only tried to switch it from private to public directly after the channel was created with the same account. I also cleaned cached while testing for this new issue.

Edit: Giving the owner role the Clean Channel History privilege did not fix it either.
In effect in 6.9.0-rc.1 it is not possible for the channel owner to edit any room details.

[dougfabris]

@Gummikavalier Nice catch, an issue indeed!
It's related to the edit-room-retention-policy permission. Room owners don't have it by default so they can't change retention policy. We should hide the retention based on this permission on the UI, I will raise the proper fix.

Thank you so much for helping us 🚀

[dougfabris]

@Gummikavalier JFYI, the fix for it will be on 6.9.0.

[Gummikavalier]

@dougfabris FYI. Sorry! While the retention policy options do not show anymore in 6.9.0 (just released), the actual error message still happens in the most common scenario of creating a new channel and then trying to edit it.

Note! After you have given the room owner the edit-room-retention-policy permission, saving a change onto it, and then again removed the permission, after that you can change the room settings without issues for that particular room. For other rooms the error still happens.

I'm guessing but this could indicate that once the first edit has been done with all required permissions thus creating the required lines into the document (any kind, even global or empty one), it succeeds from there on for that channel.

So the channel creation and editing process might need this entry to be checked if it exists first, and if not, then setting it up for default/empty. And existing channels would need to have this set too during the upgrade.
Or different type of permission for simply creating those lines for regular owner if they don't exist already.

The issue can be worked around by using that edit-room-retention-policy for the owners, but it is a bit of an overkill for that if we want to prevent owners doing these changes on their own.

[dougfabris]

@Gummikavalier Indeed, I tested only the input render and I missed checking the permission when initiating the form values, so when users without the edit-room-retention-policy permission tries to edit the room for the first time, they ended up on the error again. I will raise a fix and I'll try to add to a possible path release for 6.9.x.

Thanks again for the huge support on this!

[Gummikavalier]

Apart from that issue with permission during making changes as regular owner, the actual pruning tests seemed to work ok.

I had different channels with 3 and 1 days retention, 2 days as global policy for others, running for several days. Messages remained and disappeared accordingly.

One channel was set to never prune and the messages have stayed intact there as they should.

Looks good to me. 👍