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

Enhance error message on incorrect organisation/repo name #378

Merged
merged 11 commits into from
Jul 8, 2024

Conversation

kokerinks
Copy link
Contributor

@kokerinks kokerinks commented Jun 24, 2024

Summary:

Fixes #376

Type of change:

  • ✨ Enhancement

Changes Made:

  • Added a new error message repoOwnerNotPresentMessage, which will show now when the repo owner (i.e. organisation or username) in OWNER_NAME/REPO does not exist
    • originally showed the message repositoryNotPresentMessage instead

Screenshots:

In repo selection page:
image

After submitting:
image

Works for other repo-switching functions as well:
image
image

Proposed Commit Message:

Enhance error message on incorrect organisation/repo name

The original error message for incorrect repo name does not specify 
whether the mistake is in the org name or repo name.

Hence, let's add an additional error message to show if the 
organisation name could not be found. This will give users an easier 
time finding the error and rectifying it when keying in a repo.

Checklist:

  • I have tested my changes thoroughly.
  • I have created tests for any new code files created in this PR or provided a link to a issue/PR that addresses this.
  • I have added or modified code comments to improve code readability where necessary.
  • I have updated the project's documentation as necessary.

@nknguyenhc
Copy link
Contributor

@kokerinks If your PR is ready for review, do mark it as ready so that we can start the review process.

@kokerinks kokerinks marked this pull request as ready for review July 1, 2024 02:59
@kokerinks
Copy link
Contributor Author

@nknguyenhc marked as ready for review 👍

Copy link
Contributor

@nknguyenhc nknguyenhc left a comment

Choose a reason for hiding this comment

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

Nice job on following our coding convention!

image

nknguyenhc is a valid Github profile. You may want to check for whether the user exists as well, and change the error messages from "invalid organisation" to "invalid organisation/user"

@kokerinks
Copy link
Contributor Author

@nknguyenhc I have made the required changes, feel free to review again

Copy link
Contributor

@nknguyenhc nknguyenhc left a comment

Choose a reason for hiding this comment

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

image

When a user is viewing a repo and changes to an invalid repo,

  1. Expected behaviour: Error message pops up and user can continue viewing the previous valid repo. This is also the behaviour before this change.
  2. Actual behaviour: Error message pops up, but the user cannot continue viewing the previous valid repo.

src/app/core/services/error-message.service.ts Outdated Show resolved Hide resolved
@kokerinks
Copy link
Contributor Author

kokerinks commented Jul 6, 2024

@nknguyenhc Have addressed issues again in latest commits, let me know if there's any other changes to be made!

@kokerinks kokerinks requested a review from nknguyenhc July 6, 2024 15:23
@kokerinks kokerinks requested a review from nknguyenhc July 8, 2024 03:13
Copy link
Contributor

@nknguyenhc nknguyenhc left a comment

Choose a reason for hiding this comment

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

LGTM!

@nknguyenhc nknguyenhc merged commit 03ef9e0 into CATcher-org:main Jul 8, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect error message
2 participants