From a21839bcf68f11187b902ddb5ff245c2aa1f8f4d Mon Sep 17 00:00:00 2001 From: sagely1 <114952739+sagely1@users.noreply.github.com> Date: Tue, 16 Apr 2024 14:01:15 -0700 Subject: [PATCH 1/3] AG-1399 fix invalid ensg in url from hanging --- src/app/core/services/api.service.ts | 2 +- .../gene-details/gene-details.component.ts | 65 ++++++++++--------- .../gene-similar/gene-similar.component.ts | 8 ++- .../features/genes/services/gene.service.ts | 8 ++- src/app/testing/api-service-stub.ts | 2 +- src/app/testing/gene-service-stub.ts | 2 +- 6 files changed, 48 insertions(+), 39 deletions(-) diff --git a/src/app/core/services/api.service.ts b/src/app/core/services/api.service.ts index 401098ef0..0420af619 100644 --- a/src/app/core/services/api.service.ts +++ b/src/app/core/services/api.service.ts @@ -50,7 +50,7 @@ export class ApiService { return url; } - getGene(id: string): Observable { + getGene(id: string): Observable { return this.http.get(this.getBaseUrl() + '/api/genes/' + id, { headers: new HttpHeaders(defaultHeaders), }); diff --git a/src/app/features/genes/components/gene-details/gene-details.component.ts b/src/app/features/genes/components/gene-details/gene-details.component.ts index d4b03a220..a37476d95 100644 --- a/src/app/features/genes/components/gene-details/gene-details.component.ts +++ b/src/app/features/genes/components/gene-details/gene-details.component.ts @@ -135,40 +135,45 @@ export class GeneDetailsComponent implements OnInit, AfterViewInit { if (params.get('id')) { this.geneService .getGene(params.get('id') as string) - .subscribe((gene: Gene) => { - this.gene = gene; - - this.panels.forEach((p: Panel) => { - if (p.name == 'nominations' && !this.gene?.total_nominations) { - p.disabled = true; - } else if ( - p.name == 'experimental-validation' && - !this.gene?.experimental_validation?.length - ) { - p.disabled = true; - } else { - p.disabled = false; + .subscribe((gene) => { + if (!gene) { + this.helperService.setLoading(false); + this.router.navigateByUrl('/404-not-found', { skipLocationChange: true }); + } else { + this.gene = gene; + + this.panels.forEach((p: Panel) => { + if (p.name == 'nominations' && !this.gene?.total_nominations) { + p.disabled = true; + } else if ( + p.name == 'experimental-validation' && + !this.gene?.experimental_validation?.length + ) { + p.disabled = true; + } else { + p.disabled = false; + } + }); + + const nominationsPanel = this.panels.find( + (p) => p.name == 'nominations' + ); + if (nominationsPanel) { + nominationsPanel.disabled = !this.gene.total_nominations ? true : false; } - }); - const nominationsPanel = this.panels.find( - (p) => p.name == 'nominations' - ); - if (nominationsPanel) { - nominationsPanel.disabled = !this.gene.total_nominations ? true : false; - } + const experimentalValidationPanel = this.panels.find( + (p) => p.name == 'experimental-validation' + ); + if (experimentalValidationPanel) { + experimentalValidationPanel.disabled = !this.gene + .experimental_validation?.length + ? true + : false; + } - const experimentalValidationPanel = this.panels.find( - (p) => p.name == 'experimental-validation' - ); - if (experimentalValidationPanel) { - experimentalValidationPanel.disabled = !this.gene - .experimental_validation?.length - ? true - : false; + this.helperService.setLoading(false); } - - this.helperService.setLoading(false); }); } diff --git a/src/app/features/genes/components/gene-similar/gene-similar.component.ts b/src/app/features/genes/components/gene-similar/gene-similar.component.ts index 8414a5ad5..0e0c01d8c 100644 --- a/src/app/features/genes/components/gene-similar/gene-similar.component.ts +++ b/src/app/features/genes/components/gene-similar/gene-similar.component.ts @@ -69,9 +69,11 @@ export class GeneSimilarComponent implements OnInit { this.helperService.setLoading(true); this.geneService .getGene(params.get('id') as string) - .subscribe((gene: Gene) => { - this.gene = gene; - this.init(); + .subscribe((gene: Gene | null) => { + if (gene) { + this.gene = gene; + this.init(); + } }); } }); diff --git a/src/app/features/genes/services/gene.service.ts b/src/app/features/genes/services/gene.service.ts index 01889bff2..d6ffc12eb 100644 --- a/src/app/features/genes/services/gene.service.ts +++ b/src/app/features/genes/services/gene.service.ts @@ -27,13 +27,15 @@ export class GeneService { // ------------------------------------------------------------------------ // - getGene(id: string): Observable { + getGene(id: string): Observable { if (this.genes[id]) { return of(this.genes[id]); } - + return this.apiService.getGene(id).pipe( - map((gene: Gene) => { + map((gene: Gene | null) => { + if (!gene) + return null; gene.similar_genes_network = this.getSimilarGenesNetwork(gene); return (this.genes[id] = gene); }) diff --git a/src/app/testing/api-service-stub.ts b/src/app/testing/api-service-stub.ts index cc94237d6..a7491cb5b 100644 --- a/src/app/testing/api-service-stub.ts +++ b/src/app/testing/api-service-stub.ts @@ -20,7 +20,7 @@ import { @Injectable() export class ApiServiceStub { - getGene(id: string): Observable { + getGene(id: string): Observable { return of(geneMock1); } diff --git a/src/app/testing/gene-service-stub.ts b/src/app/testing/gene-service-stub.ts index c70dbf189..295b37bd2 100644 --- a/src/app/testing/gene-service-stub.ts +++ b/src/app/testing/gene-service-stub.ts @@ -30,7 +30,7 @@ export class GeneServiceStub { this.geneService = new GeneService(new ApiServiceStub() as ApiService); } - getGene(id: string): Observable { + getGene(id: string): Observable { return this.geneService.getGene(id); } From 19d8b66cce5b483f8a9d539570183edd467ef46d Mon Sep 17 00:00:00 2001 From: sagely1 <114952739+sagely1@users.noreply.github.com> Date: Tue, 16 Apr 2024 14:17:58 -0700 Subject: [PATCH 2/3] AG-1399 adding e2e test for 404 redirect --- tests/gene-comparison-tool.spec.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/gene-comparison-tool.spec.ts b/tests/gene-comparison-tool.spec.ts index f203600ae..551e701d3 100644 --- a/tests/gene-comparison-tool.spec.ts +++ b/tests/gene-comparison-tool.spec.ts @@ -27,4 +27,15 @@ test.describe('specific viewport block', () => { const dropdown = page.locator('#subCategory'); await expect(dropdown).toHaveText('Targeted Selected Reaction Monitoring (SRM)'); }); + + test('invalid gene results in a 404 redirect', async ({ page }) => { + // go to invalid ENSG page + await page.goto('/genes/ENSG00000000000'); + + // expect a title "to contain" a substring. + await expect(page).toHaveTitle('Agora'); + + // expect div for page not found content to be visible + expect(page.locator('.page-not-found')).toBeVisible(); + }); }); From d6e6949cb5f768d24c7b4f717dc3bddbd36cb0a6 Mon Sep 17 00:00:00 2001 From: sagely1 <114952739+sagely1@users.noreply.github.com> Date: Wed, 17 Apr 2024 10:47:11 -0700 Subject: [PATCH 3/3] AG-1399 fixing invalid ENSG for /similar route & moving e2e tests --- .../gene-similar/gene-similar.component.ts | 5 ++++- tests/gene-comparison-tool.spec.ts | 11 ----------- tests/gene-details.spec.ts | 17 +++++++++++++++++ tests/gene-similar.spec.ts | 17 +++++++++++++++++ 4 files changed, 38 insertions(+), 12 deletions(-) create mode 100644 tests/gene-details.spec.ts create mode 100644 tests/gene-similar.spec.ts diff --git a/src/app/features/genes/components/gene-similar/gene-similar.component.ts b/src/app/features/genes/components/gene-similar/gene-similar.component.ts index 0e0c01d8c..a8be4ea38 100644 --- a/src/app/features/genes/components/gene-similar/gene-similar.component.ts +++ b/src/app/features/genes/components/gene-similar/gene-similar.component.ts @@ -70,7 +70,10 @@ export class GeneSimilarComponent implements OnInit { this.geneService .getGene(params.get('id') as string) .subscribe((gene: Gene | null) => { - if (gene) { + if (!gene) { + this.helperService.setLoading(false); + this.router.navigateByUrl('/404-not-found', { skipLocationChange: true }); + } else { this.gene = gene; this.init(); } diff --git a/tests/gene-comparison-tool.spec.ts b/tests/gene-comparison-tool.spec.ts index 551e701d3..f203600ae 100644 --- a/tests/gene-comparison-tool.spec.ts +++ b/tests/gene-comparison-tool.spec.ts @@ -27,15 +27,4 @@ test.describe('specific viewport block', () => { const dropdown = page.locator('#subCategory'); await expect(dropdown).toHaveText('Targeted Selected Reaction Monitoring (SRM)'); }); - - test('invalid gene results in a 404 redirect', async ({ page }) => { - // go to invalid ENSG page - await page.goto('/genes/ENSG00000000000'); - - // expect a title "to contain" a substring. - await expect(page).toHaveTitle('Agora'); - - // expect div for page not found content to be visible - expect(page.locator('.page-not-found')).toBeVisible(); - }); }); diff --git a/tests/gene-details.spec.ts b/tests/gene-details.spec.ts new file mode 100644 index 000000000..31883aac4 --- /dev/null +++ b/tests/gene-details.spec.ts @@ -0,0 +1,17 @@ +import { test, expect } from '@playwright/test'; + +test.describe('specific viewport block', () => { + test.slow(); + test.use({ viewport: { width: 1600, height: 1200 } }); + + test('invalid gene results in a 404 redirect', async ({ page }) => { + // go to invalid ENSG page + await page.goto('/genes/ENSG00000000000'); + + // expect a title "to contain" a substring. + await expect(page).toHaveTitle('Agora'); + + // expect div for page not found content to be visible + expect(page.locator('.page-not-found')).toBeVisible(); + }); +}); \ No newline at end of file diff --git a/tests/gene-similar.spec.ts b/tests/gene-similar.spec.ts new file mode 100644 index 000000000..29341bb4e --- /dev/null +++ b/tests/gene-similar.spec.ts @@ -0,0 +1,17 @@ +import { test, expect } from '@playwright/test'; + +test.describe('specific viewport block', () => { + test.slow(); + test.use({ viewport: { width: 1600, height: 1200 } }); + + test('invalid gene results in a 404 redirect', async ({ page }) => { + // go to invalid ENSG page + await page.goto('/genes/ENSG00000000000/similar'); + + // expect a title "to contain" a substring. + await expect(page).toHaveTitle('Agora'); + + // expect div for page not found content to be visible + expect(page.locator('.page-not-found')).toBeVisible(); + }); +}); \ No newline at end of file