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-1399 fix invalid ensg in url from hanging #1299

Merged

Conversation

sagely1
Copy link
Contributor

@sagely1 sagely1 commented Apr 16, 2024

When invalid ENSG were input into the URL, the app would show a loading screen and appeared to hang.

@sagely1 sagely1 requested a review from hallieswan April 16, 2024 21:05
Copy link
Contributor

@hallieswan hallieswan left a comment

Choose a reason for hiding this comment

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

Looks good, just a few small questions!

Comment on lines 72 to 76
.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.

tests/gene-comparison-tool.spec.ts Outdated Show resolved Hide resolved
@sagely1 sagely1 requested a review from hallieswan April 17, 2024 17:47
Copy link
Contributor

@hallieswan hallieswan left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@sagely1 sagely1 merged commit c6e8c81 into Sage-Bionetworks:develop Apr 18, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants