Independent display logic for sample treatments chart #3455
Conversation
Luke-Sikina
changed the title
| @@ -6924,6 +6926,17 @@ export class StudyViewPageStore { | |||
| }); | |||
| } | |||
|
|
|||
| @computed | |||
There was a problem hiding this comment.
why do we put these in computed (instead of treating them like patientTreatments below as an instance property)? @Luke-Sikina you might just have copied an existing thing, but i'm curious.
There was a problem hiding this comment.
I'm not going to resolve this comment just in case it's still relevant, but I think these do need to be computed now, as this.studyIds is an observable.
There was a problem hiding this comment.
yes but they would be computed either way. i'm just curious why it is we treat these different than we normally treate remoteData, at least in other stores. probably there isn't an answer.
normal way (note that this will have same computed behavior):
readonly localPro = remotedata(...)
| return remoteData({ | ||
| invoke: () => { | ||
| return defaultClient.getContainsSampleTreatmentDataUsingPOST({ | ||
| studyViewFilter: this.initialFilters, |
There was a problem hiding this comment.
Question, isn't studyIds not enough to check they have treatments data or not? do we need studyViewFilter?
Luke-Sikina
force-pushed
the
sample-treatments-display-logic
branch
3 times, most recently
from
584810d
to
e874082
Compare
- Sample treatments sometimes should not be shown when patient treatments are shown - This PR uses a new API endpoint to determine when sample treatments should be shown - Also, unknown treatments are no longer shows on the frontend - Switched from passing a filter object to passing a list a study ids to the display treatment endpoints
Luke-Sikina
force-pushed
the
sample-treatments-display-logic
branch
from
e874082
to
47b18f4
Compare
Backend changes here: cBioPortal/cbioportal#7960
Fix cBioPortal/cbioportal#7928
Describe changes proposed in this pull request:
are shown