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

Jump to bottom

OV-7: error handling #21

Merged
merged 4 commits into from
Aug 23, 2024
Merged

OV-7: error handling #21

merged 4 commits into from
Aug 23, 2024

Conversation

WilsonBravo
Copy link
Contributor

FEAT: Error handling #7

Test error:

const error: ServerValidationErrorResponse = {
    errorType: 'VALIDATION',
    message: 'Validation failed for the provided data.',
    details: [
        {
            path: ['username'],
            message: 'Username is required.'
        },
        {
            path: ['email'],
            message: 'Email must be a valid email address.'
        }
    ]
};

Results:

Only message:

1

Message and details:

2

@WilsonBravo WilsonBravo added the FE Fronted feature label Aug 20, 2024
@WilsonBravo WilsonBravo added this to the Release 1.0 milestone Aug 20, 2024
@WilsonBravo WilsonBravo self-assigned this Aug 20, 2024
@WilsonBravo WilsonBravo linked an issue Aug 20, 2024 that may be closed by this pull request
FEAT: Error handling #7
Closed
6 tasks
anton-otroshchenko
anton-otroshchenko requested changes Aug 20, 2024
View reviewed changes
frontend/src/bundles/common/helpers/string-to-react-node.tsx Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

helpers should not contain .tsx files. And also create a folder for file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I deleted the helper based on the recommendations provided during the daily meeting.

frontend/src/bundles/common/middlewares/error-handling.middleware.ts Outdated
Comment on lines 32 to 40
toast({
id: toastId,
title: 'An error occurred.',
description: stringToReactNode(message),
status: 'error',
duration: 7000,
isClosable: true,
position: 'top-right',
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Create a separate service for Toast that we could call for multiple scenarios: error, warn, success, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I implemented a toast service for success, warning, error, info, and loading messages.

6

anton-otroshchenko
anton-otroshchenko requested changes Aug 21, 2024
View reviewed changes
frontend/src/bundles/common/middlewares/error-handling.middleware.ts Outdated
}
}

if (message !== '' && !toastService.isActive(toastId)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (message !== '' && !toastService.isActive(toastId)) {
if (message && !toastService.isActive(toastId)) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

frontend/src/bundles/common/services/toast/toast.service.ts Outdated
Comment on lines 15 to 24
this.toast({
id: toastId,
title: title,
description: message,
status: 'warning',
duration: 7000,
isClosable: true,
position: 'top-right',
variant: 'solid',
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

You are using the same properties for every method except for status.

Create a separate function for calling toast and just pass status as argument

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

frontend/src/bundles/common/services/toast/toast.service.ts Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename it to notification.service.ts

frontend/src/bundles/common/services/toast/toast.service.ts Outdated
toast: ReturnType<typeof createStandaloneToast>['toast'];
};

class ToastService {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

frontend/src/bundles/common/services/toast/toast.ts Outdated
Comment on lines 11 to 12
export { ToastService } from './toast.service.js';
export { toastService };
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to combine these 2 exports?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if I try to do so, I receive the next error:

Use export…from to re-export NotificationService.eslint unicorn/prefer-export-from

@nikita-remeslov nikita-remeslov merged commit e0aa34e into next Aug 23, 2024
2 checks passed
@nikita-remeslov nikita-remeslov deleted the task/OV-7-add-error-handling branch August 23, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anton-otroshchenko anton-otroshchenko anton-otroshchenko approved these changes

@o-nedashkivska o-nedashkivska o-nedashkivska approved these changes

@nikita-remeslov nikita-remeslov nikita-remeslov approved these changes

@sofiia-trokhymchuk sofiia-trokhymchuk Awaiting requested review from sofiia-trokhymchuk sofiia-trokhymchuk is a code owner

Assignees

@WilsonBravo WilsonBravo

Labels
FE Fronted feature
Projects
bsa-2024-outreachvids
Status: To Be Tested
Milestone
Pre-DEMO Release
Development

Successfully merging this pull request may close these issues.

FEAT: Error handling
4 participants
@WilsonBravo @nikita-remeslov @o-nedashkivska @anton-otroshchenko