Skip to content
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

19024 - Implemented the Involuntary Dissolution Screen #2805

Merged
merged 8 commits into from
May 7, 2024

Conversation

JazzarKarim
Copy link
Collaborator

@JazzarKarim JazzarKarim commented May 7, 2024

Issue #:
bcgov/entity#19024

Designs: https://www.figma.com/file/LcP4NgVYKVxEk8V7fXWIuc/Involuntary-Dissolution-%26-Delay-of-Dissolution?type=design&node-id=390-3159&mode=design

Description of changes:

  • Implemented the Involuntary Dissolution Screen
  • Implemented all phases (pause, resume, edit)
  • Implemented the badge
  • Implemented validation
  • Lots of misc. stuff

staff dissolution batch paused

staff dissolution batch edit

NOTE: Unit tests will be done in bcgov/entity#21154

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).

@JazzarKarim JazzarKarim self-assigned this May 7, 2024
@JazzarKarim JazzarKarim force-pushed the 19024-invol-dissolution-screen branch from 4750ce2 to 1495c48 Compare May 7, 2024 17:55
@@ -0,0 +1,64 @@
<template>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Card header:
card header

>
<span>
Moving <strong>{{ scheduleSummaryNumber }}</strong> businesses into D1 dissolution every
<strong>Tuesday</strong> at <strong>11:59 p.m</strong> Pacific Time.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Tuesday and hour here will probably change later.


@Component({
methods: {
...mapActions(useStaffStore,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We usually use the @Getter and @Action in our entities codebases but in here, I'm just following the coding practice of relationships to stay uniform across all files 😃

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was going to comment on this, not because I think mapActions() is bad, but because you also need to declare the method signatures on lines 158-161. DRY if possible.

menu = false
numberOfBusinesses = 0
isEdit = false
isOnHold = false
Copy link
Collaborator Author

@JazzarKarim JazzarKarim May 7, 2024

Choose a reason for hiding this comment

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

You may notice that I'm using the onHold a lot (instead of isRunning for example). This is because, from the configurations table (got this from Argus), we'll be getting something like this:

{
  "configurations": [
    {
      "name": "NUM_DISSOLUTIONS_ALLOWED",
      "value": "100"
    },
    {
      "name": "DISSOLUTIONS_ON_HOLD",
      "value": false
    },
   ...
  ]
}

So, I'm using the on hold term.

@bcgov bcgov deleted a comment from bcregistry-sre May 7, 2024
@JazzarKarim
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

bcregistry-sre commented May 7, 2024

@JazzarKarim JazzarKarim marked this pull request as ready for review May 7, 2024 18:16
@@ -20,7 +20,7 @@
"@bcrs-shared-components/corp-type-module": "1.0.16",
"@bcrs-shared-components/enums": "1.1.10",
"@bcrs-shared-components/interfaces": "1.1.13",
"@mdi/font": "^4.5.95",
"@mdi/font": "^5.9.55",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ochiu Ody, I noticed this change was reverted at one point: #2716

What kind of issues were you running into exactly? I'm thinking I need to change this because I need the mdi-delete-clock icon in the CardHeader (line 45 in the InvoluntaryDissolution.vue). Please let me know!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@JazzarKarim I believe there were icons we are currently using that have been removed in newer versions, so I had to revert the changes as it affected other screens.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Example:

Your PR:
Screenshot 2024-05-07 at 11 23 50 AM

Dev:
Screenshot 2024-05-07 at 11 24 20 AM

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see, OK. Got it. Thank you so much Ody.

I will talk to our UX designer and will see if we can use something else in this case. Appreciate it!

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can also pull in icons separately if the font doesn't have them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted this change to not break other stuff as Ody mentioned.

Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

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

I don't want to slow you down. Looks decent and it seems you've thought about my comments so I'll leave it to you to update if/as needed.

Copy link

sonarqubecloud bot commented May 7, 2024

Quality Gate Passed Quality Gate passed

Issues
11 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@JazzarKarim
Copy link
Collaborator Author

Got the OK to merge from Travis. Merging now.

@JazzarKarim JazzarKarim merged commit b99a207 into bcgov:main May 7, 2024
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants