Skip to content

login with external ID#403

Merged
liujoshua merged 2 commits into
ResearchStack:developfrom
DwayneJengSage:develop
Nov 11, 2017
Merged

login with external ID#403
liujoshua merged 2 commits into
ResearchStack:developfrom
DwayneJengSage:develop

Conversation

@DwayneJengSage

Copy link
Copy Markdown
Collaborator

Adding page for login with external ID.

@liujoshua liujoshua left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, looks great. One change request for consistency with ResearchKit constants

@@ -57,6 +57,8 @@ public enum SurveyItemType {
ACCOUNT_EMAIL_VERIFICATION ("emailVerification" ), // EmailVerificationStep
@SerializedName("externalID")
ACCOUNT_EXTERNAL_ID ("externalID" ), // ExternalIDStep

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ACCOUNT_EXTERNAL_ID, which currently produces a non-implemented step, is intended to support this feature. We should use this.

@dephillipsmichael, does that sound right?

See:

https://github.com/Sage-Bionetworks/BridgeAppSDK/blob/6a135f98a2e7c4020f278a76fcdf97ff2f5fc777/BridgeAppSDKSample/StudyOverviewViewController.swift#L63

@@ -690,8 +724,9 @@ public ProfileStep createProfileStep(Context context, ProfileSurveyItem item) {
* @param item InstructionSurveyItem from JSON

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please add @param loginOptions

static {
List<ProfileInfoOption> tempList = new ArrayList<>();
tempList.add(ProfileInfoOption.EXTERNAL_ID);
EXTERNAL_ID_LOGIN_OPTIONS = Collections.unmodifiableList(tempList);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

+1 for immutable

@liujoshua liujoshua merged commit 5d863e7 into ResearchStack:develop Nov 11, 2017
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.

2 participants