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

Orcid update #62

Merged
merged 6 commits into from
Sep 28, 2022
Merged

Orcid update #62

merged 6 commits into from
Sep 28, 2022

Conversation

pradtke
Copy link
Contributor

@pradtke pradtke commented Sep 14, 2022

No description provided.

@pradtke
Copy link
Contributor Author

pradtke commented Sep 26, 2022

@windhamg could you add a unit test for the email handling? I think cover these cases:

  • no emails
  • no primary email but an non-primary
  • a primary and a non-primary
  • only a primary

It's a pain to unit test the postFinalStep method due to the rest calls, so I'd recommend breaking out the logic about parsing the response to get an email into its own method that returns the email (or null), and then test that method by passing in different response arrays.

@windhamg
Copy link
Contributor

@pradtke unit test has been added

@windhamg could you add a unit test for the email handling? I think cover these cases:

  • no emails
  • no primary email but an non-primary
  • a primary and a non-primary
  • only a primary

It's a pain to unit test the postFinalStep method due to the rest calls, so I'd recommend breaking out the logic about parsing the response to get an email into its own method that returns the email (or null), and then test that method by passing in different response arrays.

@pradtke pradtke merged commit ad8c75f into cirrusidentity:master Sep 28, 2022
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.

None yet

2 participants