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: Do not automatically assign demoUsers the teamEditor role within the Templates team #3878

Merged
merged 2 commits into from
Oct 30, 2024

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Oct 30, 2024

What does this PR do?

  • Ensures that when a user is created with the demoUser role, they are not automatically granted the teamEditor role in the "templates" team
  • Test coverage for this functionality

✅ Regression tests passing against this branch here

@DafyddLlyr DafyddLlyr changed the title dp/demo user trigger update feat: Do not automatically assign demoUsers the teamEditor role within the Templates team Oct 30, 2024
@DafyddLlyr DafyddLlyr marked this pull request as draft October 30, 2024 11:42
@RODO94
Copy link
Contributor

RODO94 commented Oct 30, 2024

Create a 'demo' workspace

@DafyddLlyr DafyddLlyr force-pushed the dp/demo-user-trigger-update branch from fc95e79 to 7994d30 Compare October 30, 2024 11:46

CREATE CONSTRAINT TRIGGER grant_new_user_template_team_access
AFTER INSERT ON users
DEFERRABLE INITIALLY DEFERRED
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DEFERRABLE INITIALLY DEFERRED is new to me!

This ensures that the the trigger will run post commit, not post insert.

The insert of a new user and the associated demoUser role are grouped into a single transaction via the GraphQL mutation here.

Without DEFERRABLE INITIALLY DEFERRED, the trigger will run after the user has been inserted and then hit a race condition against insertion of the team_member record (it may check for the record before it's been inserted).

This will now wait until both the users record and team_members record has been inserted before running.

const response = await safely(() =>
$admin.client.request<{ insertUsersOne: { id: number } }>(
gql`
mutation CreateAndAddUserToTeam(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This matches the query on the frontend - might be a good reason to wrap up on planx-core?

@DafyddLlyr DafyddLlyr force-pushed the dp/demo-user-trigger-update branch from 7994d30 to b63b6e3 Compare October 30, 2024 11:53
Copy link

github-actions bot commented Oct 30, 2024

Removed vultr server and associated DNS entries

@DafyddLlyr DafyddLlyr marked this pull request as ready for review October 30, 2024 12:39
@DafyddLlyr DafyddLlyr requested a review from a team October 30, 2024 12:39
Copy link
Contributor

@RODO94 RODO94 left a comment

Choose a reason for hiding this comment

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

Working as intended for me. The DEFERRABLE INITIALLY DEFERRED bit was interesting, nice new bit of SQL

@DafyddLlyr DafyddLlyr merged commit a318850 into main Oct 30, 2024
17 checks passed
@DafyddLlyr DafyddLlyr deleted the dp/demo-user-trigger-update branch October 30, 2024 14:43
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