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

Need to know the version tag for a StudyProtocolSnapshot #446

Open
bardram opened this issue Aug 30, 2023 · 6 comments
Open

Need to know the version tag for a StudyProtocolSnapshot #446

bardram opened this issue Aug 30, 2023 · 6 comments
Assignees
Labels
feature New functionality. needs discussion This cannot yet be implemented since it requires further conversation.
Milestone

Comments

@bardram
Copy link
Collaborator

bardram commented Aug 30, 2023

In order to show it to the user in the web portal, we also need the version tag - which currently is not part of the snapshot.

See >> https://github.com/cph-cachet/carp.core-kotlin/blob/develop/carp.protocols.core/src/commonMain/kotlin/dk/cachet/carp/protocols/application/StudyProtocolSnapshot.kt

@bardram bardram added the feature New functionality. label Aug 30, 2023
@Whathecode
Copy link
Member

Whathecode commented Aug 31, 2023

I would refrain from adding it within StudyProtocolSnapshot, because protocol snapshots right now carry an "identity" which shouldn't change if the version tag changes. However, that thinking may be a bit obsolete, I'm not 100% certain about this.

Coming to think of it. The main reason not to add anything ProtocolVersion-related to StudyProtocolSnapshot is because this object is passed to the deployments subsystem, which should be agnostic about "protocols subsystem" things like that. Lifting out tag or data from ProtocolVersion, or the full object, and adding that into StudyProtocolSnapshot introduces coupling which was intentionally decoupled.

But, I understand the need. Maybe ProtocolService.getAllForOwner should return a VersionedStudyProtocol, which contains StudyProtocolSnapshot and ProtocolVersion.

That said, with a simple additional request to ProtocolService.getVersionHistoryFor, you can currently get the tag. But, I agree that gets unnecessarily chatty if you have a lot of protocols. So the request is still valid.

I think it should be possible to make changes like this with a minor version upgrade and using JSON API migration.

@bardram bardram added this to the 1.3.0 milestone Sep 3, 2024
@Whathecode
Copy link
Member

But, I understand the need. Maybe ProtocolService.getAllForOwner should return a VersionedStudyProtocol, which contains StudyProtocolSnapshot and ProtocolVersion.

@bardram Would this fulfill the current use case?

So a new type, only returned by the protocol subsystem, would be introduced.

I presume the use case is along the lines of: as a researcher, I want to see which version the protocols I manage are in, so that I can make sure the right version is used when creating new studies with it.

Again, you can see that already through a separate call. And, the getAllForOwner endpoint returns the latest version, which is what I expect you are generally interested in, if not, you can use the version history call. But, it does make sense to display to the user what this latest version is so they feel confident it matches expectations.

@Whathecode Whathecode assigned yuanchen233 and unassigned Whathecode Oct 11, 2024
@yuanchen233
Copy link
Collaborator

I believe GetAllForOwner, 'GetVersionHistoryFor', and GetBy together serves all the functionalities needed in ProtocolService.

But, I understand the need. Maybe ProtocolService.getAllForOwner should return a VersionedStudyProtocol, which contains StudyProtocolSnapshot and ProtocolVersion.

This would only be useful when a researcher 'owns' a lot of protocol, which is a rare case I would say.

After some discussion, the current use case would falls under StudyService. The version field desired is in StudySnapshot.protocolSnapshot: As a researcher, I want to know which version of the protocol is being used in the current study, without look into the content of the protocol snapshot.

This would be useful for situations where a protocol is being modified multiple times, with minor revisions, and studies are made with different versions of the same protocol. Or different versioned protocol is used in different control groups, managed in different studies.

Also, if protocols and studies are managed separately, it will be hard to see details of the protocol used by a study. For example, carp-researcher-portal visualise the content of a protocol in Protocol Section, it can get different version of the protocol using getBy. But a study does not keep track of which version of the protocol it uses, only 'latest version when creating the study', so we have to visualise the protocol used by the study in 'Study Section'.

In general, the 'version' of a protocol is treated like a new protocol, but the researcher is responsible for managing them. It makes sense of replacing 'myPtorocolV1.0, V1.1 ...' into one protocol with version tags, but then they should also be differentiated by name and tag, where the version tag part is missing.

@Whathecode
Copy link
Member

Whathecode commented Oct 11, 2024

I believe GetAllForOwner, 'GetVersionHistoryFor', and GetBy together serves all the functionalities needed in ProtocolService.

True. It would simply be a shortcut to remove the need to do one extra endpoint call. It does feel a bit like premature optimization over keeping endpoints focused/simple.

After some discussion, the current use case would falls under StudyService. The version field desired is in StudySnapshot.protocolSnapshot: As a researcher, I want to know which version of the protocol is being used in the current study, without look into the content of the protocol snapshot.

Thanks! That's a nice user story explaining the intended need.

I still don't think this means it belongs in StudyProtocolSnapshot. As you say, it is related to added requirements managed by the studies subsystem. So we need to find a way to fit it in there. That's fairly trivial in the current state, in which a protocol is "locked in" once a study goes live. We can simply store it in the study at that point in time. Only for more advanced future scenarios (trial runs within a study, after which the protocol is updated, etc.) would we need to do something more involved. That's also exactly why it belongs in the study subsystem and not in a type owned by the protocols subsystem. The main reason for change lies with how you want to/intend to support protocols in studies.

So, why not in StudySnapshot?

@Whathecode Whathecode added the needs discussion This cannot yet be implemented since it requires further conversation. label Oct 11, 2024
@yuanchen233
Copy link
Collaborator

yuanchen233 commented Oct 11, 2024

Yes, it could simply be a protocolVersion: ProtocolVersion field for now.

Only for more advanced future scenarios (trial runs within a study, after which the protocol is updated, etc.)

Interesting! and also complex, everything depends on the protocol, this will be a soft reset/re-deploy for the study.

We also keep a copy of studyProtocolSnapshot in other places tho: Recruitment, StudyDeployment.. we should also consider adding this protocolVersion to them, if we decided to go for this approach.

If so, we could have a study running, and deploy deplyments with different protocol version.

@Whathecode
Copy link
Member

Whathecode commented Oct 11, 2024

We also keep a copy of studyProtocolSnapshot in other places tho: Recruitment, StudyDeployment.. we should also consider adding this protocolVersion to them, if we decided to go for this approach.

Since Recruitment reflects the "participants" side of things in a study, I doubt it. But, we can discuss if/when the need arises.

StudyDeployment unlikely, and even more likely to be avoided. This is the deployment subsystem, which we should really keep devoid of study management things which instead should be layered on top. The deployment subsystem knows nothing about "studies". If a study subsystem implementation ever decides to run different protocol versions within the same study, the linking of which deployment has which version should happen there.

It feels naturally at home in StudyService. And, checking in a bit more detail, probably StudyDetails would be the place to expose it (through the existing getStudyDetails()), and StudySnapshot to persist it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality. needs discussion This cannot yet be implemented since it requires further conversation.
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

4 participants