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

AG-1399 fix invalid ensg in url from hanging #1299

Merged
merged 3 commits into from
Apr 18, 2024
Merged

AG-1399 fix invalid ensg in url from hanging #1299

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a21839b
AG-1399 fix invalid ensg in url from hanging
sagely1 Apr 16, 2024
19d8b66
AG-1399 adding e2e test for 404 redirect
sagely1 Apr 16, 2024
d6e6949
AG-1399 fixing invalid ENSG for /similar route & moving e2e tests
sagely1 Apr 17, 2024
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
2 changes: 1 addition & 1 deletion src/app/core/services/api.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class ApiService {
return url;
}

getGene(id: string): Observable<Gene> {
getGene(id: string): Observable<Gene | null> {
return this.http.get<Gene>(this.getBaseUrl() + '/api/genes/' + id, {
headers: new HttpHeaders(defaultHeaders),
});
Expand Down
65 changes: 35 additions & 30 deletions src/app/features/genes/components/gene-details/gene-details.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
sagely1 marked this conversation as resolved.
Show resolved Hide resolved
if (!gene) {
this.helperService.setLoading(false);
sagely1 marked this conversation as resolved.
Show resolved Hide resolved
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);
});
}

Expand Down
8 changes: 5 additions & 3 deletions src/app/features/genes/components/gene-similar/gene-similar.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this component also handle a gene that doesn't exist? Looks like navigating to https://agora.adknowledgeportal.org/genes/ENSG55555555555/similar causes the same behavior as the route mentioned in the ticket.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! The same fix applies from gene-details.

});
}
});
Expand Down
8 changes: 5 additions & 3 deletions src/app/features/genes/services/gene.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ export class GeneService {

// ------------------------------------------------------------------------ //

getGene(id: string): Observable<Gene> {
getGene(id: string): Observable<Gene | null> {
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);
sagely1 marked this conversation as resolved.
Show resolved Hide resolved
return (this.genes[id] = gene);
})
Expand Down
2 changes: 1 addition & 1 deletion src/app/testing/api-service-stub.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {

@Injectable()
export class ApiServiceStub {
getGene(id: string): Observable<Gene> {
getGene(id: string): Observable<Gene | null> {
return of(geneMock1);
}

Expand Down
2 changes: 1 addition & 1 deletion src/app/testing/gene-service-stub.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export class GeneServiceStub {
this.geneService = new GeneService(new ApiServiceStub() as ApiService);
}

getGene(id: string): Observable<Gene> {
getGene(id: string): Observable<Gene | null> {
return this.geneService.getGene(id);
}

Expand Down
11 changes: 11 additions & 0 deletions tests/gene-comparison-tool.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
sagely1 marked this conversation as resolved.
Show resolved Hide resolved
// go to invalid ENSG page
await page.goto('/genes/ENSG00000000000');

// expect a title "to contain" a substring.
await expect(page).toHaveTitle('Agora');
sagely1 marked this conversation as resolved.
Show resolved Hide resolved

// expect div for page not found content to be visible
expect(page.locator('.page-not-found')).toBeVisible();
sagely1 marked this conversation as resolved.
Show resolved Hide resolved
});
});
Loading