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

13565 Implement Missing Information alert and conversion todo item #407

Merged
merged 1 commit into from
Oct 14, 2022

Conversation

severinbeauvais
Copy link
Collaborator

@severinbeauvais severinbeauvais commented Oct 13, 2022

Issue #: bcgov/entity#13565

Description of changes:

  • removed compliance warning from "View and Change Business Info" tooltip (now handled by alert, per Tracey)
  • moved alerts from Todo List to Dashboard component
  • moved View/Hide Details button beside title for all states/types
  • added Todo List action button for conversion task for staff only
  • added Todo List content for conversion task
  • implemented loadConversionTodo()
  • added conversion code to doFileNow()
  • refactored ActionRequired component to match UI designs
  • added NotInGoodStanding component
  • added ConversionDetails component
  • included missing info warning as a blocker
  • renamed some getters
  • added Alerts section to dashboard
  • misc cleanup
  • added unit tests
  • app version = 5.7.7

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

p {
font-size: $px-15;
}
</style>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like this:

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nature of business isn't actually something we check for in the missing business info check. Is the reference to nature of business still ok?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alert looks nice at the top and closed by default btw.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If the alert needs to be changed, it will need another ticket.

p, li {
color: $gray7;
}
</style>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like this:

image

</template>
<template v-if="isNotInCompliance">
You cannot view or change business information while the business is not in compliance.
<v-icon color="orange darken-2" size="24px" class="pr-2" v-on="on">mdi-alert</v-icon>
</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.

"Not in compliance" is now an alert, so this tooltip is removed for this case (per Tracey).

This tooltip remains for "pending dissolution", except it's not implemented yet (is always false).

<!-- FUTURE: move these to a new Alerts component and inside one expansion panel -->
<MissingInformation v-if="isMissingInformationAlert"/>
<NotInGoodStanding v-if="isNotInGoodStandingAlert"/>
</section>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like this:

image

StaffNotation,
CoaWarningDialog
TodoList
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

^^ components are now sorted (no functional difference but easier to verify visually)

warnings: [{ warningType: 'COMPLIANCE' }, { warningType: 'COMPLIANCE' }],
buttonExists: true,
tooltip: 'not in compliance'
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tooltip was removed for not in compliance.

@@ -256,7 +248,6 @@ describe('EntityInfo - company info button and tooltip', () => {

store.state.entityType = _.entityType
store.state.goodStanding = _.goodStanding || false
store.state.businessWarnings = _.warnings || []
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Warnings were deleted from test array above.

@codecov
Copy link

codecov bot commented Oct 14, 2022

Codecov Report

Merging #407 (c4ccd8f) into main (46389cd) will increase coverage by 1.94%.
The diff coverage is 74.30%.

@@            Coverage Diff             @@
##             main     #407      +/-   ##
==========================================
+ Coverage   82.88%   84.83%   +1.94%     
==========================================
  Files         109      107       -2     
  Lines        2104     2143      +39     
  Branches      659      489     -170     
==========================================
+ Hits         1744     1818      +74     
+ Misses        360      325      -35     
Impacted Files Coverage Δ
src/components/AnnualReport/AGMDate.vue 73.07% <ø> (ø)
src/components/Dashboard/AddressListSm.vue 100.00% <ø> (ø)
src/components/Dashboard/CustodianListSm.vue 100.00% <ø> (ø)
src/components/Dashboard/FilingHistoryList.vue 83.33% <ø> (-0.23%) ⬇️
...ents/Dashboard/FilingHistoryList/DocumentsList.vue 100.00% <ø> (ø)
src/components/Dashboard/FirmsAddressList.vue 100.00% <ø> (ø)
.../components/Dashboard/ProprietorPartnersListSm.vue 100.00% <ø> (ø)
src/components/Dashboard/StaffNotation.vue 100.00% <ø> (ø)
...mponents/DigitalCredentials/CredentialsLanding.vue 100.00% <ø> (ø)
src/components/EntityInfo.vue 100.00% <ø> (ø)
... and 71 more

…ltip (now handled by alert, per Tracey)

- moved alerts from Todo List to Dashboard component
- moved View/Hide Details button beside title for all states/types
- added Todo List action button for conversion task for staff only
- added Todo List content for conversion task
- implemented loadConversionTodo()
- added conversion code to doFileNow()
- refactored ActionRequired component to match UI designs
- added NotInGoodStanding component
- added ConversionDetails component
- included missing info warning as a blocker
- renamed some getters
- added Alerts section to dashboard
- misc cleanup
- added unit tests
- app version = 5.7.7
@sonarcloud
Copy link

sonarcloud bot commented Oct 14, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

<v-btn v-if="showDetailsBtnRed(item)" class="expand-btn ml-1" text color="error" :ripple=false>
<v-icon>mdi-information-outline</v-icon>
<span>{{ (panel === index) ? "Hide Details" : "View Details" }}</span>
</v-btn>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like this:

image

image

Instead of this:

image

image

Expanded details:

image

image

This change was OKed by Yui.


<!-- draft with pay error -->
<div v-else-if="isStatusDraft(item) && isPayError(item)" class="todo-subtitle">
<span>PAYMENT INCOMPLETE</span>
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 up as it supersedes all other possible draft statuses.

<div v-else-if="isStatusDraft(item) && isTypeAlteration(item) && !item.goodStanding"
class="todo-list-detail body-2"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bugfix.

>
<v-icon class="pr-1" color="primary" size="18px">mdi-delete-forever</v-icon>
<v-list-item-title>Delete Draft</v-list-item-title>
<v-list-item-title>Delete draft</v-list-item-title>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is in line with "Delete changes to company information" (line 259).

id="btn-delete-draft-alt"
@click="confirmDeleteDraft(item)"
id="btn-draft-delete"
@click.stop="confirmDeleteDraft(item)"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bugfix: this stops the action from bubbling up to parent elements (which eventually causes this panel to open or close, unexpectedly).

@@ -493,7 +476,7 @@
</v-expansion-panels>

<!-- No Results Message -->
<v-card class="no-results" flat v-if="!showTodoPanel && !isActionRequired">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This panel may still display even if an action is required (which is now a "missing information" alert).

if (this.isStatusError(item) && (this.inProcessFiling !== item.filingId)) return true
if (this.isStatusPaid(item) && (this.inProcessFiling !== item.filingId)) return true
return false
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are to simplify the template.

@@ -680,11 +680,8 @@ export default class TodoList extends Vue {
}
}

// Add todo items count +1 when SP/GP with compliance warning
const todoLength = this.isActionRequired ? this.todoItems.length + 1 : this.todoItems.length
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was because the previous panel counted as a Todo item. Now that panel (alert) is in its own section above the Todo section.

let subtitle: string = null
if (task.enabled && !this.isBComp) {
subtitle = '(including Address and/or Director Change)'
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Easier-to-understand logic!

@severinbeauvais severinbeauvais marked this pull request as ready for review October 14, 2022 02:54
Copy link
Contributor

@lambert-alex lambert-alex left a comment

Choose a reason for hiding this comment

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

Looks awesome!

Copy link
Contributor

@lewischenstudio lewischenstudio left a comment

Choose a reason for hiding this comment

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

LGTM.

@severinbeauvais severinbeauvais merged commit 56e9c44 into bcgov:main Oct 14, 2022
@argush3
Copy link
Collaborator

argush3 commented Oct 14, 2022

A little late to this review but looks good. Just the one thing I brought up.

Nice to finally see this in action. Being able to actually see what's missing is a nice touch too!

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.

None yet

4 participants