#1953 - Request an Offering Change - DB Migrations #1995

ann-aot · 2023-06-07T00:14:48Z

Created application_offering_change_requests table and application_offering_change_request_status_types enum type

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql

andrepestana-aot · 2023-06-07T19:30:43Z

...ions/src/sql/ApplicationOfferingChangeRequests/Drop-application-offering_change-requests.sql

@@ -0,0 +1,2 @@
+-- DROP Table
+DROP TABLE IF EXISTS sims.application_offering_change_requests;


https://github.com/bcgov/SIMS/pull/1995/files#r1222068667

...b-migrations/src/migrations/1686094364995-CreateApplicationOfferingRequestsAndStatusTypes.ts

...apps/db-migrations/src/sql/Types/Create-application-offering-change-request-status-types.sql

...ces/packages/backend/libs/sims-db/src/entities/application-offering-change-requests.model.ts

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql

andrepestana-aot

Great job. Left minor comments and one for my knowledge. Thanks!

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql

andrewsignori-aot · 2023-06-07T19:57:03Z

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql

+  id SERIAL PRIMARY KEY,
+  application_id INT NOT NULL REFERENCES sims.applications(id),
+  change_offering_id INT NOT NULL REFERENCES sims.education_programs_offerings(id),
+  active_offering_id INT NOT NULL REFERENCES sims.education_programs_offerings(id),


Should we keep the active_offering_id in the table or just use the one from the current assessment?

mm, that makes sense. I was following the DB structure from the analysis ticket. @andrepestana-aot @dheepak-aot @guru-aot thoughts?

I think we already had a discussion, the thought of having the oactive offering id is when we create a componenet which shows both the offeringside byside, the requirement for it would be pass the 2 offering id's and it should render. Yes we can use the current_assessment, but again do we need to go through the application-> current_assessment->offering_id every time? I am okay with not having the offering id explicitly too, but looking at the row in the table can give the active and changed offering directly. Just my thoughts

We do not build the DB and API based on how the UI will work, right?
Yes @guru-aot I remember the conversation and I did not remember the output of it.
My point goes beyond saving the current offering id or not. What happens if between the request and the Ministry approval the application went through another reassessment? The real concern is that we would be copying the offering id from the current assessment into this table only to make it better for the UI.

I get your point. Does a reassessment happens inbetween? Is that a valid scenario? IF yes, dont we need to stop the reassessment from happening?

A reassessment can happen? Yes.
Can it change the Offering Id? Maybe.
Should we stop it? I am not sure.
Either way, should we have the offering id in both places?

@andrewsignori-aot @guru I read through the conversations, and I strongly feel that we need to we need to have active_offering_id here for the sake of accuracy of the application_offering_change_record.

Assume the following scenario without active_offering_id

An application with applicationId 100 and offeringId 8

application_offering_change_id ----> applicationId - 100, requested_offering_id-10, status-approved
(Consider that this application offering change is approved.)

The application might go through a re-assessment and get associated to an offering 12 later.

If that is the case, then this application offering change record data will say that the offering change happened from offering 12 to offering 10 which is not accurate. I am talking from the data perspective.

keeping as it is and as discussed will confirm with @JasonCTang and will update in next PR

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql

guru-aot · 2023-06-07T20:56:03Z

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql

+CREATE TABLE sims.application_offering_change_requests (
+  id SERIAL PRIMARY KEY,
+  application_id INT NOT NULL REFERENCES sims.applications(id),
+  change_offering_id INT NOT NULL REFERENCES sims.education_programs_offerings(id),


I know i created the DB column names :) , but I feel it would be better to have change_offering_id as requested_offering_id? Thoughts? @andrewsignori-aot @dheepak-aot @andrepestana-aot @sh16011993

sources/packages/backend/libs/sims-db/src/entities/index.ts

...ackages/backend/libs/sims-db/src/entities/application-offering-change-request-status.type.ts

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql

dheepak-aot · 2023-06-07T23:27:20Z

...apps/db-migrations/src/sql/Types/Create-application-offering-change-request-status-types.sql

@@ -0,0 +1,7 @@
+CREATE TYPE sims.application_offering_change_request_status_types AS ENUM (
+  'In progress with the student',


As per the second AC in the ticket the is not present here. But first AC has.

@hellolynn-tbtb ? which one should we go for?

this is what I had in my story, https://app.zenhub.com/workspaces/student-information-management-system-5fce9df5aa1b45000e937014/issues/gh/bcgov/sims/1953. Open for suggestion @hellolynn-tbtb

@ann-aot please use the Figma one and adjust the ticket.

as @hellolynn-tbtb confirmed going without the

dheepak-aot · 2023-06-07T23:28:06Z

...ackages/backend/libs/sims-db/src/entities/application-offering-change-request-status.type.ts

+   * The status is set when the institution creates a
+   * application specific offering change request.
+   */
+  InProgressWithTheStudent = "In progress with the student",


https://github.com/bcgov/SIMS/pull/1995/files#r1222277272

dheepak-aot · 2023-06-07T23:29:58Z

sources/packages/backend/libs/sims-db/src/entities/application-offering-change-request.model.ts

+import { Note } from "./note.model";
+import { ApplicationOfferingChangeRequestStatus } from "./application-offering-change-request-status.type";
+
+const REASON_MAX_LENGTH = 500;


This max length could be centralized like NOTES_MAX_LENGTH which could be used on POST API for class validators.

I didn't get the point here, i believe it should done in the submit story

I meant this constant must be more global as we see that it has more potential usage. like this.

I believe we can add export when we are actually using it

Yes just the export is missing 😊

sources/packages/backend/libs/sims-db/src/entities/application-offering-change-request.model.ts

dheepak-aot · 2023-06-07T23:43:15Z

Great start @ann-aot . Few minor comments.

andrepestana-aot

Looks good. Thanks for the explanations.

dheepak-aot

LGTM

sh16011993

LGTM 👍

andrewsignori-aot

Thanks for doing the changes and lets ensure that we are going to have a follow up on the active offering column 👍

guru-aot

LGTM

sonarqubecloud · 2023-06-08T23:30:31Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

github-actions · 2023-06-08T23:33:50Z

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 · 2023-06-08T23:34:09Z

Backend Unit Tests Coverage Report

Totals
Statements:	17.99% ( 2097 / 11659 )
Methods:	8.33% ( 124 / 1489 )
Lines:	20.75% ( 1835 / 8843 )
Branches:	10.4% ( 138 / 1327 )

github-actions · 2023-06-08T23:34:57Z

E2E Queue Consumers Coverage Report

Totals
Statements:	69.4% ( 390 / 562 )
Methods:	59.15% ( 42 / 71 )
Lines:	71.52% ( 344 / 481 )
Branches:	40% ( 4 / 10 )

github-actions · 2023-06-08T23:41:39Z

E2E SIMS API Coverage Report

Totals
Statements:	43.54% ( 3032 / 6964 )
Methods:	37.75% ( 339 / 898 )
Lines:	48.6% ( 2526 / 5197 )
Branches:	19.22% ( 167 / 869 )

#1953 - Request an Offering Change - DB Migrations (#1995)

initial commit

b3e83ee

ann-aot self-assigned this Jun 7, 2023

ann-aot added the DB DB migration involved label Jun 7, 2023

ann-aot added 3 commits June 7, 2023 14:56

table and refactor

c040bc7

refactor

8e99703

refactor

489f98c

ann-aot requested review from JasonCTang, andrepestana-aot, andrewsignori-aot, dheepak-aot, guru-aot and sh16011993 June 7, 2023 19:26

ann-aot marked this pull request as ready for review June 7, 2023 19:27

andrepestana-aot reviewed Jun 7, 2023

View reviewed changes

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql Outdated Show resolved Hide resolved

andrepestana-aot reviewed Jun 7, 2023

View reviewed changes

andrewsignori-aot reviewed Jun 7, 2023

View reviewed changes

...b-migrations/src/migrations/1686094364995-CreateApplicationOfferingRequestsAndStatusTypes.ts Outdated Show resolved Hide resolved

andrepestana-aot reviewed Jun 7, 2023

View reviewed changes

...apps/db-migrations/src/sql/Types/Create-application-offering-change-request-status-types.sql Show resolved Hide resolved

feedback

03df4e1

andrepestana-aot reviewed Jun 7, 2023

View reviewed changes

...ces/packages/backend/libs/sims-db/src/entities/application-offering-change-requests.model.ts Outdated Show resolved Hide resolved

andrewsignori-aot reviewed Jun 7, 2023

View reviewed changes

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql Show resolved Hide resolved

andrepestana-aot reviewed Jun 7, 2023

View reviewed changes

andrewsignori-aot reviewed Jun 7, 2023

View reviewed changes

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql Outdated Show resolved Hide resolved

andrewsignori-aot reviewed Jun 7, 2023

View reviewed changes

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql Outdated Show resolved Hide resolved

andrewsignori-aot reviewed Jun 7, 2023

View reviewed changes

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql Outdated Show resolved Hide resolved

feedback

c4cf6b6

guru-aot reviewed Jun 7, 2023

View reviewed changes

feedback

1be28a5

andrewsignori-aot reviewed Jun 7, 2023

View reviewed changes

sources/packages/backend/libs/sims-db/src/entities/index.ts Outdated Show resolved Hide resolved

andrewsignori-aot reviewed Jun 7, 2023

View reviewed changes

...ackages/backend/libs/sims-db/src/entities/application-offering-change-request-status.type.ts Outdated Show resolved Hide resolved

dheepak-aot reviewed Jun 7, 2023

View reviewed changes

...ns/src/sql/ApplicationOfferingChangeRequests/Create-application-offering-change-requests.sql Outdated Show resolved Hide resolved

dheepak-aot reviewed Jun 7, 2023

View reviewed changes

sources/packages/backend/libs/sims-db/src/entities/application-offering-change-request.model.ts Outdated Show resolved Hide resolved

feedback

d9e1430

andrepestana-aot approved these changes Jun 7, 2023

View reviewed changes

dheepak-aot approved these changes Jun 8, 2023

View reviewed changes

status name update

32f178c

sh16011993 approved these changes Jun 8, 2023

View reviewed changes

andrewsignori-aot approved these changes Jun 8, 2023

View reviewed changes

guru-aot approved these changes Jun 8, 2023

View reviewed changes

refactor

f62287d

ann-aot merged commit 7c5793f into main Jun 8, 2023

ann-aot temporarily deployed to DEV June 8, 2023 23:58 — with GitHub Actions Inactive

ann-aot temporarily deployed to DEV June 9, 2023 00:00 — with GitHub Actions Inactive

ann-aot temporarily deployed to DEV June 9, 2023 00:01 — with GitHub Actions Inactive

ann-aot temporarily deployed to DEV June 9, 2023 00:02 — with GitHub Actions Inactive

andrewsignori-aot pushed a commit that referenced this pull request Jun 9, 2023

#1953 - Request an Offering Change - DB Migrations (#1995)

776d577

#1953 - Request an Offering Change - DB Migrations (#1995)

andrewsignori-aot mentioned this pull request Jun 9, 2023

#1953 - Request an Offering Change - DB Migrations (#1995) #2004

Closed

ann-aot deleted the feature/sims-1953/migration branch June 20, 2023 23:18

		@@ -0,0 +1,2 @@
		-- DROP Table
		DROP TABLE IF EXISTS sims.application_offering_change_requests; No newline at end of file

		@@ -0,0 +1,7 @@
		CREATE TYPE sims.application_offering_change_request_status_types AS ENUM (
		'In progress with the student',

#1953 - Request an Offering Change - DB Migrations #1995

#1953 - Request an Offering Change - DB Migrations #1995

Uh oh!

Conversation

ann-aot commented Jun 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrepestana-aot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dheepak-aot Jun 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dheepak-aot Jun 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dheepak-aot commented Jun 7, 2023

Uh oh!

andrepestana-aot left a comment

Choose a reason for hiding this comment

Uh oh!

dheepak-aot left a comment

Choose a reason for hiding this comment

Uh oh!

sh16011993 left a comment

Choose a reason for hiding this comment

Uh oh!

andrewsignori-aot left a comment

Choose a reason for hiding this comment

Uh oh!

ann-aot commented Jun 7, 2023 •

edited

Loading

dheepak-aot Jun 7, 2023 •

edited

Loading

dheepak-aot Jun 7, 2023 •

edited

Loading