22072 Show previous correspondences + cleanup #2907

severinbeauvais · 2024-07-11T20:31:58Z

Issue #: bcgov/entity#22072

Description of changes:

Commit 1:

app version = 2.6.51
imported vue-auto-resize directive for textarea
added typing to ContinuationApplicationTable.vue
separated ContinuationAuthorizationReviewResult.vue into 2 new sub-components
refactored continuation review object into separate review and filing objects/props
now use for loop for previous correspondence
use label for correspondences header
use textarea for correspondence comment
compute correspondence items according to review/results
updated Review Result dropdown items
reset validation when new Review Result is selected
renamed ContinuationReviewStatus -> ReviewStatus
changed ACCEPTED to APPROVED
updated interfaces to match API responses
added fetchFiling()
updated mocked API responses
added dateToPacificTime() and apiToPacificDateTime()
renamed misc things
swapped main review section icons per Andy
refactored data fetching
added return to staff dashboard on error dialog close
added/updated unit tests

Commit 2:

refactored date conversions to use Moment
deleted date-utils.ts

Commit 3:

transformed ContinuationAuthorizationReview.vue to Composition API
transformed ExtraprovincialRegistrationBc.vue to Composition API
transformed HomeJurisdictionInformation.vue to Composition API
transformed PreviousCorrespondence.vue to Composition API
transformed ReviewResult.vue to Composition API
misc cleanup

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the sbc-auth license (Apache 2.0).

severinbeauvais · 2024-07-12T22:17:36Z

Screenshots for review id=1

https://bcregistry-account-dev--pr-2907-cwo0803q.web.app/staff/continuation-review/1

severinbeauvais · 2024-07-12T22:19:15Z

Screenshots for review id=2

https://bcregistry-account-dev--pr-2907-cwo0803q.web.app/staff/continuation-review/2

ArwenQin

I reviewed the codes, make sense to me!

PaulGarewal

LGTM 👍

severinbeauvais · 2024-07-15T17:33:28Z

deleted obsolete comment related to "account-ui-ci-pr" job

severinbeauvais · 2024-07-15T17:37:24Z

@JohnamLane I have 2 approvals but merging is blocked. What's the process to merge this PR?

severinbeauvais · 2024-07-15T17:43:25Z

auth-web/src/services/business.services.ts

@@ -239,33 +202,35 @@ export default class BusinessService {
    })
  }

+  // *** TODO: this should return type "array of reviews"


@ArwenQin FYI ^^

seeker25

Use composition-api and moment, not your own rolled date library.

We spent a whole bunch of time converting components to composition-api, you're undoing that process by writing them in class-api.

You're also introducing more tech debt by using your own date library, when we are using moment in auth-web.

Karim has written his components in composition-api, it was a requirement before merging.

severinbeauvais · 2024-07-16T16:11:45Z

Use composition-api and moment, not your own rolled date library.

We spent a whole bunch of time converting components to composition-api, you're undoing that process by writing them in class-api.

You're also introducing more tech debt by using your own date library, when we are using moment in auth-web.

Karim has written his components in composition-api, it was a requirement before merging.

I would like Composition API a lot more if it properly supported typing, but:

and then

It is my understanding that Composition API is not required for Vue3 (or Nuxt), so why change and take a hit along the way?

As for the date-time manipulations, this may be our own rolled up library, but it's been in use since 2019, and I'm going to dig in my heels a bit on Moment because what I've researched says to no longer use it. In fact, using Moment seems to be incurring tech debt. If we are dead-set against using native JS for date-time manipulations then we should look for a modern, light alternative to Moment.

Secondly, since our code is temporarily hosted in Auth Web, I want to make it easy to port out to another UI, so reusing the same date-time methods as the other Entity UIs seems like a good way to keep things consistent.

I agree with you on the goal of modern, simple code, but I think changing to Composition API and Moment are not a step ahead right now. If your research proves me wrong, please let me know. I am making my decisions based on what I know but I'm happy to learn more and come up with better solutions.

seeker25 · 2024-07-16T16:14:07Z

The requirements aren't up for discussion, unfortunately - composition-api and moment (consistent with the rest of the repo) or this isn't getting merged

severinbeauvais · 2024-07-16T16:17:51Z

The requirements aren't up for discussion, unfortunately - composition-api and moment (consistent with the rest of the repo) or this isn't getting merged

