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

19202 Removed details from Admin Freeze/Unfreeze filings #640

Merged
merged 2 commits into from
Mar 28, 2024

Conversation

severinbeauvais
Copy link
Collaborator

@severinbeauvais severinbeauvais commented Mar 27, 2024

Issue #: bcgov/entity#19202

Description of changes:

  • app version = 7.1.7
  • removed unneeded code from TodoLlist.vue
  • added paragraph
  • hide notation form
  • don't call ref methods if ref doesn't exist
  • don't save details
  • updated unit tests

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

- added paragraph
- hide notation form
- don't call ref methods if ref doesn't exist
- don't save `details`
- updated unit tests
- removed unneeded code from TodoLlist.vue
@severinbeauvais severinbeauvais self-assigned this Mar 27, 2024
@@ -1019,9 +1019,6 @@ export default class TodoList extends Mixins(AllowableActionsMixin, DateMixin, E

if (header) {
switch (header.name) {
case FilingTypes.ADMIN_FREEZE:
// do nothing for admin_freeze
break
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Admin Freeze/Unfreeze filings will never appear in the Todo List because they cannot be saved as a draft.

<v-divider
v-if="isCourtOrder"
class="mt-4"
/>
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 moved this divider into the template block above (same v-if).

If this filing is pursuant to a court order, enter the court order number. If this filing is pursuant
to a plan of arrangement, enter the court order number and select Plan of Arrangement.
</p>
<template v-if="!isAdministerFreeze">
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 wrapped both of these in a template with a single v-if, since they go together anyway,

@@ -243,7 +243,7 @@ export default class AddStaffNotationDialog extends Mixins(DateMixin) {
}

get notationLabel (): string {
if (this.isAdministrativeDissolution || this.isPutBackOn || this.isAdministerFreeze) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's no notation label for admin freeze.

@@ -258,7 +258,7 @@ export default class AddStaffNotationDialog extends Mixins(DateMixin) {
// Administrative Dissolution, Put Back On, Freeze/Unfreeze Business require a detailed comment
(v: string) => (
!!v ||
(!this.isAdministrativeDissolution && !this.isPutBackOn && !this.isAdministerFreeze) ||
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's no component, and thus no rules, for admin freeze. Ditto line 274 below.

const isCourtOrderPoaValid = !this.isAdministerFreeze ? this.$refs.courtOrderPoaRef.validate() : true
const isNotationFormValid = (!this.$refs.notationFormRef || this.$refs.notationFormRef.validate())
const isFileComponentValid = (!this.$refs.fileUploadRef || this.$refs.fileUploadRef.validate())
const isCourtOrderPoaValid = (!this.$refs.courtOrderPoaRef || this.$refs.courtOrderPoaRef.validate())
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 above are each True if the component (ref) doesn't exist (and do not validate them).

@@ -405,7 +405,6 @@ export default class AddStaffNotationDialog extends Mixins(DateMixin) {
}
} else if (this.isAdministerFreeze) {
filing[FilingTypes.ADMIN_FREEZE] = {
details: this.notation,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do not save details.

Copy link
Contributor

@jamespaologarcia jamespaologarcia left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

sonarcloud bot commented Mar 27, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

@@ -309,10 +309,10 @@ export default class AddStaffNotationDialog extends Mixins(DateMixin) {
this.enableValidation = false

// reset notation form
this.$refs.notationFormRef.reset()
this.$refs.notationFormRef?.reset()
Copy link
Collaborator Author

@severinbeauvais severinbeauvais Mar 27, 2024

Choose a reason for hiding this comment

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

SonarCloud recommended using optional chaining instead of conditional syntax.

(Previous code was this.$refs.notationFormRef && this.$refs.notationFormRef.reset())

@severinbeauvais
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

bcregistry-sre commented Mar 27, 2024

@bcgov bcgov deleted a comment from bcregistry-sre Mar 27, 2024
Copy link
Contributor

@PaulGarewal PaulGarewal left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@severinbeauvais severinbeauvais merged commit 5cc8a18 into bcgov:main Mar 28, 2024
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.

6 participants