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

Display a detailed error message for invalid molecular profiles #4306

Merged

Conversation

onursumer
Copy link
Member

@onursumer onursumer requested a review from alisman June 22, 2022 21:29

if (invalidMolecularProfiles.length > 0) {
return Promise.reject({
detailedErrorMessage: `Invalid molecular profile id(s): ${invalidMolecularProfiles.join(
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if this is the best solution. detailedErrorMessage is a custom field. We may want to have a well defined Error object for consistency and always use the same fields everywere.

<ErrorMessage />
<ErrorMessage
message={detailedErrorMessages.join('\n')}
disableDefaultContactMessage={
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we still keep the default contact message Please let us know about this error and how you got here at...?

return (
<div className={styles.container}>
<div className={styles['items-container']}>
{this.items.map(this.makeItem)}
</div>
{_.some(this.items, i => getItemStatus(i) === 'error') && (
<ErrorMessage />
<ErrorMessage
message={detailedErrorMessages.join('\n')}
Copy link
Member Author

Choose a reason for hiding this comment

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

Joining with \n is practically the same as joining with any white space. If we really need to format multiple lines of messages I think we need to pass a component instead of a simple string. Since encountering multiple detailed error messages is a low chance event, maybe it's okay to keep it like this for now?

@alisman
Copy link
Collaborator

alisman commented Jun 23, 2022

This is better. But I think we should try to just kind of abort the whole page when this happens. We've struggled with how to this. Basically we need to dispatch an error to global error handler. Let me play with it a bit.

@alisman alisman merged commit 5213086 into cBioPortal:master Jul 5, 2022
@onursumer onursumer deleted the support-custom-indicator-error-message branch July 27, 2022 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

invalid molecular profile id not handled well in study view
3 participants