Add profile filtering when adding Generic Entity IDs in Oncoprint #4596
Conversation
oplantalech
force-pushed
the
fix_oncoprint_generic_assay_bug
branch
from
914e782 to
a225a6c
Compare
There was a problem hiding this comment.
@oplantalech Thanks again for fixing this issue, it looks good to me, just had some minor comments.
| profileId | ||
| ); | ||
| } | ||
|
|
||
| @computed | ||
| private get genericAssayTabs() { | ||
| let tabs = []; | ||
| if (this.isGenericAssayDataComplete && this.showGenericAssayTabs) { |
There was a problem hiding this comment.
This is my fault, it looks like I forgot to add necessary checks in isGenericAssayDataComplete function.
Could you add this.props.store.genericAssayEntitiesGroupByMolecularProfileId.isComplete and this.selectedGenericAssayProfileIdByType.size > 0 into isGenericAssayDataComplete function?
There was a problem hiding this comment.
Somehow adding this.selectedGenericAssayProfileIdByType.size > 0 does not show any Generic Assays, so I've only added the condition this.props.store.genericAssayEntitiesGroupByMolecularProfileId.isComplete
oplantalech
changed the title
|
@dippindots Thank you for your review! I've implemented all your suggestions except one small thing (see comment). When it's all ready let me know and I'll squash all the commits before merging |
| constructor(props: IAddTrackProps) { | ||
| super(props); | ||
| makeObservable(this); | ||
| // Using the first profile as default profile option to initialize selectedGenericAssayProfileIdByType | ||
| this.genericAssayEntityOptionsReaction = when( |
There was a problem hiding this comment.
not sure why this reaction is necessary
There was a problem hiding this comment.
i would think that you can accomplish default by checking for the selected type as part of the getter and if it is undefined, just selecting the first type
There was a problem hiding this comment.
@alisman Thanks for this comment. I've simplified the code deleting the reaction
| @@ -60,6 +67,13 @@ export const COUNT_PADDING_WIDTH = 17; | |||
| export default class TracksMenu extends React.Component<IAddTrackProps, {}> { | |||
| @observable tabId: Tab | string = Tab.CLINICAL; | |||
|
|
|||
| private genericAssayEntityOptionsReaction: IReactionDisposer; | |||
|
|
|||
| private selectedGenericAssayProfileIdByType = observable.map< | |||
There was a problem hiding this comment.
is it possible to have multiple profiles selected?
There was a problem hiding this comment.
You always have one profile selected per Generic Assay Type:
So in this example there are three Generic Assay Types (Generic Assay Patient Test, Treatment Response and Mutational Signature) and both have one profile selected. You cannot have more than one profile selected per Generic Assay Type.
oplantalech
force-pushed
the
fix_oncoprint_generic_assay_bug
branch
from
0d891a6 to
a546714
Compare
|
@alisman Can we merge? |
|
@oplantalech Merged! Really appreciate you fixing this issue! |
Fix cBioPortal/cbioportal#10078