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

Improve Users sync #545

Merged
merged 9 commits into from
Apr 3, 2024
Merged

Improve Users sync #545

merged 9 commits into from
Apr 3, 2024

Conversation

enahum
Copy link
Contributor

@enahum enahum commented Mar 11, 2024

Summary

Refactor syncUsers so it reads better and fixes the issue where users being activated and deactivated constantly.

We now do not create synthetic users if they are not present in MM but disabled on MS Teams

Ticket Link

https://mattermost.atlassian.net/browse/MM-56942
https://mattermost.atlassian.net/browse/MM-56886

@enahum enahum added the 2: Dev Review Requires review by a core committer label Mar 11, 2024
@lieut-data lieut-data requested review from JulienTant and removed request for lieut-data March 13, 2024 15:44
@lieut-data
Copy link
Member

@JulienTant, adding you as reviewer as part of helping to own all things synthetic users. Happy to chime in as needed -- feel free to ping me!

@JulienTant
Copy link
Member

I was reviewing the PR and accidentally committed and pushed fixes for something else here 😓 reverted now, sorry for the noise!

@lieut-data lieut-data added this to the v1.10 milestone Mar 18, 2024
Copy link
Member

@JulienTant JulienTant left a comment

Choose a reason for hiding this comment

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

Thanks for the improvements and the comments!

Copy link
Member

@jespino jespino left a comment

Choose a reason for hiding this comment

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

LGTM

@jespino jespino added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Mar 19, 2024
@jespino
Copy link
Member

jespino commented Mar 19, 2024

@enahum there is a conflict, but other than that looks good to me.

@sbishel sbishel modified the milestones: v1.10, v1.11 Mar 19, 2024
@enahum
Copy link
Contributor Author

enahum commented Mar 22, 2024

@jespino can you help look into why the tests are failing?

@JulienTant
Copy link
Member

@enahum I reran the job and everything went green! :)

@enahum enahum merged commit 3d33d70 into main Apr 3, 2024
7 checks passed
@enahum enahum deleted the user-sync branch April 3, 2024 09:24
lieut-data pushed a commit that referenced this pull request Apr 4, 2024
* refactor syncUser

* skip user creation if they are deactivated on msTeams

* add EmailVerified when creating synthetic user from a getter

* Revert "add EmailVerified when creating synthetic user from a getter"

This reverts commit 1d2a69c.

* fix bad merge

* disable selectiveSync test

---------

Co-authored-by: Julien Tant <julien@craftyx.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants