-
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
#1792-Enable application details for public institution user #1914
#1792-Enable application details for public institution user #1914
Conversation
…_Public_Institution_User
...rs/application/_tests_/application.institutions.controller.getApplicationDetails.e2e-spec.ts
Outdated
Show resolved
Hide resolved
...rs/application/_tests_/application.institutions.controller.getApplicationDetails.e2e-spec.ts
Outdated
Show resolved
Hide resolved
...rs/application/_tests_/application.institutions.controller.getApplicationDetails.e2e-spec.ts
Outdated
Show resolved
Hide resolved
...es/backend/apps/api/src/route-controllers/application/application.institutions.controller.ts
Outdated
Show resolved
Hide resolved
...es/backend/apps/api/src/route-controllers/application/application.institutions.controller.ts
Outdated
Show resolved
Hide resolved
name: InstitutionRoutesConst.STUDENT_APPLICATION_DETAILS, | ||
props: true, | ||
component: InstitutionApplicationView, | ||
meta: { |
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.
I believe we don't need these meta for children if the parent has the same meta
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.
did u test this scenario @guru-aot . just to confirm
@@ -79,15 +79,18 @@ export class ApplicationApi extends HttpBaseClient { | |||
|
|||
/** | |||
* API Client for application detail. | |||
* @param applicationId | |||
* @returns | |||
* @param applicationId for the application. |
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.
👍
@@ -15,7 +15,7 @@ | |||
Student Application Details | |||
{{ | |||
applicationDetail.applicationNumber | |||
? " - " + applicationDetail.applicationNumber | |||
? HYPHEN_WITH_SPACE + applicationDetail.applicationNumber |
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.
Will there be an application here without number? We can skip the expression if there is no such possibility.
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.
Student Application Details | ||
{{ | ||
applicationDetail.applicationNumber | ||
? HYPHEN_WITH_SPACE + applicationDetail.applicationNumber |
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.
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.
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.
removed it and used emptyStringFiller
@@ -10,6 +10,7 @@ dayjs.extend(customParseFormat); | |||
|
|||
const DEFAULT_EMPTY_VALUE = "-"; | |||
export const DATE_ONLY_ISO_FORMAT = "YYYY-MM-DD"; | |||
export const HYPHEN_WITH_SPACE = " - "; |
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.
Please see #1914 (comment)
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.
removed it
}); | ||
}); | ||
|
||
it("Should not have access to get the student application details if the student submitted and application to non-public institution.", async () => { |
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.
Minor: we're using "Should/when" in the test cases.
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.
LGTM @guru-aot 👍. 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.
Thanks for doing the changes, looks good 👍
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!
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.
Thanks for doing the changes @guru-aot 👍LGTM.
…_Public_Institution_User
Kudos, SonarCloud Quality Gate passed!
|
Enable application details page for public institution user
Acceptance Criteria
The sidebar for the application details is added till students, @ann-aot will update the assessments part.

Additional screenshots on the Application details page.
