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

Permission ordering should show shallow permissions first #1011

Closed
3 tasks done
chronark opened this issue Feb 20, 2024 · 16 comments · Fixed by #2273
Closed
3 tasks done

Permission ordering should show shallow permissions first #1011

chronark opened this issue Feb 20, 2024 · 16 comments · Fixed by #2273
Assignees

Comments

@chronark
Copy link
Collaborator

chronark commented Feb 20, 2024

Preliminary Checks

Reproduction / Replay Link (Optional)

No response

Issue Summary

All non-nested permissions should show before nested permissions

CleanShot 2024-02-20 at 16 47 57@2x

Steps to Reproduce

  1. (for example) Went to ...
  2. Clicked on...
  3. ...

Expected behavior

The order should be:

  • domain
    • delete_domain
    • read_domain
    • update_domain
    • dns
      • create_record
      • delete_record
      • read_record
      • update_record

Other information

No response

Screenshots

No response

Version info

- OS:
- Node:
- npm:
@chronark chronark added Bug Something isn't working Needs Approval Needs approval from Unkey labels Feb 20, 2024
Copy link

linear bot commented Feb 20, 2024

@chronark chronark removed the Needs Approval Needs approval from Unkey label Feb 20, 2024
@AkshayBandi027
Copy link
Contributor

AkshayBandi027 commented Feb 21, 2024

hey @chronark,

I have made some changes to tree component. Added a function that maps over nestedPermissions then every checks every permission if it as any nested permission if it contain then pushes to bottom of array and if doesn't then pushes to top of array.

image

And then finally we map over the sortedNestedPermissions array and return recursivePermission component.

image

It works as Excepted!!

https://www.loom.com/share/87013f98356f467da5752b5b0abbf543?sid=92c83072-0a95-429a-bb77-0e08af5c23e9

Any feedback would be appericated!

@chronark
Copy link
Collaborator Author

Nice, thank you

can you still sort alphabetically within one depth?
Right now they are out of order, it should be

  • delete_record
  • read_record
  • update_record
  • write_record
    CleanShot 2024-02-21 at 10 24 08@2x

@chronark chronark assigned chronark and AkshayBandi027 and unassigned chronark Feb 21, 2024
@AkshayBandi027 AkshayBandi027 removed their assignment Mar 5, 2024
@perkinsjr perkinsjr added the Good first issue Good for newcomers label Jul 12, 2024
@kfahad5607
Copy link

Can this issue be assigned to me? I have fixed the sorting order.

Here is the sorter function:

image

Sorting the arrays using .sort before mapping:

image
image

Permissions in proper order:

image
image

@DeepaPrasanna
Copy link
Contributor

/assign

Copy link

oss-gg bot commented Oct 7, 2024

Assigned to @DeepaPrasanna! Please open a draft PR linking this issue within 48h ⚠️ If we can't detect a PR from you linking this issue in 48h, you'll be unassigned automatically 🕹️ Excited to have you ship this 🚀

@nilaygit-10721
Copy link

/assign

Copy link

oss-gg bot commented Oct 10, 2024

This issue is already assigned to another person. Please find more issues here.

@aryanbansal73
Copy link

/assign

Copy link

oss-gg bot commented Oct 10, 2024

This issue is already assigned to another person. Please find more issues here.

@aryanbansal73
Copy link

/assign

Copy link

oss-gg bot commented Oct 11, 2024

This issue is already assigned to another person. Please find more issues here.

@suraj-xd
Copy link

/assign

Copy link

oss-gg bot commented Oct 12, 2024

This issue is already assigned to another person. Please find more issues here.

@sour2001
Copy link

/assign

Copy link

oss-gg bot commented Oct 15, 2024

This issue is already assigned to another person. Please find more issues here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants