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

Feature/40 add error handling via notifications #125

Merged
merged 15 commits into from
Nov 14, 2024
Merged

Feature/40 add error handling via notifications #125

merged 15 commits into from
Nov 14, 2024

Conversation

zsoltker
Copy link
Contributor

@zsoltker zsoltker commented Nov 5, 2024

No description provided.

@zsoltker
implement toasters / notifications
97c2675 
Signed-off-by: Keresztes Zsolt <[email protected]>
@zsoltker
implement custom ErrorBoundary (part1)
240a27f 
Signed-off-by: Keresztes Zsolt <[email protected]>
@zsoltker
error notification working version 1 -- small refinements needed
52dea0f 
Signed-off-by: Keresztes Zsolt <[email protected]>
@zsoltker
error notification working version 2 -- small refinements needed
351441c 
Signed-off-by: Keresztes Zsolt <[email protected]>
@zsoltker
Merge branch 'refs/heads/main' into feature/40-add-error-handling-via…
cca0669 
…-notifications
@zsoltker
error notification working version 3 -- small refinements needed
17df050 
Signed-off-by: Keresztes Zsolt <[email protected]>
@zsoltker
error notification working version 4-- small refinements needed
8ca0e4e 
Signed-off-by: Keresztes Zsolt <[email protected]>
@zsoltker
error notification working version 5 (error context provider)
6dc1c1b 
Signed-off-by: Keresztes Zsolt <[email protected]>
@zsoltker
error notification working version 6
aa2bc19 
Signed-off-by: Keresztes Zsolt <[email protected]>
@zsoltker zsoltker requested a review from devbysp November 5, 2024 13:06
devbysp
devbysp requested changes Nov 7, 2024
View reviewed changes
src/types/gaiaXException.model.ts Outdated
@@ -0,0 +1,2 @@
export type GaiaXExceptionType = 'BusinessException' | 'TechnicalException';

This comment was marked as resolved.

Sign in to view

src/common/exceptions/GaiaXException.ts Outdated
@@ -0,0 +1,11 @@
import { GaiaXExceptionType } from '../../types/gaiaXException.model';

export abstract class GaiaXException extends Error {

This comment was marked as resolved.

Sign in to view

src/common/exceptions/GaiaXException.ts Outdated
import { GaiaXExceptionType } from '../../types/gaiaXException.model';

export abstract class GaiaXException extends Error {
abstract type: GaiaXExceptionType;

This comment was marked as resolved.

Sign in to view

src/common/exceptions/GaiaXException.ts Outdated
export abstract class GaiaXException extends Error {
abstract type: GaiaXExceptionType;

protected constructor(message: string) {

This comment was marked as resolved.

Sign in to view

src/common/exceptions/GaiaXException.ts Outdated
import { GaiaXExceptionType } from '../../types/gaiaXException.model';

export abstract class GaiaXException extends Error {
abstract type: GaiaXExceptionType;

This comment was marked as resolved.

Sign in to view

src/types/notification.model.ts Outdated

export interface Notification {
messageType?: NotificationMessageType,
message?: ToastContent;

This comment was marked as duplicate.

Sign in to view

src/components/notification/Notification.ts Outdated

const toastOptions = {
...notification.options,
autoClose: getAutoClose(notification.options ? notification.options.autoClose : undefined),

This comment was marked as resolved.

Sign in to view

src/components/notification/Notification.ts Outdated

export const notify = (notification: Notification) => {

function getAutoClose(autoClose: number | undefined | false): number | false {

This comment was marked as resolved.

Sign in to view

src/index.css
@@ -15,3 +15,7 @@ code {
font-family: source-code-pro, Menlo, Monaco, Consolas, 'Courier New',
monospace;
}

body > iframe {

This comment was marked as resolved.

Sign in to view

src/common/exceptions/GaiaXException.ts Outdated
super(message);
}

abstract handleNotification(addException?: () => void): void;

This comment was marked as resolved.

Sign in to view

@devbysp
Copy link
Contributor

devbysp commented Nov 12, 2024

Please write tests except for these files (add this comment /* test coverage not required */ add the beginning of the files in order to not be included in the test coverage.):

  • src/index.js
  • src/types/exception.model.ts
  • src/types/notification.model.ts

image

@zsoltker
test for notifications, error boundary and error context provider
b8780e2 
Signed-off-by: Keresztes Zsolt <[email protected]>
@zsoltker
reason !!! npm error Could not resolve dependency:
41eb60f 
       npm error peer uuid@"^3.4.0 || ^7.0.0 || ^8.0.0 || ^9.0.0" from [email protected]

Signed-off-by: Keresztes Zsolt <[email protected]>
@zsoltker
fix typo
09ce387 
Signed-off-by: Keresztes Zsolt <[email protected]>
@zsoltker zsoltker requested a review from devbysp November 13, 2024 14:17
@zsoltker zsoltker removed the request for review from Huber-David November 14, 2024 12:36
@zsoltker zsoltker merged commit 0e69628 into main Nov 14, 2024
3 checks passed
@zsoltker zsoltker deleted the feature/40-add-error-handling-via-notifications branch November 14, 2024 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devbysp devbysp devbysp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zsoltker @devbysp