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.

Sign up for GitHub

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

Fix issue for unauthorized studies global message #4138

Closed
wants to merge 1 commit into from
Closed

Fix issue for unauthorized studies global message #4138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/shared/components/query/QueryContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { StudySelectorStats } from 'shared/components/query/StudySelectorStats';
import $ from 'jquery';
import { serializeEvent } from 'shared/lib/tracking';
import { ModifyQueryParams } from 'pages/resultsView/ResultsViewPageStore';
import { getServerConfig } from 'config/config';

interface QueryContainerProps {
store: QueryStore;
Expand Down Expand Up @@ -202,11 +203,13 @@ export default class QueryContainer extends React.Component<
</div>
)}

{this.store.unknownStudyIds.isComplete && (
<UnknownStudiesWarning
ids={this.store.unknownStudyIds.result}
/>
)}
{this.store.unknownStudyIds.isComplete &&
!!getServerConfig()
.skin_home_page_show_unauthorized_studies === false && (
<UnknownStudiesWarning
ids={this.store.unknownStudyIds.result}
/>
)}

{this.store.forDownloadTab && (
<div className={'alert alert-danger'} role="alert">
Expand Down
5 changes: 4 additions & 1 deletion src/shared/components/query/QueryStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import {
import { isMixedReferenceGenome } from 'shared/lib/referenceGenomeUtils';
import { getSuffixOfMolecularProfile } from 'shared/lib/molecularProfileUtils';
import { VirtualStudy } from 'shared/api/session-service/sessionServiceModels';
import { isQueriedStudyAuthorized } from 'pages/studyView/StudyViewUtils';

// interface for communicating
export type CancerStudyQueryUrlParams = {
Expand Down Expand Up @@ -832,7 +833,9 @@ export class QueryStore {
let result: { [studyId: string]: string[] } = {};

_.each(this.physicalStudiesSet.result, (study, studyId) => {
result[studyId] = [studyId];
if (isQueriedStudyAuthorized(study)) {
result[studyId] = [studyId];
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

it seems at least possible that an unauthorized study could make it's way into a virtual study from a user that has privelages to it. see below where we add studyids from virtual studies. should we do same check? or perhaps we should just fail so that a user will know that there is a "problem" with virtual study.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't add it because the unauthorized studies feature shouldn't apply to the virtual studies. Also in the StudyViewPageStore, I see that the isQueriedStudyAuthorized is added only to the physical studies.

Copy link
Contributor

Choose a reason for hiding this comment

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

@alisman

it seems at least possible that an unauthorized study could make it's way into a virtual study from a user that has privelages to it.

This is already the case when not running with the unauthorized studies feature. Or am I missing something in your feedback?

});

_.each(
Expand Down
28 changes: 15 additions & 13 deletions src/shared/components/studyTagsTooltip/StudyTagsTooltip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ export type StudyInfoOverlayTooltipProps = {
iconType: IconType;
};

function addHTMLDescription(description: string) {
return { __html: description };
}

@observer
class StudyInfoOverlay extends React.Component<
StudyInfoOverlayTooltipProps,
Expand All @@ -77,16 +81,12 @@ class StudyInfoOverlay extends React.Component<
makeObservable(this);
}

addHTMLDescription(description: string) {
return { __html: description };
}

render() {
let overlay: any = '';
if (this.props.isVirtualStudy) {
overlay = (
<div
dangerouslySetInnerHTML={this.addHTMLDescription(
dangerouslySetInnerHTML={addHTMLDescription(
this.props.studyDescription
)}
/>
Expand All @@ -99,7 +99,7 @@ class StudyInfoOverlay extends React.Component<
.length;
const description = (
<div
dangerouslySetInnerHTML={this.addHTMLDescription(
dangerouslySetInnerHTML={addHTMLDescription(
this.props.studyDescription
)}
/>
Expand All @@ -122,7 +122,8 @@ class StudyInfoOverlay extends React.Component<
const message = replaceJsonPathPlaceholders(
getServerConfig()
.skin_home_page_unauthorized_studies_global_message,
this.studyMetadata.result
this.studyMetadata.result,
this.props.studyId
);

// if the placeholders couldn't be replaced, then show default global message
Expand All @@ -131,7 +132,7 @@ class StudyInfoOverlay extends React.Component<
) : (
<div
style={{ maxWidth: 300 }}
dangerouslySetInnerHTML={this.addHTMLDescription(
dangerouslySetInnerHTML={addHTMLDescription(
message.toString()
)}
/>
Expand All @@ -153,7 +154,7 @@ export default class StudyTagsTooltip extends React.Component<
> {
renderTooltip() {
return (
<DefaultTooltip
<DefaultTooltip
mouseEnterDelay={this.props.mouseEnterDelay}
placement={this.props.placement}
overlay={
Expand All @@ -162,12 +163,13 @@ export default class StudyTagsTooltip extends React.Component<
getServerConfig()
.skin_home_page_unauthorized_studies_global_message
) === false ? (
<div className={styles.tooltip}>
{
<div
className={styles.tooltip}
dangerouslySetInnerHTML={addHTMLDescription(
getServerConfig()
.skin_home_page_unauthorized_studies_global_message
}
</div>
)}
/>
) : (
<StudyInfoOverlay
studyDescription={this.props.studyDescription}
Expand Down
28 changes: 18 additions & 10 deletions src/shared/lib/JsonPathUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,33 @@
export function hasJsonPathPlaceholders(message: String) {
const jp = require('jsonpath');

export function hasJsonPathPlaceholders(message: string) {
if (message.match(/[{][$][^}]*[}]/g) === null) return false;
return true;
}

export function replaceJsonPathPlaceholders(
message: String,
studyMetadata: any
message: string,
studyMetadata: any,
studyId: string
) {
let placeholders = message.match(/[{][$][^}]*[}]/g);
let placeholdersReplaced: boolean = true;
if (placeholders !== null) {
let jp = require('jsonpath');
placeholders.forEach(placeholder => {
let placeholderReplaceValue = jp.query(
studyMetadata,
placeholder.replace(/["'{}]/g, '')
);
if (placeholderReplaceValue.length === 0)
let placeholderReplaceValue = '';
dianab0 marked this conversation as resolved.
Show resolved Hide resolved
if (placeholder === '{$.studyId}') {
placeholderReplaceValue = studyId;
} else {
placeholderReplaceValue = jp.query(
studyMetadata,
placeholder.replace(/["'{}]/g, '')
);
}
if (placeholderReplaceValue.length === 0) {
placeholdersReplaced = false;
else
} else {
message = message.replace(placeholder, placeholderReplaceValue);
}
});
}
return message;
Expand Down