Skip to content

CARDS-2558: Clinician dashboard#2001

Open
sdumitriu wants to merge 11 commits intodevfrom
CARDS-2558-clinician-dashboard
Open

CARDS-2558: Clinician dashboard#2001
sdumitriu wants to merge 11 commits intodevfrom
CARDS-2558-clinician-dashboard

Conversation

@sdumitriu
Copy link
Member

To test:

  • build
  • run with ./start_cards.sh -P prems --dev -f 'mvn:io.uhndata.cards/cards-clinician-portal/${CARDS_VERSION}/slingosgifeature' -f 'mvn:io.uhndata.cards/cards-locking/${CARDS_VERSION}/slingosgifeature'
  • create visits, make sure the patient chart, visit chart, forms show up correctly
  • also test with a trusted user

@sdumitriu sdumitriu added the urgent Needs to be completed and merged asap label Jun 26, 2025
@sdumitriu sdumitriu force-pushed the CARDS-2558-clinician-dashboard branch 2 times, most recently from b9e478d to 4e5676e Compare June 26, 2025 01:05
@marta- marta- requested review from Hajany and veronikaslc July 3, 2025 15:31
@sdumitriu sdumitriu force-pushed the CARDS-2558-clinician-dashboard branch 2 times, most recently from a63342f to 889c76c Compare July 10, 2025 16:24
@marta- marta- removed the urgent Needs to be completed and merged asap label Aug 7, 2025
@veronikaslc
Copy link
Contributor

Missing some commits from 2609 (by Andrew), local branch with those commits exists.

@acrowthe acrowthe force-pushed the CARDS-2558-clinician-dashboard branch from 889c76c to 907d787 Compare January 22, 2026 19:27
- Remove ClinicianForms as it got renamed and then readded due to a merge issue
- Add expand button back to clinician dashboard subjects and forms table
ids.push(q);
}
});
!questionnaireIds && setQuestionnaireIds(ids);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need a separate state var for questionnaireIds if they are just Object.keys(surveyData)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Morevover, we only reset questionnaireIds when there were none. When questionnaireSetId changes and we reload data, ids become stale forever.

Copy link
Contributor

@marta- marta- Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like code adapted from QuestionnaireSet.jsx. There, we may have introduced questionnaireIds to be used as an effect dependency in an attempt to reduce the frequency of that effect in a different version of the code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really mind this state var, definitely not a top refactoring priority.

Copy link
Contributor

@marta- marta- Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On that note, in a future iteration we should explore actually reusing, rather than adapting, code between clinician-dashboard/Visit.jsx and QuestionnaireSet.jsx, for example via a QuestionnaireSetUtils.

@acrowthe
Copy link
Contributor

Most of the comments should be addressed by the latest commit, haven't had time to double check or thoroughly test yet though

Comment on lines +51 to +62
const fetchToken = () => {
setFetchingLink(true);
fetchWithReLogin(globalLoginDisplay, `${visitURL}.token.html`)
.then((response) => response.ok ? response.text() : Promise.reject(response))
.then((text) => {
let link = window.location.origin + "/Survey.html?auth_token=" + text.trim();
setSurveyLink(link);
copy(link);
})
.catch(() => setError("Could not generate survey link"))
.finally(() => setFetchingLink(false));
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to build in the ability to specify the validity of the token, or at least use a better default. For example, there could be an admin config that allows to specify how many days a token created this way would be valid from the moment of it's creation. This way, the functionality could be used to provide a new link to expired surveys in just one step. It doesn't have to be done in this PR unless it's trivial. Thoughts? @acrowthe @sdumitriu

Comment on lines +476 to +484
let isActionEnabled = (action) => (!!!actionSwitches || !!(actionSwitches[action]?.(data)));

let isDropdnEnabled = () => (
(isEdit && isActionEnabled("subject"))
|| (!isEdit && isActionEnabled("text"))
|| isActionEnabled("delete")
);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would have been a really good time for implementing https://phenotips.atlassian.net/browse/CARDS-1084 . Maybe in the next iteration?

{
key: "",
label: "Subject",
format: (row) => (row.subject ? getHierarchy(row.subject, undefined, undefined, props.extensionURL) : ''),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style: why not extract extensionURL form props? This what we do in the majority of other components, and helps know which props are actually relevant.

…ctions

- Address code review comments: Clean up and handle edge cases
- Clean up extensionUrl definition
@acrowthe acrowthe force-pushed the CARDS-2558-clinician-dashboard branch from efc02d4 to bdb7c76 Compare January 29, 2026 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants