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

#2116 - Request an Offering Change - Submit/View UI - Validations #2149

Merged

ann-aot merged 12 commits into main from feature/sims-#2116

Aug 9, 2023

Contributor

ann-aot commented Jul 31, 2023 •

edited

Loading

Added banner for the past end date.
Tabs are now router based and the page is redirected to the in-progress tab after successful submission.
Centralized PIR validation logic and used in application offering submission.
Error code and error msg in student-assessment.service.ts was incorrect and adjusted.
getApplicationInfo was retrieving assessment details with respect to student assessment in the application and then it was filtering, it is now taking from the current assessment.
sources/packages/web/src/components/generic/Banner.vue removed validator from property type and added type with PropType as there were some vue warnings in the console.

Screenshots

Next PR

E2E tests

ann-aot added 4 commits

July 26, 2023 19:36


          initial commit

89e71a0


          Merge branch 'main' into feature/sims-#2116

129423b


          redirect and validation

d59d1f4


          refactor

b84fea9

ann-aot self-assigned this

ann-aot requested review from andrewsignori-aot, andrepestana-aot, sh16011993, dheepak-aot and guru-aot and removed request for andrepestana-aot

July 31, 2023 19:55

ann-aot added Web Portal SIMS-Api labels

ann-aot added 2 commits

July 31, 2023 16:41


          refactor

25f3515


          refactor

34b812b

ann-aot marked this pull request as ready for review

July 31, 2023 20:46

andrewsignori-aot reviewed

View reviewed changes

...ation-offering-change-request/application-offering-change-request.institutions.controller.ts Show resolved Hide resolved

andrewsignori-aot reviewed

View reviewed changes

...ation-offering-change-request/application-offering-change-request.institutions.controller.ts Show resolved Hide resolved

ann-aot added 2 commits

August 1, 2023 18:32


          refactor

7570f1a


          refactor

f25ca03

dheepak-aot reviewed

View reviewed changes

sources/packages/backend/apps/api/src/constants/error-code.constants.ts

+              /**
+               * Offering does not belong to the location.
+               */
+              export const OFFERING_DOES_NOT_BELONG_TO_LOCATION =

Collaborator

dheepak-aot Aug 2, 2023

Suggestion: on the same context OFFERING_LOCATION_MISMATCH ?

Contributor Author

ann-aot Aug 3, 2023 •

edited

Loading

IMO @dheepak-aot MISMATCH is when there is a direct comparison between two values. So, keeping as it is

dheepak-aot reviewed

View reviewed changes

sources/packages/backend/apps/api/src/services/application/application.service.ts Outdated Show resolved Hide resolved

dheepak-aot reviewed

View reviewed changes

sources/packages/backend/apps/api/src/services/application/application.service.ts Outdated

+                  selectedOffering: number,
+                  locationId: number,
+                  options?: {
+                    isPir: boolean;

Collaborator

dheepak-aot Aug 2, 2023

isPIR?

dheepak-aot reviewed

View reviewed changes

sources/packages/backend/apps/api/src/services/application/application.service.ts Outdated Show resolved Hide resolved

dheepak-aot reviewed

View reviewed changes

sources/packages/backend/apps/api/src/services/application/application.service.ts Outdated

+                  const offering = await this.offeringService.getOfferingLocationId(
+                    selectedOffering,
+                  );
+                  if (offering?.institutionLocation.id !== locationId) {

Collaborator

dheepak-aot Aug 2, 2023

May be we should check if offering exist before doing this validation? Because if the offeringId is not a valid one offering not found would be more accurate

dheepak-aot reviewed

View reviewed changes

sources/packages/backend/apps/api/src/services/application/application.service.ts

+                }
+                /**
+                 * Set of validations for existing application and newly

Collaborator

dheepak-aot Aug 2, 2023

Just a thought. As this class(ApplicationService) is having more than 1500 lines of code already, should we consider creating a service like ApplicationOfferingService and move this method there?

Contributor Author

ann-aot Aug 3, 2023

I also had similar thought when I was doing this, but this validation is not specific to Application Offering change request, so I thought its better to do it in application.service

dheepak-aot reviewed

View reviewed changes

...ges/web/src/views/institution/locations/request-a-change/request-a-change.vue/InProgress.vue Outdated

		@@ -0,0 +1,20 @@
		<template>

Collaborator

dheepak-aot Aug 2, 2023

The file path has request-a-change.vue

Contributor Author

ann-aot Aug 3, 2023

Thanks @dheepak-aot . Nice catch

Collaborator

dheepak-aot Aug 8, 2023

Still wondering why request-a-change is repeated in the directory names

sh16011993 reviewed

View reviewed changes

sources/packages/web/src/views/institution/locations/request-a-change/RequestAChange.vue

@@ @@ -72,13 +37,37 @@ export default defineComponent({ @@
                 },
                 setup(props) {
                   const { getLocationName } = useInstitutionState();
-                  const tab = ref(ActiveRequestAChangeTab.AvailableToChangeTab);
+                  const tabItems = ref([

Collaborator

sh16011993 Aug 2, 2023

Nice to see the tabs implemented as routes 👍

sh16011993 reviewed

View reviewed changes

sources/packages/web/src/router/InstitutionRoutes.ts Outdated Show resolved Hide resolved

sh16011993 reviewed

View reviewed changes

sources/packages/web/src/router/InstitutionRoutes.ts Outdated Show resolved Hide resolved

andrewsignori-aot reviewed

View reviewed changes

sources/packages/web/src/composables/useFormatters.ts Show resolved Hide resolved

andrewsignori-aot reviewed

View reviewed changes

.../src/views/institution/locations/request-a-change/request-a-change.vue/AvailableToChange.vue Outdated Show resolved Hide resolved

andrewsignori-aot reviewed

View reviewed changes

Collaborator

andrewsignori-aot left a comment

Great work and thanks for taking this forward. Please tale a look at the comments. There is one or another that maybe I am missing something and just need some clarification.

ann-aot added 2 commits

August 4, 2023 18:06


          refactor

e5bb57d


          feedback

f8bae6e

andrewsignori-aot reviewed

View reviewed changes

sources/packages/web/src/composables/useFormatters.ts Outdated Show resolved Hide resolved

andrewsignori-aot reviewed

View reviewed changes

...i/src/route-controllers/program-info-request/program-info-request.institutions.controller.ts Outdated Show resolved Hide resolved

andrewsignori-aot reviewed

View reviewed changes

...ckend/apps/api/src/services/education-program-offering/education-program-offering.service.ts Outdated Show resolved Hide resolved

andrewsignori-aot reviewed

View reviewed changes

Collaborator

andrewsignori-aot left a comment

Thanks for doing the changes. Minor comments left only.

ann-aot added 2 commits

August 8, 2023 18:06


          feedback

4ad3cb1


          feedback

766cd4c

github-actions bot commented Aug 8, 2023

Backend Unit Tests Coverage Report

Totals
Statements:	17.87% ( 2141 / 11982 )
Methods:	8.25% ( 126 / 1528 )
Lines:	20.68% ( 1877 / 9077 )
Branches:	10.02% ( 138 / 1377 )

sonarqubecloud bot commented Aug 8, 2023

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

andrewsignori-aot approved these changes

View reviewed changes

Collaborator

andrewsignori-aot left a comment

Thanks for doing the changes, look good 👍

github-actions bot commented Aug 8, 2023

E2E Queue Consumers Coverage Report

Totals
Statements:	72.5% ( 406 / 560 )
Methods:	63.38% ( 45 / 71 )
Lines:	74.53% ( 357 / 479 )
Branches:	40% ( 4 / 10 )

github-actions bot commented Aug 8, 2023

E2E Workflow Workers Coverage Report

Totals
Statements:	49.81% ( 267 / 536 )
Methods:	41.56% ( 32 / 77 )
Lines:	55.33% ( 218 / 394 )
Branches:	26.15% ( 17 / 65 )

github-actions bot commented Aug 8, 2023

E2E SIMS API Coverage Report

Totals
Statements:	52.44% ( 3765 / 7179 )
Methods:	49.14% ( 457 / 930 )
Lines:	57.56% ( 3082 / 5354 )
Branches:	25.25% ( 226 / 895 )

guru-aot approved these changes

View reviewed changes

Collaborator

guru-aot left a comment

LGTM, nice work @ann-aot

Collaborator

dheepak-aot commented Aug 8, 2023

Thanks for doing the changes, Just one minor comment #2149 (comment)

dheepak-aot approved these changes

View reviewed changes

Collaborator

dheepak-aot left a comment

LGTM 👍Thanks for explaining the comment #2149 (comment)

sh16011993 approved these changes

View reviewed changes

Collaborator

sh16011993 left a comment

Thank You for doing the changes. Nice work 👍

ann-aot merged commit 4734765 into main

ann-aot temporarily deployed to DEV

August 9, 2023 16:37

— with

GitHub Actions Inactive

ann-aot temporarily deployed to DEV

August 9, 2023 16:38

— with

GitHub Actions Inactive

ann-aot temporarily deployed to DEV

August 9, 2023 16:38

— with

GitHub Actions Inactive

ann-aot temporarily deployed to DEV

August 9, 2023 16:38

— with

GitHub Actions Inactive

ann-aot temporarily deployed to DEV

August 9, 2023 16:38

— with

GitHub Actions Inactive

ann-aot temporarily deployed to DEV

August 9, 2023 16:40

— with

GitHub Actions Inactive

ann-aot temporarily deployed to DEV

August 9, 2023 16:40

— with

GitHub Actions Inactive

ann-aot deleted the feature/sims-#2116 branch

August 9, 2023 17:06

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

Labels