Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: Permissions missing on new Room Edit and Contact Edit form #21315

Merged
merged 30 commits into from
Apr 1, 2021

Conversation

murtaza98
Copy link
Contributor

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

@murtaza98 murtaza98 marked this pull request as draft March 26, 2021 07:40
@murtaza98 murtaza98 marked this pull request as ready for review March 26, 2021 08:16
Copy link
Contributor

@renatobecker renatobecker left a comment

Choose a reason for hiding this comment

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

The Edit Contact action works like a charm, but the Edit Room action doesn't. This is what I get:

Screen Shot 2021-03-26 at 10 59 55 AM

@murtaza98
Copy link
Contributor Author

murtaza98 commented Mar 26, 2021

The Edit Contact action works like a charm, but the Edit Room action doesn't. This is what I get:

Screen Shot 2021-03-26 at 10 59 55 AM

@renatobecker Sorry I wasn't able to reproduce this error. Could you please check again ?
btw, I was able to edit room within the directory as well as chat room

@renatobecker
Copy link
Contributor

@murtaza98 the implementation fixes the issues, however, it's clear that both Contact and Room contextual bar are placed in the wrong folder -> client/omnichannel/directory/.

Since those components are used in different areas of the product, they should be placed in a "global" folder, not inside the "directory" folder.

@rafaelblink please, help us reviewing this PR. Thanks.

@murtaza98
Copy link
Contributor Author

@murtaza98 the implementation fixes the issues, however, it's clear that both Contact and Room contextual bar are placed in the wrong folder -> client/omnichannel/directory/.

Since those components are used in different areas of the product, they should be placed in a "global" folder, not inside the "directory" folder.

@rafaelblink please, help us reviewing this PR. Thanks.

@renatobecker I've moved out the files. Could you please review it again? Thanks

Copy link
Contributor

@renatobecker renatobecker left a comment

Choose a reason for hiding this comment

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

Where are we checking permissions for room editing and contact editing? I didn't find it.
This is how we used to check it on the old version:

https://github.com/RocketChat/Rocket.Chat/blob/develop/app/livechat/client/views/app/tabbar/visitorInfo.js#L184-L189

Also, we can't just add a permission check for rendering the button or not because the user can access the route through the URL, so the permission needs to allow/deny accessing the edit form(room or contact) anyways.

@murtaza98
Copy link
Contributor Author

@renatobecker For Room i've implemented the same logic as here.

However I'm not sure what permission to use for Contact edit? Should I create a new global permission edit-contacts and by default apply this permission to agents and managers?

@renatobecker
Copy link
Contributor

Should I create a new global permission edit-contacts

edit-omnichannel-contact

and by default apply this permission to agents and managers?

Yes.

@murtaza98
Copy link
Contributor Author

Should I create a new global permission edit-contacts

edit-omnichannel-contact

and by default apply this permission to agents and managers?

Yes.

Hey @renatobecker, This part is complete

@renatobecker renatobecker changed the title [Regression] Allow editing Rooms and Contact in the Chat Room Regression: Permissions missing on new Room Edit and Contact Edit form Mar 31, 2021
@@ -890,7 +890,7 @@ Meteor.startup(() => {

let room = Rooms.findOne({ _id: rid }, { fields: { t: 1 } });

if (room?.t === 'l' && !FlowRouter.getParam('tab')) {
if (room?.t === 'l') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@renatobecker I forgot to revert this change in the previous commit. So I'm doing it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we don't have a Chat History button in contacts directory, this is no longer needed

@renatobecker renatobecker added this to the 3.13.0 milestone Apr 1, 2021
@renatobecker renatobecker merged commit f06fd76 into develop Apr 1, 2021
@renatobecker renatobecker deleted the omnichannel/March2021-regression-2 branch April 1, 2021 03:03
@sampaiodiego sampaiodiego mentioned this pull request Apr 3, 2021
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants