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.

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

add search filter criteria for authorized and unauthorized studies #4825

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 11 additions & 1 deletion src/shared/components/query/QueryStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export class QueryStore {

@computed
get queryParser() {
return new QueryParser(this.referenceGenomes);
return new QueryParser(this.referenceGenomes, this.readPermissions);
}

initialize(urlWithInitialParams?: string) {
Expand Down Expand Up @@ -1511,6 +1511,16 @@ export class QueryStore {
return new Set(referenceGenomes);
}

@computed get readPermissions(): Set<string> {
const studies = Array.from(this.treeData.map_node_meta.keys()).filter(
s => typeof (s as CancerStudy).readPermission !== 'undefined'
Copy link
Collaborator

Choose a reason for hiding this comment

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

(s:CancerStudy)=>s.readPermission===undefined
should work. You don't need typeof anymore. Possible that you will need to cast with as, but not sure.

);
const readPermissions = studies.map(n =>
(n as CancerStudy).readPermission.toString()
Copy link
Collaborator

@alisman alisman Apr 15, 2024

Choose a reason for hiding this comment

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

(n:CancerStudy)=>readPermission (probably already a string?)

);
return new Set(readPermissions);
}

@computed get selectableSelectedStudies() {
return this.selectableSelectedStudyIds
.map(
Expand Down
7 changes: 4 additions & 3 deletions src/shared/components/query/filteredSearch/Phrase.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export class ListPhrase implements Phrase {
for (const fieldName of this.fields) {
let anyPhraseMatch = false;
const fieldValue = study[fieldName];
if (fieldValue) {
if (typeof fieldValue !== 'undefined') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

fieldValue===undefined

for (const phrase of this._phraseList) {
anyPhraseMatch =
anyPhraseMatch ||
Expand Down Expand Up @@ -165,7 +165,8 @@ function matchPhrase(phrase: string, fullText: string) {

/**
* Full match using lowercase
* Need to convert boolean to string before applying lowercase
*/
function matchPhraseFull(phrase: string, fullText: string) {
return fullText.toLowerCase() === phrase.toLowerCase();
function matchPhraseFull(phrase: string, toMatch: boolean | string | number) {
return _.toString(toMatch).toLowerCase() === phrase.toLowerCase();
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably don'tneed to use _ (lodash here). just toMatch.toString().toLowerCase()

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import {
} from 'shared/components/query/filteredSearch/field/CheckboxFilterField';
import { CancerTreeSearchFilter } from 'shared/lib/query/textQueryUtils';
import { ListPhrase } from 'shared/components/query/filteredSearch/Phrase';
import {
toFilterFieldOption,
toFilterFieldValue,
} from 'shared/components/query/filteredSearch/field/FilterFieldOption';

describe('CheckboxFilterField', () => {
describe('createQueryUpdate', () => {
Expand All @@ -12,7 +16,7 @@ describe('CheckboxFilterField', () => {
nodeFields: ['studyId'],
form: {
input: FilterCheckbox,
options: ['a', 'b', 'c', 'd', 'e'],
options: ['a', 'b', 'c', 'd', 'e'].map(toFilterFieldOption),
label: 'Test label',
},
} as CancerTreeSearchFilter;
Expand Down Expand Up @@ -48,7 +52,11 @@ describe('CheckboxFilterField', () => {
it('removes all update when only And', () => {
const checked = dummyFilter.form.options;
const toRemove: ListPhrase[] = [];
const result = createQueryUpdate(toRemove, checked, dummyFilter);
const result = createQueryUpdate(
toRemove,
checked.map(toFilterFieldValue),
dummyFilter
);
expect(result.toAdd?.length).toEqual(0);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@ import {
} from 'shared/lib/query/textQueryUtils';
import { FieldProps } from 'shared/components/query/filteredSearch/field/FilterFormField';
import { ListPhrase } from 'shared/components/query/filteredSearch/Phrase';
import {
FilterFieldOption,
toFilterFieldValue,
} from 'shared/components/query/filteredSearch/field/FilterFieldOption';

export type CheckboxFilterField = {
input: typeof FilterCheckbox;
label: string;
options: string[];
options: FilterFieldOption[];
};

export const FilterCheckbox: FunctionComponent<FieldProps> = props => {
Expand All @@ -40,52 +44,52 @@ export const FilterCheckbox: FunctionComponent<FieldProps> = props => {
}
});
for (const option of options) {
const isChecked = isOptionChecked(option, relevantClauses);
const isChecked = isOptionChecked(option.value, relevantClauses);
if (isChecked) {
checkedOptions.push(option);
checkedOptions.push(option.value);
}
}
return (
<div className="filter-checkbox">
<h5>{props.filter.form.label}</h5>
<div>
{options.map((option: string) => {
const id = `input-${option}`;
let isChecked = checkedOptions.includes(option);
{options.map((option: FilterFieldOption) => {
const id = `input-${option.displayValue}-${option.value}`;
let isChecked = checkedOptions.includes(option.value);
return (
<div
style={{
display: 'inline-block',
padding: '0 1em 0 0',
}}
>
<input
type="checkbox"
id={id}
value={option}
checked={isChecked}
onClick={() => {
isChecked = !isChecked;
updatePhrases(option, isChecked);
const update = createQueryUpdate(
toRemove,
checkedOptions,
props.filter
);
props.onChange(update);
}}
style={{
display: 'inline-block',
}}
/>
<label
htmlFor={id}
style={{
display: 'inline-block',
padding: '0 0 0 0.2em',
}}
>
{option}
<input
type="checkbox"
id={id}
value={option.displayValue}
checked={isChecked}
onClick={() => {
isChecked = !isChecked;
updatePhrases(option.value, isChecked);
const update = createQueryUpdate(
toRemove,
checkedOptions,
props.filter
);
props.onChange(update);
}}
style={{
display: 'inline-block',
}}
/>{' '}
{option.displayValue}
</label>
</div>
);
Expand Down Expand Up @@ -152,7 +156,8 @@ export function createQueryUpdate(
toAdd = [];
} else if (onlyNot || moreAnd) {
const phrase = options
.filter(o => !optionsToAdd.includes(o))
.filter(o => !optionsToAdd.includes(o.value))
.map(toFilterFieldValue)
.join(FILTER_VALUE_SEPARATOR);

toAdd = [new NotSearchClause(createListPhrase(prefix, phrase, fields))];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export type FilterFieldOption = {
value: string;
displayValue: string;
};

export function toFilterFieldOption(option: string) {
return { value: option, displayValue: option };
}

export function toFilterFieldValue(option: FilterFieldOption) {
return option.value;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,22 @@ import { SearchClause } from 'shared/components/query/filteredSearch/SearchClaus
import { Phrase } from 'shared/components/query/filteredSearch/Phrase';
import './ListFormField.scss';
import { toQueryString } from 'shared/lib/query/textQueryUtils';
import { FilterFieldOption } from 'shared/components/query/filteredSearch/field/FilterFieldOption';

export type ListFilterField = {
label: string;
input: typeof FilterList;
options: string[];
options: FilterFieldOption[];
};

export const FilterList: FunctionComponent<FieldProps> = props => {
const form = props.filter.form as ListFilterField;
const allPhrases = toUniquePhrases(props.query);
const queryString = toQueryString(props.query);
return (
<div className="filter-list">
<h5>{props.filter.form.label}</h5>
{form.options.map(option => {
const update = props.parser.parseSearchQuery(option);
const update = props.parser.parseSearchQuery(option.value);
return (
<li className="dropdown-item">
<a
Expand All @@ -32,7 +32,7 @@ export const FilterList: FunctionComponent<FieldProps> = props => {
});
}}
>
{option}
{option.displayValue}
</a>
</li>
);
Expand Down
58 changes: 51 additions & 7 deletions src/shared/components/unknownStudies/UnknownStudiesWarning.tsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,70 @@
import * as React from 'react';
import { observer } from 'mobx-react';
import { Collapse } from 'react-collapse';
import classnames from 'classnames';

@observer
export default class UnknownStudiesWarning extends React.Component<
{ ids: String[] },
{ studiesCollapsed: boolean },
{}
> {
constructor(props: { ids: String[] }) {
super(props);
this.state = {
Copy link
Collaborator

@alisman alisman Apr 15, 2024

Choose a reason for hiding this comment

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

we try to use mobx state instead of react state. for consistency. just put it as an observable property on the component

studiesCollapsed: true
};
}

toggleStudiesCollapse = () => {
this.setState(prevState => ({
studiesCollapsed: !prevState.studiesCollapsed
}));
}

render() {
const { studiesCollapsed } = this.state;
if (this.props.ids.length > 0) {
return (
<div
className="alert alert-danger"
data-test="unkown-study-warning"
style={{ marginBottom: 0 }}
>
<i className="fa fa-exclamation-triangle"></i> The following
studies do not exist or you do not have access to them:
<ul style={{ margin: '10px 0' }}>
{this.props.ids.map(id => (
<li>{id}</li>
))}
</ul>

<div
style={{
width: '100%',
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably don't need width:100%

display: 'flex',
alignItems: 'center',
}}
onClick={() => this.toggleStudiesCollapse()}
>
<i className="fa fa-exclamation-triangle"></i> The following
studies do not exist or you do not have access to them.
<span style={{ float: 'right' }}>
{studiesCollapsed ? (
<i
className={classnames(
'fa fa-chevron-down'
)}
/>
) : (
<i
className={classnames(
'fa fa-chevron-up'
)}
/>
)}
</span>
</div>
<Collapse isOpened={!studiesCollapsed}>
<ul style={{ margin: '10px 0' }}>
{this.props.ids.map(id => (
<li>{id}</li>
))}
</ul>
</Collapse>
Please resubmit your query below.
</div>
);
Expand Down
2 changes: 1 addition & 1 deletion src/shared/lib/query/QueryParser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { QueryParser } from 'shared/lib/query/QueryParser';
import { StringPhrase } from 'shared/components/query/filteredSearch/Phrase';

describe('QueryParser', () => {
const parser = new QueryParser(new Set<string>());
const parser = new QueryParser(new Set<string>(), new Set<string>());
const referenceGenomeFields = parser.searchFilters.find(
f => f.phrasePrefix === 'reference-genome'
)!.nodeFields;
Expand Down
28 changes: 25 additions & 3 deletions src/shared/lib/query/QueryParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@ import {
ListPhrase,
Phrase,
} from 'shared/components/query/filteredSearch/Phrase';
import { toFilterFieldOption } from 'shared/components/query/filteredSearch/field/FilterFieldOption';

export class QueryParser {
/**
* Can be extended with additional search filters
*/
private readonly _searchFilters: CancerTreeSearchFilter[];

constructor(referenceGenomes: Set<string>) {
constructor(referenceGenomes: Set<string>, readPermissions: Set<string>) {
this._searchFilters = [
/**
* Example queries:
Expand All @@ -38,7 +39,7 @@ export class QueryParser {
input: FilterList,
options: ServerConfigHelpers.skin_example_study_queries(
getServerConfig()!.skin_example_study_queries || ''
),
).map(toFilterFieldOption),
},
},
/**
Expand All @@ -49,10 +50,31 @@ export class QueryParser {
nodeFields: ['referenceGenome'],
form: {
input: FilterCheckbox,
options: [...referenceGenomes],
options: [...referenceGenomes].map(toFilterFieldOption),
label: 'Reference genome',
},
},
/**
* Show Authorized Studies
*/
{
phrasePrefix: 'authorized',
nodeFields: ['readPermission'],
form: {
input: FilterCheckbox,
options:
readPermissions.size > 1
? [
{ value: 'true', displayValue: 'Authorized' },
{
value: 'false',
displayValue: 'Unauthorized',
},
]
: [],
label: 'Controlled access',
},
},
];
}

Expand Down
2 changes: 1 addition & 1 deletion src/shared/lib/query/textQueryUtils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { QueryParser } from 'shared/lib/query/QueryParser';
import { StringPhrase } from 'shared/components/query/filteredSearch/Phrase';

describe('textQueryUtils', () => {
const parser = new QueryParser(new Set<string>());
const parser = new QueryParser(new Set<string>(), new Set<string>());
const referenceGenomeFields = parser.searchFilters.find(
f => f.phrasePrefix === 'reference-genome'
)!.nodeFields;
Expand Down
Loading