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

Meshery UI permissions addition/fix #10728

Merged
merged 25 commits into from Apr 17, 2024

Conversation

dragon-slayer875
Copy link
Contributor

@dragon-slayer875 dragon-slayer875 commented Apr 12, 2024

Notes for Reviewers

This PR adds missing permissions for various actions in -

  • Header
  • Dashboard
    • ConnectionChart
    • DashboardMeshModelGraph
    • MesheryConfigurationChart
    • WorkloadChart
  • Filters
  • Designs
    • Configurator
    • CustomToolbarSelect
    • MesheryPatterns
    • MesheryPatternCards
  • Environments
    • EnvironmentsCard
  • Lifecycle
    • TransferList
  • Workspaces
    • WorkspaceCard
  • User menu
  • Settings
  • MesheryMetrics

This also fixes -

  • Settings displaying a blank screen when user doesn't have permissions.

Signed commits

  • Yes, I signed my commits.

@github-actions github-actions bot added the component/ui User Interface label Apr 12, 2024
@dragon-slayer875 dragon-slayer875 added the pr/do-not-merge PRs not ready to be merged label Apr 12, 2024
Copy link

github-actions bot commented Apr 12, 2024

@dragon-slayer875 dragon-slayer875 changed the title Meshery UI permissions Meshery UI permissions addition/fix Apr 14, 2024
Yashsharma1911
Yashsharma1911 previously approved these changes Apr 16, 2024
Copy link
Contributor

@Yashsharma1911 Yashsharma1911 left a comment

Choose a reason for hiding this comment

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

Other LGTM

Comment on lines 392 to 395
disabled={
!CAN(keys.UNDEPLOY_DESIGN.action, keys.UNDEPLOY_DESIGN.subject) ||
(CAN(keys.UNDEPLOY_DESIGN.action, keys.UNDEPLOY_DESIGN.subject) && !disabled)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We can simplify this, any reason we should stick with the change you made?

Suggested change
disabled={
!CAN(keys.UNDEPLOY_DESIGN.action, keys.UNDEPLOY_DESIGN.subject) ||
(CAN(keys.UNDEPLOY_DESIGN.action, keys.UNDEPLOY_DESIGN.subject) && !disabled)
}
disabled={!CAN(keys.UNDEPLOY_DESIGN.action, keys.UNDEPLOY_DESIGN.subject) || !disabled}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Yashsharma1911 I wrote it that way because we need to first check if the person has the permission to do that action or not, and then check if the disabled property (which is dependent on the length of the k8s context.) is true or false.
Your suggestion has the chance to enable the action even if the person doesn't have permission for that action.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though I just realised the '!' mark before diabled isn't needed and have updated the PR for the same.

Comment on lines 410 to 413
disabled={
!CAN(keys.DEPLOY_DESIGN.action, keys.DEPLOY_DESIGN.subject) ||
(CAN(keys.DEPLOY_DESIGN.action, keys.DEPLOY_DESIGN.subject) && !disabled)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

Comment on lines +62 to +64
pointerEvents: !CAN(keys.VIEW_CONNECTIONS.action, keys.VIEW_CONNECTIONS.subject)
? 'none'
: 'auto',
Copy link
Contributor

Choose a reason for hiding this comment

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

Thankyou for this

dragon-slayer875 and others added 20 commits April 17, 2024 17:04
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Signed-off-by: Rudraksh Tyagi <rudraksh875@gmail.com>
Copy link
Contributor

@Yashsharma1911 Yashsharma1911 left a comment

Choose a reason for hiding this comment

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

LGTM, please keep your eye after merging this to make sure we don't introduce any error or issue

@Yashsharma1911 Yashsharma1911 merged commit 5ee8aeb into meshery:master Apr 17, 2024
9 of 12 checks passed
@Yashsharma1911
Copy link
Contributor

Ohh ho, I missed the pr do not merge label, should we revert this PR? is this contain any breaking change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/ui User Interface pr/do-not-merge PRs not ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants