-
Notifications
You must be signed in to change notification settings - Fork 87
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
refactor: migrate appendVerifiedSpcpResponses to TypeScript #693
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
{ | ||
_id: '', | ||
question: 'SingPass Validated NRIC', | ||
fieldType: 'authenticationSp' as BasicField, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add typings for this? Then we can update fieldType to be a union of these typings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk, I wasn't really sure how to deal with this. one option is to create types for 'authenticationSp' and 'authenticationCp'. but actually this fieldType isn't even used anywhere, I just left it in because fieldType
is a required field for ProcessedFieldResponse
. I was hoping that when we eventually disentangle email-submissions.server.controller.js
, we'll find a better way to handle SPCP fields than creating fake fieldTypes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tl;dr I left it like this because I thought types wouldn't be useful
* refactor: standardise usage of getUniqueMyInfoAttrs (#636) * ref: migrate /spcp/redirect endpoint to TypeScript (#635) * refactor: migrate /spcp/validate to TypeScript (#656) * refactor: migrate addSpcpSessionInfo to TypeScript (#664) * refactor: migrate isSpcpAuthenticated to TypeScript (#666) * refactor: create addLoginFromForm static method (#679) * refactor: move passThroughSpcp business logic into service (#667) * refactor: migrate SPCP /login endpoints to TypeScript (#680) * refactor: migrate appendVerifiedSpcpResponses to TypeScript (#693) * docs: add docs for all new functions (#696) * test: add tests for SPCP login endpoints (#695) * feat: log login HTML * fix: change title check to null-only
This PR migrates the final SPCP factory function to TypeScript.