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

[ENG-2562] Fix Source Tags for Users #10696

Conversation

Johnetordoff
Copy link
Contributor

@Johnetordoff Johnetordoff commented Aug 8, 2024

Purpose

Adds source tags to users created via email invitation or sso.

Changes

  • adds line to SSO user creation method adding system tag
  • adds line to unregistered_user method to add system tag

QA Notes

N/A

Documentation

N/A

Side Effects

N/A

Ticket

https://openscience.atlassian.net/browse/ENG-2562

@Johnetordoff Johnetordoff force-pushed the fix-source-tags branch 5 times, most recently from 9786736 to 4efe8a4 Compare August 8, 2024 16:53
Copy link
Contributor

@cslzchen cslzchen 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 quick work, looks good overall 🎆

One requested change is correctly handle tags secondary institution.

In addition, need to check (probably with @brianjgeiger) if the tags we added here make sense.

Finally, need to check (probably with Product folks) if existing user should be tagged with institution when they get affiliated.

api/providers/serializers.py Show resolved Hide resolved
osf/models/mixins.py Show resolved Hide resolved
osf/models/osf_group.py Show resolved Hide resolved
website/project/views/contributor.py Show resolved Hide resolved
framework/auth/__init__.py Show resolved Hide resolved
api_tests/institutions/views/test_institution_auth.py Outdated Show resolved Hide resolved
website/util/metrics.py Show resolved Hide resolved
osf/models/user.py Show resolved Hide resolved
website/util/metrics.py Show resolved Hide resolved
@brianjgeiger
Copy link
Collaborator

I do not have a preference on the source tag names. Product uses them, so if there's going to be a preference, they'll have it. My guess is they won't be too fussy as long as they are distinct enough.

@eolson3
Copy link

eolson3 commented Aug 12, 2024 via email

api/institutions/authentication.py Outdated Show resolved Hide resolved
@brianjgeiger brianjgeiger changed the base branch from develop to feature/b-and-i-24-14 August 13, 2024 13:44
@brianjgeiger brianjgeiger changed the base branch from feature/b-and-i-24-14 to develop August 13, 2024 13:44
@brianjgeiger
Copy link
Collaborator

@Johnetordoff Could you rebase and target the b&i branch for this?

@Johnetordoff Johnetordoff changed the base branch from develop to feature/b-and-i-24-14 August 13, 2024 14:51
@Johnetordoff Johnetordoff force-pushed the fix-source-tags branch 2 times, most recently from 497424e to 0aadaee Compare August 13, 2024 15:01
@brianjgeiger brianjgeiger merged commit 9d02957 into CenterForOpenScience:feature/b-and-i-24-14 Aug 14, 2024
6 checks passed
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Aug 21, 2024
…penScience/osf.io into fix-preprint-emails

* 'feature/b-and-i-24-14' of https://github.com/CenterForOpenScience/osf.io:
  Make resubmissions more like submissions (CenterForOpenScience#10709)
  renamed files with travis in their names
  fixed case where it was more appropriate
  removed all travis mentions and replaced them with CI
  [ENG-2562] add system tags to users created via institutional sign up system (CenterForOpenScience#10696)
  refactor handle_duplicate_notifications and add tests
  Shorten lines; rename script for test clarity
  fix flake8 errors
  add admin screen to manage duplicate notifications
  [ENG-2814] Allow Read-only and Read/Write contributors to view a project's draft registrations (CenterForOpenScience#10660)
  [CR][ENG-5997] merge develop into b-and-i branch (CenterForOpenScience#10691)
  add exception handling in case state doesn't change
  [ENG-4527] Fix citation to use registered date (CenterForOpenScience#10678)
  restrict state changes more and allow no-ops
  split apart change provider views from general preprint view and machine_state change viewa
  Re-add permissions changes for files on withdrawn registrations (CenterForOpenScience#10671)
  [ENG-4903] Fixes issue with email confirmation links failing due to database congestion (CenterForOpenScience#10662)

# Conflicts:
#	osf/utils/notifications.py
#	website/templates/emails/reviews_resubmission_confirmation.html.mako
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Aug 21, 2024
…penScience/osf.io into fix-preprint-emails

* 'feature/b-and-i-24-14' of https://github.com/CenterForOpenScience/osf.io:
  Make resubmissions more like submissions (CenterForOpenScience#10709)
  renamed files with travis in their names
  fixed case where it was more appropriate
  removed all travis mentions and replaced them with CI
  [ENG-2562] add system tags to users created via institutional sign up system (CenterForOpenScience#10696)
  refactor handle_duplicate_notifications and add tests
  Shorten lines; rename script for test clarity
  fix flake8 errors
  add admin screen to manage duplicate notifications
  [ENG-2814] Allow Read-only and Read/Write contributors to view a project's draft registrations (CenterForOpenScience#10660)
  [CR][ENG-5997] merge develop into b-and-i branch (CenterForOpenScience#10691)
  add exception handling in case state doesn't change
  [ENG-4527] Fix citation to use registered date (CenterForOpenScience#10678)
  restrict state changes more and allow no-ops
  split apart change provider views from general preprint view and machine_state change viewa
  Re-add permissions changes for files on withdrawn registrations (CenterForOpenScience#10671)
  [ENG-4903] Fixes issue with email confirmation links failing due to database congestion (CenterForOpenScience#10662)

# Conflicts:
#	osf/utils/notifications.py
#	website/templates/emails/reviews_resubmission_confirmation.html.mako
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.

4 participants