Please provide a reference to the requirements. Thank you.

seeker25 · 2024-07-16T16:18:17Z

Here they are: Composition-api and moment

severinbeauvais · 2024-07-17T18:49:06Z

@seeker25 So this is the library that you want me to use for date/time manipulations, even though it's a legacy project, now in maintenance mode, and they say we should choose a different library?

seeker25 · 2024-07-17T20:57:07Z

Yeah we're using Luxon for Vue3 FAS-UI, it's fairly straight forward to convert...

const apiToPacificDateTime = (apiDateTime: string): string => {
  if (!apiDateTime) return apiDateTime
  return DateTime.fromISO(apiDateTime).setZone('America/Vancouver').toFormat('fff')
}

- imported vue-auto-resize directive for textarea - added typing to ContinuationApplicationTable.vue - separated ContinuationAuthorizationReviewResult.vue into 2 new sub-components - refactored continuation review object into separate review and filing objects/props - now use 'for' loop for previous correspondence - use label for correspondences header - use textarea for correspondence comment - compute correspondence items according to review/results - updated Review Result dropdown items - reset validation when new Review Result is selected - renamed ContinuationReviewStatus -> ReviewStatus - changed ACCEPTED to APPROVED - updated interfaces to match API responses - added fetchFiling() - replaced mocked API responses with actual responses - added dateToPacificTime() and apiToPacificDateTime() - renamed misc things - swapped main review section icons per Andy - refactored data fetching - added return to staff dashboard on error dialog close - added/updated unit tests - replaced mocked submitContinuationReviewResult() with real Axios call

auth-web/src/components/auth/staff/continuation-application/ExtraprovincialRegistrationBc.vue

auth-web/src/components/auth/staff/continuation-application/HomeJurisdictionInformation.vue

auth-web/src/views/auth/staff/ContinuationAuthorizationReview.vue

severinbeauvais · 2024-07-18T00:09:12Z

~~I'll fix the unit test errors tomorrow!~~ fixed!

- changed get -> computed

- changed America/Vancouver conversion from toLocaleString() to tz()

- fixed unit tests

severinbeauvais · 2024-07-18T17:31:11Z

/gcbrun

severinbeauvais · 2024-07-18T17:34:38Z

auth-web/tests/unit/components/ExtraprovincialRegistrationBc.spec.ts

severinbeauvais · 2024-07-18T17:34:59Z

auth-web/tests/unit/components/HomeJurisdictionInformation.spec.ts

bcregistry-sre · 2024-07-18T17:35:15Z

Temporary Url for review: https://bcregistry-account-dev--pr-2907-cwo0803q.web.app

SB says, go to https://bcregistry-account-dev--pr-2907-cwo0803q.web.app/staff/dashboard/active and View/Review any of the Continuation Applications listed.

severinbeauvais · 2024-07-18T17:35:18Z

auth-web/tests/unit/components/PreviousCorrespondence.spec.ts

severinbeauvais · 2024-07-18T17:35:52Z

auth-web/tests/unit/components/ReviewResult.spec.ts

severinbeauvais · 2024-07-18T17:36:23Z

auth-web/tests/unit/views/ContinuationAuthorizationReview.spec.ts

severinbeauvais · 2024-07-18T17:56:34Z

@seeker25 Ready for re-review and hopefully, merging :)

seeker25 · 2024-07-18T18:09:47Z

auth-web/src/components/auth/staff/continuation-application/ContinuationApplications.vue

+    ContinuationApplicationTable
+  }
+})
+export default class ContinuationApplications extends Vue {}


still class-api?

Arwen worked on this component. I'll ask her to transform it to Composition API.

@ArwenQin ^^

Got it. Can I do it in ticket #21983 (the enhanced table ticket I'm working on)?

It's a pretty simple one, lets just merge I caused you enough side tracking

Arwen - yes, do it in 21983. Perfect.

auth-web/src/components/auth/staff/continuation-application/HomeJurisdictionInformation.vue

auth-web/src/components/auth/staff/continuation-application/ExtraprovincialRegistrationBc.vue

auth-web/src/components/auth/staff/continuation-application/PreviousCorrespondence.vue

seeker25

LGTM

severinbeauvais added the Do Not Merge label Jul 11, 2024

severinbeauvais self-assigned this Jul 11, 2024

severinbeauvais force-pushed the 22072 branch 2 times, most recently from 86b116a to ba84544 Compare July 12, 2024 21:45

severinbeauvais removed the Do Not Merge label Jul 12, 2024

severinbeauvais force-pushed the 22072 branch from ba84544 to 7567a42 Compare July 12, 2024 21:57

severinbeauvais marked this pull request as ready for review July 12, 2024 21:57

severinbeauvais requested review from ArwenQin, vysakh-menon and PaulGarewal July 12, 2024 21:57

ArwenQin approved these changes Jul 12, 2024

View reviewed changes

PaulGarewal approved these changes Jul 15, 2024

View reviewed changes

severinbeauvais force-pushed the 22072 branch 2 times, most recently from e9f270c to 3dd0313 Compare July 15, 2024 17:29

severinbeauvais commented Jul 15, 2024

View reviewed changes

seeker25 requested changes Jul 16, 2024

View reviewed changes

severinbeauvais force-pushed the 22072 branch from 0985bd8 to a6142a9 Compare July 16, 2024 16:02

bcgov deleted a comment from bcregistry-sre Jul 16, 2024

severinbeauvais force-pushed the 22072 branch from 831c211 to da9a20a Compare July 17, 2024 23:41

severinbeauvais commented Jul 17, 2024

View reviewed changes

auth-web/src/components/auth/staff/continuation-application/ExtraprovincialRegistrationBc.vue Outdated Show resolved Hide resolved

severinbeauvais commented Jul 17, 2024

View reviewed changes

auth-web/src/components/auth/staff/continuation-application/HomeJurisdictionInformation.vue Show resolved Hide resolved

severinbeauvais commented Jul 17, 2024

View reviewed changes

auth-web/src/components/auth/staff/continuation-application/HomeJurisdictionInformation.vue Show resolved Hide resolved

- misc cleanup

b1788d7

bcgov deleted a comment from bcregistry-sre Jul 18, 2024

severinbeauvais commented Jul 18, 2024

View reviewed changes

auth-web/src/views/auth/staff/ContinuationAuthorizationReview.vue Show resolved Hide resolved

bcgov deleted a comment from bcregistry-sre Jul 18, 2024

Severin Beauvais added 4 commits July 18, 2024 09:35

- changed this -> state

423bf31

- changed get -> computed

- imported package moment-timezone

20df3db

- changed America/Vancouver conversion from toLocaleString() to tz()

- implemented navigation guard for unsaved changes

9e7005f

- fixed bugs

2c9fa99

- fixed unit tests

bcgov deleted a comment from bcregistry-sre Jul 18, 2024

severinbeauvais commented Jul 18, 2024

View reviewed changes

auth-web/tests/unit/components/ExtraprovincialRegistrationBc.spec.ts Outdated

Copy link

Collaborator Author

severinbeauvais Jul 18, 2024

Choose a reason for hiding this comment

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

severinbeauvais commented Jul 18, 2024

View reviewed changes

auth-web/tests/unit/components/HomeJurisdictionInformation.spec.ts Outdated

Copy link

Collaborator Author

severinbeauvais Jul 18, 2024

Choose a reason for hiding this comment

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

severinbeauvais commented Jul 18, 2024

View reviewed changes

auth-web/tests/unit/components/PreviousCorrespondence.spec.ts Outdated

Copy link

Collaborator Author

severinbeauvais Jul 18, 2024

Choose a reason for hiding this comment

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

severinbeauvais commented Jul 18, 2024

View reviewed changes

auth-web/tests/unit/components/ReviewResult.spec.ts Outdated

Copy link

Collaborator Author

severinbeauvais Jul 18, 2024

Choose a reason for hiding this comment

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

severinbeauvais commented Jul 18, 2024

View reviewed changes

auth-web/tests/unit/views/ContinuationAuthorizationReview.spec.ts Outdated

Copy link

Collaborator Author

severinbeauvais Jul 18, 2024

Choose a reason for hiding this comment

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

bcgov deleted a comment from bcregistry-sre Jul 18, 2024

- disabled unit tests dates that don't work correctly in CI

b3cec75

severinbeauvais force-pushed the 22072 branch from bbef589 to b3cec75 Compare July 18, 2024 17:49

