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

AG-1328: improve handling of target nominations table display strings #1302

Merged
merged 3 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,22 @@ import { HttpClientTestingModule } from '@angular/common/http/testing';
// -------------------------------------------------------------------------- //
import { GeneNominatedTargetsComponent } from './';
import { ApiService, HelperService } from '../../../../core/services';
import { Gene, GenesResponse } from '../../../../models';
import { geneMock1, targetNominationMock1 } from '../../../../testing/gene-mocks';
import { of } from 'rxjs';

// -------------------------------------------------------------------------- //
// Tests
// -------------------------------------------------------------------------- //
describe('Component: Gene Nominated Targets', () => {
let fixture: ComponentFixture<GeneNominatedTargetsComponent>;
let component: GeneNominatedTargetsComponent;
let element: HTMLElement;
let mockApiService: ApiService;

const COLUMN_INDICES = {
'cohort_study': 4
};

beforeEach(async () => {
await TestBed.configureTestingModule({
Expand All @@ -29,10 +38,120 @@ describe('Component: Gene Nominated Targets', () => {
beforeEach(async () => {
fixture = TestBed.createComponent(GeneNominatedTargetsComponent);
component = fixture.componentInstance;
fixture.detectChanges();
});

const setUp = (genes: Gene[]) => {
const genesResponse: GenesResponse = {
items: genes
};
mockApiService = TestBed.inject(ApiService);
spyOn(mockApiService, 'getNominatedGenes').and.returnValue(
of(genesResponse)
);
fixture.detectChanges();
element = fixture.nativeElement;

expect(mockApiService.getNominatedGenes).toHaveBeenCalled();

const table = element.querySelector('table');
expect(table).not.toBeNull();

const rows = Array.from(
table?.querySelectorAll('tbody tr') || []
) as HTMLTableRowElement[];

return { rows };
};
Comment on lines +43 to +64
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Create setUp method so that each test can specify which genes should be returned by the mock API service


it('should create', () => {
setUp([geneMock1]);
expect(component).toBeTruthy();
});

it('should not show null study values', () => {
const gene: Gene = {
...geneMock1,
target_nominations: [
{ ...targetNominationMock1, study: null },
{ ...targetNominationMock1, study: 'XYZ Study, ABC Study' },
{ ...targetNominationMock1, study: '' },
{ ...targetNominationMock1, study: 'Study 123, Study 456' },
],
};
const { rows } = setUp([gene]);
expect(rows.length).toBe(1);

const cols = rows[0].cells;
expect(cols.length).toBeGreaterThan(COLUMN_INDICES.cohort_study);

expect(cols[COLUMN_INDICES.cohort_study].textContent?.trim()).toEqual(
'ABC Study, Study 123, Study 456, XYZ Study'
);
});

it('should display sorted, unique study values', () => {
const expectedStudyString = 'ACT, Banner, BLSA, Kronos, MSBB, ROSMAP';
const { rows } = setUp([geneMock1]);

expect(rows.length).toBe(1);

const cols = rows[0].cells;
expect(cols.length).toBeGreaterThan(COLUMN_INDICES.cohort_study);

expect(cols[COLUMN_INDICES.cohort_study].textContent?.trim()).toEqual(
expectedStudyString
);
});

it('should correctly flatten comma separated arrays', () => {
setUp([]);

expect(component.commaFlattenArray([])).toEqual([]);

expect(
component.commaFlattenArray(['ACT, BLSA, Banner', 'ACT, BLSA, Banner'])
).toEqual(['ACT', 'BLSA', 'Banner', 'ACT', 'BLSA', 'Banner']);

expect(component.commaFlattenArray(['A, B, C', 'D', 'E, F, G, H'])).toEqual(
['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H']
);

expect(component.commaFlattenArray(['A', 'B', 'C'])).toEqual([
'A',
'B',
'C',
]);
});

it('should correctly format display values', () => {
setUp([]);

expect(
component.getCommaSeparatedStringOfUniqueSortedValues([])
).toEqual('');

expect(
component.getCommaSeparatedStringOfUniqueSortedValues([
'ACT',
'BLSA',
'Banner',
'ACT',
'BLSA',
'Banner',
])
).toEqual('ACT, Banner, BLSA');

expect(
component.getCommaSeparatedStringOfUniqueSortedValues([
'Z',
'Y',
'X',
'A',
'B',
'C',
'B',
'C',
])
).toEqual('A, B, C, X, Y, Z');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ export class GeneNominatedTargetsComponent implements OnInit {
// First map all entries nested in the data to a new array
if (de.target_nominations?.length) {
teamsArray = de.target_nominations.map((nt: TargetNomination) => nt.team);
studyArray = de.target_nominations.map(
(nt: TargetNomination) => nt.study
);
studyArray = de.target_nominations
.map((nt: TargetNomination) => nt.study)
.filter((item) => Boolean(item)) as string[];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix for AG-1328 -- drop study values that are null or empty strings. See Cohort Study column below.

develop:
AG-1328_study_develop

feature:
AG-1328_study_feature

Copy link
Contributor

Choose a reason for hiding this comment

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

as a suggestion, perhaps we can create a helper method to remove null/empty values .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in the next commit

programsArray = de.target_nominations.map(
(nt: TargetNomination) => nt.source
);
Expand All @@ -120,37 +120,14 @@ export class GeneNominatedTargetsComponent implements OnInit {
inputDataArray = this.commaFlattenArray(inputDataArray);

// Populate targetNomination display fields
de.teams_display_value = '';
if (teamsArray.length) {
de.teams_display_value = teamsArray
.filter(this.getUnique)
.sort((a: string, b: string) => a.localeCompare(b))
.join(', ');
}
sagely1 marked this conversation as resolved.
Show resolved Hide resolved

de.study_display_value = '';
if (teamsArray.length) {
sagely1 marked this conversation as resolved.
Show resolved Hide resolved
de.study_display_value = studyArray
.filter(this.getUnique)
.sort((a: string, b: string) => a.localeCompare(b))
.join(', ');
}

de.programs_display_value = '';
if (programsArray.length) {
de.programs_display_value = programsArray
.filter(this.getUnique)
.sort((a: string, b: string) => a.localeCompare(b))
.join(', ');
}

de.input_data_display_value = '';
if (inputDataArray.length) {
de.input_data_display_value = inputDataArray
.filter(this.getUnique)
.sort((a: string, b: string) => a.localeCompare(b))
.join(', ');
}
de.teams_display_value =
this.getCommaSeparatedStringOfUniqueSortedValues(teamsArray);
de.study_display_value =
this.getCommaSeparatedStringOfUniqueSortedValues(studyArray);
de.programs_display_value =
this.getCommaSeparatedStringOfUniqueSortedValues(programsArray);
de.input_data_display_value =
this.getCommaSeparatedStringOfUniqueSortedValues(inputDataArray);

de.initial_nomination_display_value = initialNominationArray.length
? Math.min(...initialNominationArray)
Expand Down Expand Up @@ -189,27 +166,29 @@ export class GeneNominatedTargetsComponent implements OnInit {
return self.indexOf(value) === index;
}

commaFlattenArray(array: any[]): any[] {
const finalArray: any[] = [];
if (array.length) {
array.forEach((t) => {
if (t) {
const i = t.indexOf(', ');
if (i > -1) {
const tmpArray = t.split(', ');
finalArray.push(tmpArray[0]);
finalArray.push(tmpArray[1]);
} else {
finalArray.push(t);
}
} else {
finalArray.push('');
}
});
array = finalArray;
}
commaFlattenArray(array: string[]): string[] {
sagely1 marked this conversation as resolved.
Show resolved Hide resolved
const finalArray: string[] = [];
array.forEach((t) => {
const i = t.indexOf(', ');
if (i > -1) {
const tmpArray = t.split(', ');
tmpArray.forEach((val) => finalArray.push(val));
sagely1 marked this conversation as resolved.
Show resolved Hide resolved
} else {
finalArray.push(t);
}
});
return finalArray;
}

return array;
getCommaSeparatedStringOfUniqueSortedValues(inputArray: string[]) {
let display_value = '';
if (inputArray.length) {
display_value = inputArray
.filter(this.getUnique)
.sort((a: string, b: string) => a.localeCompare(b))
.join(', ');
}
return display_value;
}

onSearch(event: any) {
Expand Down
2 changes: 1 addition & 1 deletion src/app/models/genes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface TargetNomination {
predicted_therapeutic_direction: string;
data_used_to_support_target_selection: string;
data_synapseid: string;
study: string;
study: string | null;
input_data: string;
validation_study_details: string;
initial_nomination: number;
Expand Down
20 changes: 19 additions & 1 deletion src/app/testing/gene-mocks.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,24 @@
/* eslint-disable */

import { Gene, GCTGene } from '../models';
import { Gene, GCTGene, TargetNomination } from '../models';

export const targetNominationMock1: TargetNomination = {
source: 'Treat-AD',
team: 'Emory-Sage-SGC',
rank: '7',
hgnc_symbol: 'MSN',
target_choice_justification:
'MSN was identified as a potential driver protein based on protein coexpression analysis. The group of proteins coexpressed with MSN is conserved across the 3 datasets considered, is enriched for inflammatory processes, and for protein products of genes near loci previously associated with AD risk. MSN has increased abundance in AD across all 3 cohorts examined, and progressively increases in asymptomatic (prodromal) AD to symptomatic AD, and also correlates with both hallmark AD pathology scores (CERAD for amyloid burden; and Braak for Tau extent of spread). MSN is highly expressed as a marker of disease-associated microglia and/or endothelial cell types.',
predicted_therapeutic_direction:
'Antagonism predicted to reduce disease progression. Phosphorylation downstream of Rho/Rock influences actin, focal adhesion binding; may have redundancy with EZR and RDX, complicating targeting. MSN-directed therapeutics that improve microglial motility and/or phagocytosis competence would reduce abeta/amyloid plaque burden.',
data_used_to_support_target_selection:
'Discovery quantitative proteomics of FrCx \r\n WPCNA of multiple and consensus cohorts\r\n ANOVA',
data_synapseid: 'syn17008058',
study: 'ACT, BLSA, Banner',
input_data: 'Protein',
validation_study_details: 'validation studies ongoing',
initial_nomination: 2018,
};

export const geneMock1: Gene = {
_id: '628ea1be0e8d04279fdbaa26',
Expand Down
Loading