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

feature(Profile Archival): Added social auth removal to Person archive. #1923

Conversation

lauryndbrown
Copy link
Contributor

Fixes #1920

Before Archival:
Screen Shot 2021-05-02 at 6 48 03 PM

After Archival:
Screen Shot 2021-05-02 at 6 52 55 PM

@lauryndbrown lauryndbrown self-assigned this May 3, 2021
@lauryndbrown lauryndbrown added python Pull requests that update Python code type: new feature labels May 3, 2021
@lauryndbrown lauryndbrown added this to In progress in Profile Archival via automation May 3, 2021
@lauryndbrown lauryndbrown added this to the v3.0 milestone May 3, 2021
@lauryndbrown lauryndbrown moved this from In progress to Review in progress in Profile Archival May 3, 2021
Copy link
Contributor

@pbanaszkiewicz pbanaszkiewicz left a comment

Choose a reason for hiding this comment

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

I think there's some mocking required. Other than that this looks good.

self.user.save()
self.user.synchronize_usersocialauth()
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this would call the GitHub API for this user. Can you mock the underlying Github class (see github_auth.py for details)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha added it.

# Add social auth to admin
self.admin.github = "admin-github"
self.admin.is_active = True
self.admin.synchronize_usersocialauth()
Copy link
Contributor

Choose a reason for hiding this comment

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

Does admin account in tests require sync with GitHub?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not necessarily. I mostly have just done it in case there was any difference between the way the admin works in comparison to the normal user.

@lauryndbrown lauryndbrown force-pushed the 1920/archived-users-cannot-login-with-github branch from 60c5a6d to 72c8327 Compare May 20, 2021 16:02
@lauryndbrown lauryndbrown force-pushed the 1920/archived-users-cannot-login-with-github branch from 72c8327 to b9d020a Compare May 20, 2021 16:04
Profile Archival automation moved this from Review in progress to Reviewer approved May 23, 2021
@lauryndbrown lauryndbrown merged commit 64bf7db into carpentries:develop May 23, 2021
Profile Archival automation moved this from Reviewer approved to Done May 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Pull requests that update Python code type: new feature
Projects
Development

Successfully merging this pull request may close these issues.

Profile Archival: Archived Users should not be able to log in via github
2 participants