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

126 add error handling mechanism to the UI #518

Merged
merged 12 commits into from
Oct 15, 2024

Conversation

ChengShi-1
Copy link
Contributor

@ChengShi-1 ChengShi-1 commented Oct 4, 2024

What this PR does / why we need it:

In some cases, unexpected errors may occur in the application. To manage these effectively, we need a consistent error-handling mechanism in the UI. This not only ensures that error messages are displayed to users on the page but also enables developers to track errors by viewing detailed messages in the console.

Which issue(s) this PR closes:

Special notes for your reviewer:

The error page didn't do component test and e2e test.
Component test: we log errors using the hook useRouteError, which must be used within a data router. However, the component test used memory router.
e2e test: we didn't find a way to throw an error in the test environment to trigger the page. Cypress always caught the errors we throw.

Suggestions on how to test this:

Step 1: Run the Development Environment

  1. Execute npm i.
  2. Navigate with cd packages/design-system && npm run build.
  3. Return with cd ../../.
  4. Ensure you have a .env file similar to .env.example, with the variable VITE_DATAVERSE_BACKEND_URL=http://localhost:8000.
  5. Navigate with cd dev-env.
  6. Start the environment using ./run-env.sh unstable.
  7. To verify the environment, visit http://localhost:8000/ and check your local Dataverse installation.

Step 2: Test the feature
Since we could only test it manually, please edit some codes in Dataverse-frontend repository.

  1. Inside dataverse-frontend/src/sections/homepage/Homepage.tsx or any other children routes, we could add a line throw new Error('This is an error') before returns.
  2. check if the error messages are printed in the console
  3. check if the error page UI is shown correctly, with the header and footer
  4. check if the button, header and footer work well

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

image

Is there a release notes update needed for this change?:

No

Additional documentation:

@ChengShi-1 ChengShi-1 linked an issue Oct 4, 2024 that may be closed by this pull request
@coveralls
Copy link

coveralls commented Oct 4, 2024

Coverage Status

coverage: 97.512% (-0.02%) from 97.533%
when pulling a1bbdbe on 126-add-error-handling-mechanism-to-the-ui
into 36c7b07 on develop.

@ChengShi-1 ChengShi-1 marked this pull request as ready for review October 4, 2024 15:38
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

Just a drive-by comment about brand name. 😄

src/sections/error-page/ErrorPage.tsx Outdated Show resolved Hide resolved
@ChengShi-1 ChengShi-1 added FY25 Sprint 7 FY25 Sprint 7 (2024-09-25 - 2024-10-09) Size: 3 A percentage of a sprint. 2.1 hours. SPA.Q3 Not related to any specific Q3 feature UI Tasks related to the user interface (UI) or frontend development GREI Re-arch GREI re-architecture-related pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows pm.GREI-d-2.7.2 NIH, yr2, aim7, task2: Implement UI modules for creating datasets and publishing workflows FY25 Sprint 6 FY25 Sprint 6 labels Oct 7, 2024
@ekraffmiller ekraffmiller self-assigned this Oct 7, 2024
@ChengShi-1 ChengShi-1 removed the FY25 Sprint 6 FY25 Sprint 6 label Oct 7, 2024
ekraffmiller
ekraffmiller previously approved these changes Oct 8, 2024
Copy link
Contributor

@ekraffmiller ekraffmiller 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!

@ekraffmiller ekraffmiller removed their assignment Oct 8, 2024
src/sections/error-page/ErrorPage.tsx Outdated Show resolved Hide resolved
@ekraffmiller
Copy link
Contributor

Sorry if I approved this prematurely, I thought it was ready for QA, but maybe I misread the ticket!

@GPortas GPortas added SPA.Q4 Not related to any specific Q4 feature and removed SPA.Q3 Not related to any specific Q3 feature labels Oct 9, 2024
@g-saracca g-saracca assigned g-saracca and unassigned g-saracca Oct 9, 2024
@g-saracca
Copy link
Contributor

Sorry if I approved this prematurely, I thought it was ready for QA, but maybe I misread the ticket!

Don't worry, is just that I left a comment without a formal review I think

@cmbz cmbz added the FY25 Sprint 8 FY25 Sprint 8 (2024-10-09 - 2024-10-23) label Oct 9, 2024
Copy link
Contributor

@g-saracca g-saracca left a comment

Choose a reason for hiding this comment

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

Thanks @ChengShi-1, could you add a story for the error page? I think we are missing that

@g-saracca
Copy link
Contributor

We wont be able to add a story for this page right now, we need an extra configuration for Storybook explained in this issue I have created.

@ChengShi-1 could you solve the merge conflicts? thanks!

@ChengShi-1 ChengShi-1 dismissed g-saracca’s stale review October 10, 2024 19:21

Create another issue for storybook

@ChengShi-1
Copy link
Contributor Author

We wont be able to add a story for this page right now, we need an extra configuration for Storybook explained in this issue I have created.

@ChengShi-1 could you solve the merge conflicts? thanks!

Hi German, thanks for reminding me the conflicts. Please check that I solved the conflicts and merged 'develop' to this branch.

src/router/routes.tsx Outdated Show resolved Hide resolved
@g-saracca g-saracca assigned g-saracca and unassigned ChengShi-1 Oct 14, 2024
@g-saracca g-saracca removed their assignment Oct 14, 2024
@ofahimIQSS ofahimIQSS self-assigned this Oct 15, 2024
@ofahimIQSS
Copy link
Contributor

Tested in local and was able to verify the error page was thrown.

image

@ofahimIQSS ofahimIQSS merged commit 72348db into develop Oct 15, 2024
13 of 14 checks passed
@ofahimIQSS ofahimIQSS deleted the 126-add-error-handling-mechanism-to-the-ui branch October 15, 2024 15:53
@ofahimIQSS ofahimIQSS removed their assignment Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FY25 Sprint 7 FY25 Sprint 7 (2024-09-25 - 2024-10-09) FY25 Sprint 8 FY25 Sprint 8 (2024-10-09 - 2024-10-23) GREI Re-arch GREI re-architecture-related Original size: 3 pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows pm.GREI-d-2.7.2 NIH, yr2, aim7, task2: Implement UI modules for creating datasets and publishing workflows Size: 3 A percentage of a sprint. 2.1 hours. SPA.Q4 Not related to any specific Q4 feature UI Tasks related to the user interface (UI) or frontend development
Projects
Status: Done 🧹
Development

Successfully merging this pull request may close these issues.

Add error handling mechanism to the UI
8 participants