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

Jump to bottom

Improve search of authorized and unauthorized studies #4521

Closed
wants to merge 17 commits into from
Closed

Improve search of authorized and unauthorized studies #4521

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
a88c674
fix 9915 improve search of authorized studies
jagnathan Feb 15, 2023
dcc0c0d
fix 9915 improve search of authorized studies
jagnathan Feb 15, 2023
450fa29
Merge branch 'fix-9915-improved-finability' of https://github.com/jag…
jagnathan Feb 15, 2023
5847a79
Use _.toString in matchPhraseFull, and specify type
Feb 16, 2023
d7895ea
Unnest fieldName check in Phrase.match
Feb 16, 2023
8162310
Introduce FilterFieldOption type that includes a displayValue
Feb 16, 2023
54bd4a9
Merge pull request #1 from BasLee/fix-9915-improved-finability
jagnathan Feb 16, 2023
ccdd9c5
Update QueryParser.ts
jagnathan Feb 17, 2023
6a76f28
Update CheckboxFilterField.tsx
jagnathan Mar 3, 2023
fc24858
Modify the Select all checkbox to select only authorized studies
jagnathan Mar 21, 2023
c06b4f9
Update checkbox label text
jagnathan Mar 29, 2023
fff8f44
fix all Authorized or Unauthorized studies scenario
jagnathan Apr 5, 2023
90e7b03
Updated logic and refactoring
jagnathan Apr 21, 2023
2361d73
fix 9915 improve search of authorized studies
jagnathan Feb 15, 2023
a4e8aa6
Updated logic and refactoring
jagnathan Apr 21, 2023
f352643
Update Phrase.tsx
jagnathan Apr 25, 2023
222e1f4
Merge branch 'fix-9915-improved-finability' of https://github.com/jag…
jagnathan Apr 25, 2023
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
143 changes: 104 additions & 39 deletions src/shared/components/query/CancerStudySelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ export default class CancerStudySelector extends React.Component<
onCheckAllFiltered: () => {
this.logic.mainView.toggleAllFiltered();
},
onCheckAuthorizedFiltered: () => {
this.logic.mainView.toggleAllAuthorizedAndFiltered();
},
onClearFilter: () => {
this.store.setSearchText('');
},
Expand Down Expand Up @@ -193,6 +196,9 @@ export default class CancerStudySelector extends React.Component<
shownAndSelectedStudies,
} = this.logic.mainView.getSelectionReport();

const shownAndAuthorizedStudies = shownStudies.filter(study => {
return study.readPermission;
});
const quickSetButtons = this.logic.mainView.quickSelectButtons(
getServerConfig().skin_quick_select_buttons
);
Expand Down Expand Up @@ -307,45 +313,104 @@ export default class CancerStudySelector extends React.Component<
</div>
</Then>
<Else>
<label>
<input
type="checkbox"
data-test="selectAllStudies"
style={{ top: -2 }}
onClick={
this.handlers
.onCheckAllFiltered
}
checked={
shownAndSelectedStudies.length ===
shownStudies.length
}
/>
<strong>
{shownAndSelectedStudies.length ===
shownStudies.length
? `Deselect all listed studies ${
shownStudies.length <
this.store
.cancerStudies
.result.length
? 'matching filter'
: ''
} (${
shownStudies.length
})`
: `Select all listed studies ${
shownStudies.length <
this.store
.cancerStudies
.result.length
? 'matching filter'
: ''
} (${
shownStudies.length
})`}
</strong>
</label>
<If
condition={
getServerConfig()
.skin_home_page_show_unauthorized_studies ===
false
}
>
<Then>
<label>
<input
type="checkbox"
data-test="selectAllStudies"
style={{ top: -2 }}
onClick={
this.handlers
.onCheckAllFiltered
}
checked={
shownAndSelectedStudies.length ===
shownStudies.length
}
/>
<strong>
{shownAndSelectedStudies.length ===
shownStudies.length
? `Deselect all listed studies ${
shownStudies.length <
this.store
.cancerStudies
.result
.length
? 'matching filter'
: ''
} (${
shownStudies.length
})`
: `Select all listed studies ${
shownStudies.length <
this.store
.cancerStudies
.result
.length
? 'matching filter'
: ''
} (${
shownStudies.length
})`}
</strong>
</label>
</Then>
<Else>
<label>
<input
type="checkbox"
data-test="selectAuthorizedStudies"
style={{ top: -2 }}
onClick={
this.handlers
.onCheckAuthorizedFiltered
}
checked={
shownAndSelectedStudies.length ===
shownAndAuthorizedStudies.length &&
shownAndAuthorizedStudies.length >
0
}
/>
<strong>
{shownAndSelectedStudies.length ===
shownAndAuthorizedStudies.length &&
shownAndAuthorizedStudies.length >
0
jagnathan marked this conversation as resolved.
Show resolved Hide resolved
? `Deselect all authorized studies ${
shownStudies.length <
this.store
.cancerStudies
.result
.length
jagnathan marked this conversation as resolved.
Show resolved Hide resolved
? 'matching filter'
: ''
} (${
shownAndAuthorizedStudies.length
})`
: `Select all authorized studies ${
shownStudies.length <
this.store
.cancerStudies
.result
.length
? 'matching filter'
: ''
} (${
shownAndAuthorizedStudies.length
})`}
</strong>
</label>
</Else>
</If>
</Else>
</If>
</If>
Expand Down
11 changes: 10 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 @@ -129,7 +129,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 @@ -1450,6 +1450,15 @@ 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' );
console.log(studies);
Copy link
Member

Choose a reason for hiding this comment

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

We can delete this log

const readPermissions = studies
.map(n => (!!((n as CancerStudy).readPermission)).toString())
.filter(n => !!n);
jagnathan marked this conversation as resolved.
Show resolved Hide resolved
return new Set(readPermissions);
}

@computed get selectableSelectedStudies() {
return this.selectableSelectedStudyIds
.map(
Expand Down
36 changes: 36 additions & 0 deletions src/shared/components/query/StudyListLogic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,42 @@ export class FilteredCancerTreeView {
);
}

@action toggleAllAuthorizedAndFiltered() {
const {
selectableSelectedStudyIds,
selectableSelectedStudies,
shownStudies,
shownAndSelectedStudies,
} = this.getSelectionReport();

let updatedSelectableSelectedStudyIds: string[] = [];
const shownAndAuthorizedStudies = shownStudies.filter(study => {
return study.readPermission;
});
if (
shownAndAuthorizedStudies.length === shownAndSelectedStudies.length
) {
// deselect
updatedSelectableSelectedStudyIds = _.without(
this.store.selectableSelectedStudyIds,
...shownAndAuthorizedStudies.map(
(study: CancerStudy) => study.studyId
)
Comment on lines +421 to +423
Copy link
Member

Choose a reason for hiding this comment

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

This also shows twice, maybe we can create a constant variable for it

const shownAndAuthorizedStudyIds = shownAndAuthorizedStudies.map(
    (study: CancerStudy) => study.studyId
);

);
} else {
updatedSelectableSelectedStudyIds = _.union(
this.store.selectableSelectedStudyIds,
shownAndAuthorizedStudies.map(
(study: CancerStudy) => study.studyId
)
);
}

this.store.selectableSelectedStudyIds = updatedSelectableSelectedStudyIds.filter(
id => !_.includes(this.store.deletedVirtualStudies, id)
);
}

@action selectAllMatchingStudies(match: string | string[]) {
const {
selectableSelectedStudyIds,
Expand Down
18 changes: 10 additions & 8 deletions src/shared/components/query/filteredSearch/Phrase.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,14 @@ export class ListPhrase implements Phrase {
public match(study: CancerTreeNode): boolean {
let anyFieldMatch = false;
for (const fieldName of this.fields) {
let anyPhraseMatch = false;
if (!_.has(study, fieldName)) {
continue;
}
const fieldValue = (study as any)[fieldName];
if (fieldValue) {
for (const phrase of this._phraseList) {
anyPhraseMatch =
anyPhraseMatch || matchPhraseFull(phrase, fieldValue);
}
let anyPhraseMatch = false;
for (const phrase of this._phraseList) {
anyPhraseMatch =
anyPhraseMatch || matchPhraseFull(phrase, fieldValue);
}
anyFieldMatch = anyFieldMatch || anyPhraseMatch;
}
Expand Down Expand Up @@ -162,7 +163,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();
}
12 changes: 10 additions & 2 deletions src/shared/components/query/filteredSearch/field/CheckboxFilterField.spec.ts
View file
Open in desktop
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
Loading