-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add participants to studies identified through other mechanisms than email #459
Comments
The answer to the questions is most likely yes, so we are proceeding with @Whathecode was there a specific reason for the current |
Yes. Application service endpoints reflect API endpoints. The caller potentially doesn't need to know about account identities. Letting them create the concrete type at best achieves exactly the same, but at worst may leak implementation details unnecessarily. However, if we do expect more extensibility of account identity (as indicated in the description of this issue, this was currently not considered), the abstraction will necessarily need to be lifted out into the API. And, a single endpoint which takes a polymorphic account identity would make most sense. It sounds like the current needs fit more with what was originally anticipated, so instead, we should probably just add an endpoint for username/password specifically. How to deal with naming in that regard may warrant some discussion. |
I think there is not a whole lot to this issue, now that I have looked into it: in most of the places it seems that @Whathecode In general when would you do an api migration? |
I'd always do an API migration if it means a cleaner API. The point of the migration strategy is to be able to do traditionally breaking changes without any impact. The bigger question is: what is a good API here? A single generic call which requires the caller to care about AccountIdentity (I don't think they do yet), or separate calls per concrete type? I think the former is preferred if the type is already exposed through one of the types in the current API (consistency) or when it needs to be an extendable type (custom account identity implementations). The latter if these assumptions are unlikely to hold. |
Currently, CARP core supports adding participants using their email address (
RecruitmentService.AddParticipant
).There are two underlying assumptions in this design.
First, the invited participant is identified and authenticated using their email address. Adding a participant using their email ends up in a call to
AccountService.inviteNewAccount
(orAccountService.inviteExistingAccount
for existing users). As a parameter, this takesEmailAccountIdentity
as theidentity
. The implementing infrastructure typically sends out an invite to the email address to set up their account. This expectation is reflected in the documentation:Second, it assumes that when adding a participant, the researcher has access to an email address.
These assumptions don't always hold. E.g., you may want to invite people via SMS, or in Denmark using their CPR number. CARP core prepared for this through the
AccountIdentity
abstraction, whichEmailAccountIdentity
extends from.AccountService
simply forwards the original identity which was used when adding a participant (which currently only supports by email). It is up to the implementing infrastructure to map different types of identities to different authentication and invitation mechanisms.UsernameAccountIdentity
was originally introduced to reflect a traditional username/password authentication. You could create a username/password and deliver it to a user via any communication channel. As far as I recall, other than in tests, this has gone unused by platforms using CARP.For ICAT, there is now a need for an alternative to the typical email sign-up:
It sounds like the intent is to submit some data, which could be embedded in a hyperlink, which is then used to prove their identity. This raises two questions for me:
My expectation is the answer to these questions will quickly become yes, in which case
UsernameAccountIdentity
would probably be the way forward. If not, more information needs to be known, and it may warrant introducing a differentAccountIdentity
tailored to that purpose.Either way, a
RecruitmentService.AddParticipant
which takes differentAccountIdentity
types as a parameter is missing, and will need to be added. Additionally, depending on the answer to the questions above, there may be a need for platforms to easily introduce their ownAccountIdentity
types without having to modify CARP core. This would require adding anUnknownPolymorphicSerializer
forAccountIdentity
.But, the majority of other changes/considerations are most likely on an infrastructure level.
The text was updated successfully, but these errors were encountered: