Skip to content

Conversation

@ubprudhvi
Copy link

No description provided.

@ubprudhvi ubprudhvi linked an issue Dec 6, 2024 that may be closed by this pull request
@github-actions github-actions bot added bug Something isn't working enhancement New feature or request release security labels Dec 6, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🎉 Thanks for opening your first pull request! We really appreciate your contribution to making this project better. We'll review your changes and get back to you soon.

Copy link
Contributor

@shivamvijaywargi shivamvijaywargi left a comment

Choose a reason for hiding this comment

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

Good work and welcome on board🎉

= "Something went wrong, please try again later";
export const VALIDATION_ERROR_MESSAGE = "The validation error(s)";
export const SERVER_ERROR = "Server side error(s)";
export const GROUP_DOESNT_EXIST = "Group with id does not exist";
Copy link
Contributor

Choose a reason for hiding this comment

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

How about we have a function for these messages and where and when needed we will pass the params and put them in place?

export const RESOURCE_NOT_FOUND = (resourceName: string, resourceId: string) => ${resourceName} with id ${resourceId} does not exist;

And for the common messages we will have no arguments to the function just to stay consistent.

@GenieWizards/work-o-holics Any thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes this is a good idea. This way error will be more precise and easy to build.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ubprudhvi Please follow this and update the PR.

Copy link
Author

Choose a reason for hiding this comment

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

ok

message: z.string(),
}),
"Server side error(s)",
SERVER_ERROR,
Copy link
Contributor

Choose a reason for hiding this comment

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

For this we should have INTERNAL_SERVER_ERROR which should say 'Something went wrong, please try again.'.

@sonarqubecloud
Copy link

Copy link
Contributor

@shivamvijaywargi shivamvijaywargi left a comment

Choose a reason for hiding this comment

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

Also, please resolve merge conflicts.

= "Something went wrong, please try again later";
= "Something went wrong, please try again later.";
export const VALIDATION_ERROR_MESSAGE = "The validation error(s)";
export const NOT_AUTHORIZED = "User is not authorized";
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have an unauthorised error message constant.

export const AUTHORIZATION_ERROR_MESSAGE
= "You are not authorized, please login.";
export const FORBIDDEN_ERROR_MESSAGE
= "You are not allowed to perform this action.";
Copy link
Contributor

Choose a reason for hiding this comment

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

For these static messages also better to have them as functions for consistency
Eg: () => 'Error message';

= "Something went wrong, please try again later.";
export const VALIDATION_ERROR_MESSAGE = "The validation error(s)";
export const NOT_AUTHORIZED = "User is not authorized";
export const RESOURCE_NOT_FOUND = (resourceName: string, resourceId: ZodString) => `${resourceName} with id ${resourceId} does not exist`;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably have an enum for resourceName.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request release security

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[REFACTOR]: Export common error messages from a single file

4 participants