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

[CAPT-2051] Better OL name #3454

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

[CAPT-2051] Better OL name #3454

wants to merge 3 commits into from

Conversation

asmega
Copy link
Contributor

@asmega asmega commented Dec 5, 2024

Context

Changes

  • Persist all the names that OL returns
  • When checking against OL name we match entered name matches the start and end of returned OL name
  • onelogin_idv_full_name is no longer a virtual attribute and returns the full name OL returns
  • We backfill onelogin_idv_full_name with data we have which is concatenation of first and last name

Notes

  • Once this is complete we can work to remove onelogin_idv_first_name and onelogin_idv_last_name as these will become redundant

@asmega asmega added the deploy Deploy a review app for this PR label Dec 5, 2024
Copy link
Contributor

@slorek slorek left a comment

Choose a reason for hiding this comment

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

Alternatively we could consider keeping the separate first name and last name fields, but concatenating the name parts for GivenName and FamilyName types, rather than only selecting the first of each

@@ -21,6 +21,10 @@ def date_of_birth
Date.parse(decoded_jwt[0]["vc"]["credentialSubject"]["birthDate"][0]["value"])
end

def full_name
name_parts.map { |hash| hash["value"] }.join(" ")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we ensure that the FamilyName part(s) are always last? I suspect there's no guarantee it will be ordered this way already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy Deploy a review app for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants