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

BCeID FN LN Issue Fix #243

Merged
merged 2 commits into from
Jan 30, 2025
Merged

BCeID FN LN Issue Fix #243

merged 2 commits into from
Jan 30, 2025

Conversation

sanjaytkbabu
Copy link
Contributor

@sanjaytkbabu sanjaytkbabu commented Jan 21, 2025

Description

PADS-436
BCeID was passing both fn and ln in given_name, now we're splitting it on space to fn and ln.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

Copy link

github-actions bot commented Jan 21, 2025

Coverage Report (Application)

Totals Coverage
Statements: 36.98% ( 1103 / 2983 )
Methods: 26.82% ( 136 / 507 )
Lines: 48.66% ( 727 / 1494 )
Branches: 24.44% ( 240 / 982 )

Copy link

github-actions bot commented Jan 21, 2025

Coverage Report (Frontend)

Totals Coverage
Statements: 46.5% ( 3160 / 6795 )
Methods: 35.56% ( 416 / 1170 )
Lines: 54.31% ( 1828 / 3366 )
Branches: 40.55% ( 916 / 2259 )

@sanjaytkbabu sanjaytkbabu marked this pull request as ready for review January 22, 2025 02:18
qhanson55
qhanson55 previously approved these changes Jan 22, 2025
@@ -27,12 +27,15 @@ const service = {
.filter((claims) => claims) // Drop falsy values from array
.concat(undefined)[0]; // Set undefined as last element of array

const firstName = token.given_name.split(' ')[0];
const lastName = token.family_name.trim().length > 0 ? token.family_name : token.given_name.split(' ')[1];
Copy link
Collaborator

Choose a reason for hiding this comment

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

If someone is named Jim Joe Smith, this would only store Joe as their last name, and Smith would be omitted entirely.

This will also array out of bounds in the possibility a user comes in with only a single word for their given_name and no family_name.

jujaga
jujaga previously requested changes Jan 22, 2025
Copy link
Member

@jujaga jujaga left a comment

Choose a reason for hiding this comment

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

We cannot do this - anything we get from the JWT is canonical in nature and should not be manipulated under any circumstances when storing it in our system. We should not be interpreting the values in here because we do not have the authority to do so.

@qhanson55 qhanson55 dismissed their stale review January 22, 2025 21:55

needs further discussion

@kyle1morel kyle1morel force-pushed the bugfix/bceid-contact branch 2 times, most recently from 8724a87 to 27ff536 Compare January 28, 2025 23:37
@kyle1morel kyle1morel force-pushed the bugfix/bceid-contact branch 3 times, most recently from 5d7c5f6 to 1878549 Compare January 30, 2025 17:19
const contactSchema = object({
firstName: string().required().max(255).label('First name'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should we start using i18n for validation schemas?

@kyle1morel kyle1morel force-pushed the bugfix/bceid-contact branch from 1878549 to f010853 Compare January 30, 2025 18:28
@kyle1morel kyle1morel merged commit 4127bcb into master Jan 30, 2025
19 checks passed
@kyle1morel kyle1morel deleted the bugfix/bceid-contact branch January 30, 2025 18:46
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.

5 participants