parse-community / parse-dashboard

A dashboard for managing Parse Server
https://parseplatform.org
Other
3.74k stars 1.38k forks source link

feat: Add links to users and roles in ACL dialog and handle invalid entries #2436

Closed FransGH closed 1 year ago

FransGH commented 1 year ago

New Pull Request Checklist

Issue Description

The PermissionsDialog currently offers no way to inspect users and roles.

Closes: #2435 and #2437

Approach

This makes roles and user in the most simple way clickable and open in a separate browser window (tab). It avoids complex in-app navigation issues by not attempting to browse within the same window and offer an option to return back to the CLP dialog.

TODOs before merging

parse-github-assistant[bot] commented 1 year ago

Thanks for opening this pull request!

uffizzi-cloud[bot] commented 1 year ago

Uffizzi Ephemeral Environment deployment-26634

:watch: Updated May 25, 2023, 12:24 UTC

:cloud: https://app.uffizzi.com/github.com/parse-community/parse-dashboard/pull/2436

:page_facing_up: View Application Logs etc.

What is Uffizzi? Learn more

FransGH commented 1 year ago

The way the url is composed is a bit hackie and will clearly fail if the current location is not ../browser/.. Didn't dive that deep into the code to get a clean base url but happy with suggestions!

mtrezza commented 1 year ago

Could you add a before / after screenshot so its easier to see where the changes are?

FransGH commented 1 year ago

Screenshot from v3.2.1 vs latest alpha. Don't ask me what happened to the role-objectIds between these versions, I didn't take them out. The only visual change as a result of the PR is that the names are now blue (= default link color) as opposed to grey and the cursor changes to a pointer while hovering (standard link behaviour). PastedGraphic-2 PastedGraphic-1

mtrezza commented 1 year ago

I've just tried it out, I think it's nicely done, for me the links are even better visible than in your screenshot:

image
mtrezza commented 1 year ago

The way the url is composed is a bit hackie and will clearly fail if the current location is not ../browser/..

@parse-community/dashboard any suggestion?

FransGH commented 1 year ago

There appear to be two different layouts for the permission list. One shows the object id's the other doesn't. The one with the object Id's, like in your screenshot, looks better. I see both flavours on both the older and latest dashboard depending on the object's ACL.

This is now fixed, see #2437

FransGH commented 1 year ago

image

mtrezza commented 1 year ago

Great, could you explain, what is the "and handle invalid entries"?

FransGH commented 1 year ago

Great, could you explain, what is the "and handle invalid entries"? Basically fixed a bug that caused the render function to not work as intended when just one of the role or user objects was not found. With 'handling' I mean that they now explicitly appear as 'not found' on the UI.

mtrezza commented 1 year ago

Got it. That could have been a separate PR, but we can merge this as 1 since you it's already mixed.

In what cases do you think the URL approach could cause issues?

FransGH commented 1 year ago

I haven't experienced any issues with the url approach so far. It really solved a major pain I was having while migrating from user to role based ACLs. Still like to improve how the url is composed so hoping for some feedback.

mtrezza commented 1 year ago

Well if there is no feedback then let's merge as is. If it's not working someone will hopefully open an issue and we can look at it then.

Maybe you want to take a look at the code and see how that is solved at other places?

If you take a look at this for example:

https://github.com/parse-community/parse-dashboard/blob/52ca7fc064cca60c7c7474da7417b39d0e596dbd/src/dashboard/Data/Browser/Browser.react.js#L858

There's a generatePath import that may be helpful. It seems to be used to compose the URL from the current context.

FransGH commented 1 year ago

Yep, that worked, thx!

parseplatformorg commented 1 year ago

🎉 This change has been released in version 5.2.0-alpha.2

parseplatformorg commented 1 year ago

🎉 This change has been released in version 5.2.0-beta.2

parseplatformorg commented 11 months ago

🎉 This change has been released in version 5.2.0