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

Fix 9915 improving search filter by authorization #4985

Open
wants to merge 6 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 @@ -1573,6 +1573,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'
);
const readPermissions = studies.map(n =>
(n as CancerStudy).readPermission.toString()
);
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 (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 toMatch.toString().toLowerCase() === phrase.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
60 changes: 53 additions & 7 deletions src/shared/components/unknownStudies/UnknownStudiesWarning.tsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,72 @@
import * as React from 'react';
import { observer } from 'mobx-react';
import { Collapse } from 'react-collapse';
import classnames from 'classnames';
import { action, computed, observable, makeObservable } from 'mobx';


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

@observable studiesCollapsed = false;

@action.bound
toggleStudiesCollapse() {
this.studiesCollapsed = !this.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={{
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