seeker25 reviewed Jul 18, 2024

View reviewed changes

auth-web/src/components/auth/staff/continuation-application/HomeJurisdictionInformation.vue Show resolved Hide resolved

seeker25 reviewed Jul 18, 2024

View reviewed changes

auth-web/src/components/auth/staff/continuation-application/ExtraprovincialRegistrationBc.vue Show resolved Hide resolved

seeker25 reviewed Jul 18, 2024

View reviewed changes

auth-web/src/components/auth/staff/continuation-application/PreviousCorrespondence.vue Show resolved Hide resolved

seeker25 approved these changes Jul 18, 2024

View reviewed changes

severinbeauvais merged commit 7076b5b into bcgov:main Jul 18, 2024
4 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

22072 Show previous correspondences + cleanup #2907

22072 Show previous correspondences + cleanup #2907

severinbeauvais commented Jul 11, 2024 •

edited

Loading

severinbeauvais commented Jul 12, 2024 •

edited

Loading

severinbeauvais commented Jul 12, 2024 •

edited

Loading

ArwenQin left a comment

PaulGarewal left a comment

severinbeauvais commented Jul 15, 2024 •

edited

Loading

severinbeauvais commented Jul 15, 2024

severinbeauvais Jul 15, 2024

seeker25 left a comment •

edited

Loading

severinbeauvais commented Jul 16, 2024

seeker25 commented Jul 16, 2024 •

edited

Loading

severinbeauvais commented Jul 16, 2024

seeker25 commented Jul 16, 2024

severinbeauvais commented Jul 17, 2024

seeker25 commented Jul 17, 2024 •

edited

Loading

severinbeauvais commented Jul 18, 2024 •

edited

Loading

severinbeauvais commented Jul 18, 2024

severinbeauvais Jul 18, 2024

severinbeauvais Jul 18, 2024

bcregistry-sre commented Jul 18, 2024 •

edited by severinbeauvais

Loading

severinbeauvais Jul 18, 2024

severinbeauvais Jul 18, 2024

severinbeauvais Jul 18, 2024

severinbeauvais commented Jul 18, 2024

seeker25 Jul 18, 2024

severinbeauvais Jul 18, 2024

severinbeauvais Jul 18, 2024

ArwenQin Jul 18, 2024

seeker25 Jul 18, 2024

severinbeauvais Jul 18, 2024

seeker25 left a comment

22072 Show previous correspondences + cleanup #2907

22072 Show previous correspondences + cleanup #2907

Conversation

severinbeauvais commented Jul 11, 2024 • edited Loading

severinbeauvais commented Jul 12, 2024 • edited Loading

Screenshots for review id=1

severinbeauvais commented Jul 12, 2024 • edited Loading

Screenshots for review id=2

ArwenQin left a comment

Choose a reason for hiding this comment

PaulGarewal left a comment

Choose a reason for hiding this comment

severinbeauvais commented Jul 15, 2024 • edited Loading

severinbeauvais commented Jul 15, 2024

Choose a reason for hiding this comment

seeker25 left a comment • edited Loading

Choose a reason for hiding this comment

severinbeauvais commented Jul 16, 2024

seeker25 commented Jul 16, 2024 • edited Loading

severinbeauvais commented Jul 16, 2024

seeker25 commented Jul 16, 2024

severinbeauvais commented Jul 17, 2024

seeker25 commented Jul 17, 2024 • edited Loading

severinbeauvais commented Jul 18, 2024 • edited Loading

severinbeauvais commented Jul 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcregistry-sre commented Jul 18, 2024 • edited by severinbeauvais Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

severinbeauvais commented Jul 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seeker25 left a comment

Choose a reason for hiding this comment

severinbeauvais commented Jul 11, 2024 •

edited

Loading

severinbeauvais commented Jul 12, 2024 •

edited

Loading

severinbeauvais commented Jul 12, 2024 •

edited

Loading

severinbeauvais commented Jul 15, 2024 •

edited

Loading

seeker25 left a comment •

edited

Loading

seeker25 commented Jul 16, 2024 •

edited

Loading

seeker25 commented Jul 17, 2024 •

edited

Loading

severinbeauvais commented Jul 18, 2024 •

edited

Loading

bcregistry-sre commented Jul 18, 2024 •

edited by severinbeauvais

Loading