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

feat: all users can edit "Templates" team and moveFlows to teams they're allowed to edit #2239

Merged
merged 2 commits into from
Sep 27, 2023

Conversation

jessicamcinchak
Copy link
Member

@jessicamcinchak jessicamcinchak commented Sep 27, 2023

  • I added a "Templates" team to production & staging, this change ensures any user will have teamEditor access to "Templates" by default when they're added independent of records in the team_members table (if this feels too liberal, very happy to re-think how we should do this!)
  • moveFlow Editor store method now checks if the given user has permission to move the flow into the requested team. If they don't, it'll display and error and return early. The copy/move menu is only visible in teams you can edit already, so I don't think we need to do anything similar with other store methods besides moveFlow yet.

Screenshot from 2023-09-27 09-46-52

Testing:

  • (If you're a platformAdmin, toggle this off first in https://hasura.2239.planx.pizza/console/login)
  • I see an "edit" icon next to Templates even though I do not have a team_members entry
  • Inside "Templates", I can create a flow, copy it, and move it - I can successfully move to a team that I have edit access to, but see an error message when I try to move it to a team I can only view

@jessicamcinchak jessicamcinchak changed the title feat: all users can edit Templates team feat: all users can edit Templates team and moveFlows to teams they're allowed to edit Sep 27, 2023
@jessicamcinchak jessicamcinchak changed the title feat: all users can edit Templates team and moveFlows to teams they're allowed to edit feat: all users can edit "Templates" team and moveFlows to teams they're allowed to edit Sep 27, 2023
Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

Unfortunately, I think this approach won't work as expected.

The way I understand it to work is that Hasura will run a check against the team_members table before any actions, so if we modify the UI or even the JWT to insert a role for the templates team it should fail.

There's a few ways we could achieve this -

  1. Creating a record for each user in the team_members table to give them teamEditor role for the templates team. A migration for all existing users + maybe a trigger on user creation to insert a record could work.

  2. Modify the permissions check in Hasura to say "if user is team member, or team = templates"

No strong feelings either way here - I suspect option 1 is a little more robust / discoverable / expected?

@DafyddLlyr
Copy link
Contributor

DafyddLlyr commented Sep 27, 2023

Ah actually I might be wrong here sorry...!

I thought it was a copy + move we needed, but I think this would work for just a move. I'm going to get a coffee... ☕ 😅

@github-actions
Copy link

github-actions bot commented Sep 27, 2023

Removed vultr server and associated DNS entries

@jessicamcinchak
Copy link
Member Author

jessicamcinchak commented Sep 27, 2023

Next steps:

  • This currently works because the underlying API routes for moving & copying aren't scoped to user permissions yet
  • We'll need to update this approach once that is implemented, but we're happy to merge this in the meantime because the UI will remain consistent

@jessicamcinchak jessicamcinchak merged commit f7259d2 into main Sep 27, 2023
12 checks passed
@jessicamcinchak jessicamcinchak deleted the jess/templates-team-edit-access branch September 27, 2023 08:51
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.

2 participants