Add separate endpoint for create and invite participant group methods…

[yuanchen233]

… to RecruitmentService

Also marks InviteNewParticipantGroup Endpoint and related method as deprecated, and replace their usage with new create and invite method respectively.

---

• I did not include connectedDevicePreregistration here, as I think it should be a separate issue/commit.
• I did not include 'add UpdateParticipantGroup endpoint' in this PR, to keep thing clean here.

Qs:

• Should test cases related to inviteNewParticipant be removed? as its functionalities are now in create and invite, respectively and we have coverage for them. So some suppress can be removed. ( Kotlin compiler fails it even the deprecation level is warning )

---

TODO(not for this PR):

1. Update 'stopParticipantGroup' parameter in the order from the most relevant one to secondary ones.
2. Create an issue to keep all creation endpoint consistent: let user pass IDs.
3. Create new endpoint UpdateParticipantGroup

[Whathecode]

Only reviewed the interface for now.

[Whathecode]

Reviewed RecruitmentServiceHost. Primarily, we should try to come to a decision on whether or not we want idempotency and then pursue something consistent.

[Whathecode]

Still reviewing, but here are some further comments. :)

[Whathecode]

Some minor comments, but then I ran into the getRecruitments() method on the repository, which is a bigger problem and I'll already post now so you can fix it while I continue reviewing later on. :)

[Whathecode]

Re-reading this again, I think this exception warrants a bit more explanation. I would add:

As long as no final study protocol is locked in for the study, a participant group can't be created since participant roles to which participants need to be assigned are unknown.

Don't forget to also update RecruitmentServiceHost. :)

That's at least where the current limitation comes from. But, that does seem limiting and perhaps a useful area to improve functionality for later on.

In the main use case there is only a single participant and participant roles are irrelevant. That's why AssignTo.All exists. If group only contains AssignTo.All, that should be a valid configuration for any study protocol, even if it changes after.

And/or, maybe it is fine to only validate the assignments in context of the current protocol, make the assignments nullable, and simply invalidate existing role assignments when a new protocol is set.

Does this make sense? If so, we should likely spin off a new feature/discussion issue for this.

[yuanchen233]

Yes, this feature would decouple study setup from inviting participants. So as a researcher, I can recruit participant before finalize technical details "protocol".

In the main use case there is only a single participant and participant roles are irrelevant. That's why AssignTo.All exists. If group only contains AssignTo.All, that should be a valid configuration for any study protocol, even if it changes after.

Since protocol also defines the number of participant, AssignTo.All could be misused here. But yep, the discussion should be hold in a separate issue.

[yuanchen233]

Re-reading this again, I think this exception warrants a bit more explanation. I would add:
As long as no final study protocol is locked in for the study, a participant group can't be created since participant roles to which participants need to be assigned are unknown.

Should this also be added to Recruitment.addParticipantGroup's documentation, or the error message?

[Whathecode]

Minor: drop punctuation at the end. Seemingly it's not used in any of the other bullet lists.

[Whathecode]

I think you can safely drop "ByGroupId" as that is already documented by the parameter name.

[Whathecode]

This is a very bad idea. It's a full table dump and will inevitably get slower and slower.

I guess you added this to be able to drop the studyId parameter. But, the way the repository pattern in DDD works is that you are supposed to expose the queries you need from the domain layer in the interface (the use cases), instead of the very typical simplistic CRUD operations and end up writing query code in the layer above.

So, instead what we likely need is a getRecruitmentWithParticipantGroup( groupId ). This can be optimized by whatever underlying DB technology and doesn't pull in unnecessary data.