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

15092 - remove fee summary components from App.vue add to each view instead #458

Merged
merged 3 commits into from
Mar 18, 2023

Conversation

jonathan-longe
Copy link
Contributor

@jonathan-longe jonathan-longe commented Mar 17, 2023

Issue #: /bcgov/entity#15092

This is an incremental PR as the changes are potentially disruptive. The PR removes the fee summaries from App.vue and moves them to a ViewWrapper component that's added to the following views:

  • Alteration.vue
  • Change.vue
  • Conversion.vue
  • Correction.vue
  • Restoration.vue
  • SpecialResolution.vue

By removing the fee summary components from App.vue it gives us more flexibility to handle the actions from FeeSummaryShared differently.

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

@jonathan-longe jonathan-longe self-assigned this Mar 17, 2023
Comment on lines +5 to +7
<v-col cols="9" class="left-side">
<slot></slot>
</v-col>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ViewWrapper takes a significant chunk of the template from App.vue. Here we're using a <slot> to allow the views to add back their own content with the minimum of disruption.

@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Merging #458 (95b5bcd) into main (eaf687f) will decrease coverage by 1.65%.
The diff coverage is 82.07%.

@@            Coverage Diff             @@
##             main     #458      +/-   ##
==========================================
- Coverage   85.70%   84.06%   -1.65%     
==========================================
  Files         177      186       +9     
  Lines        3289     3558     +269     
  Branches      524      711     +187     
==========================================
+ Hits         2819     2991     +172     
- Misses        468      566      +98     
+ Partials        2        1       -1     
Impacted Files Coverage Δ
...mponents/Alteration/Articles/CompanyProvisions.vue 100.00% <ø> (ø)
src/components/common/ErrorContact.vue 100.00% <ø> (ø)
...ourCompany/NameTranslations/AddNameTranslation.vue 100.00% <ø> (ø)
...nents/common/YourCompany/NameTranslations/index.ts 100.00% <ø> (ø)
src/dialogs/StaffPaymentErrorDialog.vue 100.00% <ø> (ø)
src/services/legal-services.ts 93.22% <0.00%> (-6.78%) ⬇️
src/utils/feature-flag-utils.ts 15.78% <ø> (ø)
src/mixins/filing-template-mixin.ts 39.00% <19.17%> (-5.08%) ⬇️
src/utils/FetchConfig.ts 95.12% <50.00%> (-2.32%) ⬇️
src/mixins/common-mixin.ts 82.95% <78.57%> (+6.21%) ⬆️
... and 81 more

Comment on lines +2 to +3
<ViewWrapper>
<section class="pb-10" id="alteration-view">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The wrapper is added in each view like this.

Comment on lines +41 to -80
<AlterationSummary
class="mt-10"
:sectionNumber="showTransactionalFolioNumber ? '4.' : '3.'"
:autoValidation="getAppValidate"
:validate="getAppValidate"
@haveChanges="onAlterationSummaryChanges()"
/>

<StaffPayment
<DocumentsDelivery
class="mt-10"
:sectionNumber="showTransactionalFolioNumber ? '5.' : '4.'"
@haveChanges="onStaffPaymentChanges()"
sectionNumber="1."
:validate="getAppValidate"
@valid="setDocumentOptionalEmailValidity($event)"
/>
</template>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hasn't changed. It's just Github being dumb with how it recognizes changes.

Comment on lines -654 to -669

it('renders the fee summary properly following changes', async () => {
store.state.stateModel.tombstone.entityType = 'SP'
store.state.stateModel.tombstone.filingType = 'changeOfRegistration'
store.state.stateModel.entitySnapshot = mockEntitySnapshot
store.state.stateModel.officeAddresses = mockAddresses
store.state.stateModel.filingData = {
filingTypeCode: 'FMCHANGE',
entityType: 'SP',
priority: false,
waiveFees: false
}
await Vue.nextTick()

expect(wrapper.findComponent(FeeSummaryShared).exists()).toBe(true) // not displayed initially
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this test as FeeSummaryShared is no longer rendered with App.vue

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.

LGTM! Nice and concise!

Copy link
Collaborator

@ketaki-deodhar ketaki-deodhar 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. Clean!

Copy link
Collaborator

@JazzarKarim JazzarKarim left a comment

Choose a reason for hiding this comment

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

Love it. No complaints here!

@jonathan-longe jonathan-longe merged commit e18a1c7 into bcgov:main Mar 18, 2023
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