Skip to content

[FIX] Several problems with read-only rooms and muted users#11311

Merged
rodrigok merged 27 commits intodevelopfrom
fix.post-readonly-permission
May 16, 2019
Merged

[FIX] Several problems with read-only rooms and muted users#11311
rodrigok merged 27 commits intodevelopfrom
fix.post-readonly-permission

Conversation

@Hudell
Copy link
Copy Markdown
Contributor

@Hudell Hudell commented Jul 2, 2018

Closes #4769

The permission was checked when adding users to readonly channel (to determine if they are muted or not) but if the user received or lost the permission later, it would never be checked again.

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11311 July 2, 2018 16:25 Inactive
@ggazzo ggazzo added this to the 0.67.0 milestone Jul 6, 2018
@theorenck theorenck modified the milestones: 0.67.0, 0.68.0 Jul 19, 2018
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11311 August 20, 2018 13:38 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11311 August 20, 2018 13:40 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11311 August 20, 2018 13:43 Inactive
@Hudell Hudell added this to the 0.69.0 milestone Aug 20, 2018
@rodrigok
Copy link
Copy Markdown
Member

rodrigok commented Aug 20, 2018

@Hudell wouldn't be better to ignore the array of muted users since we are putting everyone in there?

@Hudell
Copy link
Copy Markdown
Contributor Author

Hudell commented Aug 20, 2018

Looks like there are some places where only the muted list is checked. I'll take a better look into the whole feature.

@Hudell Hudell mentioned this pull request Sep 13, 2018
Closed
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11311 October 11, 2018 05:27 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11311 October 15, 2018 18:27 Inactive
@Hudell
Copy link
Copy Markdown
Contributor Author

Hudell commented Oct 15, 2018

With the new changes, the muted list will only be used for manually muted users. When a room is set as readonly, the permission for sending messages anyway will be checked to determine if the user can send it.

I've also added a migration to clear the muted list of readonly rooms, since that list used to have every user in the room added automatically to it.

sampaiodiego
sampaiodiego previously requested changes Oct 20, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Along with inline comments, this PR is removing a functionality of allowing specific people to talk on a read only room. That was done by unmuting the user from members list view:
image

@theorenck theorenck modified the milestones: 0.69.0, Short-term Dec 12, 2018
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11311 March 12, 2019 22:59 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11311 March 18, 2019 17:32 Inactive
@Hudell Hudell requested a review from sampaiodiego March 25, 2019 02:05
MarcosSpessatto
MarcosSpessatto suggested changes Mar 29, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@MarcosSpessatto MarcosSpessatto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change all these imports from absolute paths(e.g from /app/models) to relative imports, mainly because @rodrigok made a PR to adopt this behavior.

@Hudell Hudell changed the title [FIX] post-readonly permission is only checked when the user is added to the channel [FIX] Several problems with read-only rooms May 7, 2019
@Hudell Hudell changed the title [FIX] Several problems with read-only rooms [FIX] Several problems with read-only rooms and muted users May 7, 2019
@Hudell Hudell mentioned this pull request May 7, 2019
Closed
@Hudell Hudell requested a review from MarcosSpessatto May 7, 2019 19:06
@MarcosSpessatto
Copy link
Copy Markdown
Contributor

MarcosSpessatto commented May 7, 2019

@Hudell can you please take a look on tests?

@rodrigok rodrigok modified the milestones: Short-term, 1.1.0 May 15, 2019
@rodrigok
Copy link
Copy Markdown
Member

rodrigok commented May 15, 2019

@Hudell can you fix the tests?

@rodrigok rodrigok merged commit 8b81b17 into develop May 16, 2019
@rodrigok rodrigok deleted the fix.post-readonly-permission branch May 16, 2019 18:28
@sampaiodiego sampaiodiego mentioned this pull request May 28, 2019
Merged
@Hudell Hudell mentioned this pull request Aug 21, 2019
Closed
@winterstefan winterstefan mentioned this pull request Sep 16, 2019
Closed
@pierre-lehnen-rc pierre-lehnen-rc mentioned this pull request May 23, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sampaiodiego sampaiodiego Awaiting requested review from sampaiodiego

1 more reviewer

@MarcosSpessatto MarcosSpessatto MarcosSpessatto approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.1.0

Development

Successfully merging this pull request may close these issues.

readonly and post-readonly permission not working properly

8 participants

@Hudell @rodrigok @MarcosSpessatto @sampaiodiego @engelgabriel @theorenck @ggazzo @pierre-lehnen-rc