-
Notifications
You must be signed in to change notification settings - Fork 14
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
#3258 - Supplier Errors Part 3 #4031
Conversation
...ute-controllers/cas-supplier/_tests_/cas-supplier.aest.controller.getCASSupplier.e2e-spec.ts
Show resolved
Hide resolved
...ute-controllers/cas-supplier/_tests_/cas-supplier.aest.controller.getCASSupplier.e2e-spec.ts
Outdated
Show resolved
Hide resolved
...ute-controllers/cas-supplier/_tests_/cas-supplier.aest.controller.getCASSupplier.e2e-spec.ts
Outdated
Show resolved
Hide resolved
sources/packages/backend/libs/test-utils/src/factories/cas-supplier.ts
Outdated
Show resolved
Hide resolved
sources/packages/web/src/views/aest/student/CASSupplierInformation.vue
Outdated
Show resolved
Hide resolved
sources/packages/web/src/views/aest/student/CASSupplierInformation.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, please take a look at the comments.
SonarCloud test are failing! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job. There's just some missing changes for the columns and minor comments.
Please take a look at the PR description and if you have some idea, please feel free to share 😉 |
}, | ||
{ | ||
title: "Site protected", | ||
sortable: false, | ||
key: "siteProtected", | ||
}, | ||
{ | ||
title: "Address line 1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AC states "Address line" only.
There was a problem hiding this 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! Thanks for doing the changes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The table labels vary between starting the second-word uppercase/lowercase.
We should not have it inside the same table. Please check with biz which one should be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed with @CarlyCotton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, the UI is looking great 👍
Quality Gate failedFailed conditions |
CAS errors are shown in the row expander of the CAS Supplier Information table.
E2E test case is created and existing is updated for the changed API.
Failure in the sonarcloud is due to code duplication but I am not able to find the duplication in the code submitted in this PR.