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

[BI-1761] Error message with the backend code #352

Merged
merged 2 commits into from
Jan 4, 2024
Merged

Conversation

dmeidlin
Copy link
Contributor

@dmeidlin dmeidlin commented Nov 16, 2023

Description

Story: BI-1761

The npm packages dompurify and @types/dompurify were added as project dependencies.
The component ErrorNotification was changed to sanitize the error message from the backend with calling dompurify and render the result as html.

Dependencies

bi-api:latest

Testing

Checklist:

  • I have performed a self-review of my own code
  • I have tested my code and ensured it meets the acceptance criteria of the story
  • I have create/modified unit tests to cover this change
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to documentation
  • I have run TAF: <link to TAF run>

@dmeidlin dmeidlin requested review from a team, davedrp and mlm483 and removed request for a team November 16, 2023 19:16
Copy link
Contributor

@mlm483 mlm483 left a comment

Choose a reason for hiding this comment

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

I recommend closing this PR and making the change in bi-api instead:

EDIT:
I see the techspecs on the Jira card, but I don't think we should start treating error messages from bi-api as HTML just for this one line break.

I would take Shawn's suggested error message, and make the change on the backend. It's shorter, it can be a single line. https://breedinginsight.atlassian.net/browse/BI-1761?focusedCommentId=14200

@davedrp
Copy link
Contributor

davedrp commented Nov 29, 2023

developer test PASSED

@dmeidlin
Copy link
Contributor Author

dmeidlin commented Dec 6, 2023

I changed the message on the back end and created a new PR for bi-api, but I'm leaving the changes in place on the front end since they work with the existing messages and if rich messages are needed in the future, it won't break anything.

@dmeidlin dmeidlin requested a review from mlm483 December 6, 2023 19:08
@dmeidlin dmeidlin merged commit ac8550a into develop Jan 4, 2024
1 check passed
@dmeidlin dmeidlin deleted the feature/BI-1761 branch January 4, 2024 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants