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 · 1 comment
Open

Need to know the version tag for a StudyProtocolSnapshot #446

bardram opened this issue Aug 30, 2023 · 1 comment
Assignees
Labels
feature New functionality.

Comments

@bardram
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality.
Projects
None yet
Development

No branches or pull requests

3 participants