-
Notifications
You must be signed in to change notification settings - Fork 15
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
(feat) Add session and cohort identifiers for group sessions #72
(feat) Add session and cohort identifiers for group sessions #72
Conversation
@@ -36,6 +37,7 @@ const SessionDetailsForm = () => { | |||
|
|||
const { patientUuids } = useContext(GroupFormWorkflowContext); | |||
const { patients, isLoading } = useGetPatients(patientUuids); | |||
register("sessionUuid", { value: uuid() }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect that this would be part of the GroupFormWorkflowContext
, which is where everything else about the session is initialized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, add it to the Context
type and set a default value here, so it becomes not just part of the form, but part of the data persisted to localStorage (so we don't end up with inconsistent sessionUuids
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, add it to the
Context
type and set a default value here, so it becomes not just part of the form, but part of the data persisted to localStorage (so we don't end up with inconsistentsessionUuids
.
Thanks for your review @ibacher . I made a small refactor, following the same logic that is implemented in the other identifiers, now using the "activeSessionUuid", so everything is more organized and coherent.
I'd ask for a new review.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ibacher , can you review it again when you have time?
Thanks,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, @icrc-jofrancisco. Thanks.
Requirements
Summary
Added functionality to enhance group session data storage by incorporating session and cohort identifiers. This includes additional fields for cohort name, cohortId, and sessionUuId. These new fields complement existing ones such as sessionName, sessionDate, and sessionNotes, providing a more comprehensive dataset for group sessions.
Screenshots
Related Issue
Other
Thanks,
CC: @ibacher