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

[fix] permission sync is now scheduled correctly for new users #54258

Merged
merged 3 commits into from Jun 27, 2023

Conversation

kopancek
Copy link
Contributor

Previously we relied on users_with_no_perms query to schedule new users. However this might not schedule anything for the new user, because the user might already have permissions granted from pending permissions.

This resulted in sync job for the user never reaching the permission_sync_jobs table in the database. Which in turn also resulted in user not getting the permissions that they needed as fast as possible. It also resutled in scheduled permission syncs not being scheduled at all for the user, since the schedule depends on a record in permission_sync_jobs table for each user.

Test plan

unit tests

Previously we relied on users_with_no_perms query to schedule new users.
However this might not schedule anything for the new user, because the
user might already have permissions granted from pending permissions.

This resulted in sync job for the user never reaching the permission_sync_jobs
table in the database. Which in turn also resulted in user not getting the permissions
that they needed as fast as possible. It also resutled in scheduled permission syncs
not being scheduled at all for the user, since the schedule depends on a record in
permission_sync_jobs table for each user.
@kopancek kopancek requested review from a team June 27, 2023 08:25
@kopancek kopancek self-assigned this Jun 27, 2023
@cla-bot cla-bot bot added the cla-signed label Jun 27, 2023
Copy link
Contributor

@sashaostrikov sashaostrikov left a comment

Choose a reason for hiding this comment

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

Nice fix!

@sourcegraph-bot
Copy link
Contributor

📖 Storybook live preview

@kopancek kopancek merged commit c68432a into main Jun 27, 2023
27 of 28 checks passed
@kopancek kopancek deleted the milan/schedule_perms_sync_for_new_user branch June 27, 2023 10:46
keegancsmith pushed a commit that referenced this pull request Jun 27, 2023
Previously we relied on users_with_no_perms query to schedule new users.
However this might not schedule anything for the new user, because the
user might already have permissions granted from pending permissions.

This resulted in sync job for the user never reaching the
permission_sync_jobs table in the database. Which in turn also resulted
in user not getting the permissions that they needed as fast as
possible. It also resutled in scheduled permission syncs not being
scheduled at all for the user, since the schedule depends on a record in
permission_sync_jobs table for each user.

## Test plan

unit tests

(cherry picked from commit c68432a)
keegancsmith pushed a commit that referenced this pull request Jun 27, 2023
…ew users (#54274)

Previously we relied on users_with_no_perms query to schedule new users.
However this might not schedule anything for the new user, because the
user might already have permissions granted from pending permissions.

This resulted in sync job for the user never reaching the
permission_sync_jobs table in the database. Which in turn also resulted
in user not getting the permissions that they needed as fast as
possible. It also resutled in scheduled permission syncs not being
scheduled at all for the user, since the schedule depends on a record in
permission_sync_jobs table for each user.



## Test plan

unit tests
 <br> Backport c68432a from #54258

Co-authored-by: Milan Freml <kopancek@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants