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

16577 - Alteration - Name Request changes #518

Merged
merged 15 commits into from
Jul 25, 2023
Merged

Conversation

seeker25
Copy link
Collaborator

@seeker25 seeker25 commented Jul 24, 2023

Issue #: /bcgov/entity#16577

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

@seeker25 seeker25 requested a review from Jxio July 24, 2023 19:35
@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Merging #518 (8fdcd40) into main (e481296) will increase coverage by 0.10%.
Report is 4 commits behind head on main.
The diff coverage is 100.00%.

❗ Current head 8fdcd40 differs from pull request most recent head 696bfc3. Consider uploading reports for the commit 696bfc3 to get more accurate results

@@            Coverage Diff             @@
##             main     #518      +/-   ##
==========================================
+ Coverage   87.11%   87.21%   +0.10%     
==========================================
  Files         207      207              
  Lines        3919     3935      +16     
  Branches      510      510              
==========================================
+ Hits         3414     3432      +18     
+ Misses        491      489       -2     
  Partials       14       14              
Files Changed Coverage Δ
src/store/state/state-model.ts 100.00% <ø> (ø)
...mponents/common/YourCompany/ChangeBusinessType.vue 100.00% <100.00%> (ø)
...mon/YourCompany/CorrectName/CorrectNameRequest.vue 100.00% <100.00%> (ø)
src/components/common/YourCompany/EntityName.vue 100.00% <100.00%> (ø)
src/store/store.ts 83.31% <100.00%> (+0.40%) ⬆️

@seeker25 seeker25 changed the title 16577 - Fix getUpdatedName, implement nameRequestRequiredError for non-number… 16577 - Alteration - Name Request related changes Jul 24, 2023
@seeker25 seeker25 changed the title 16577 - Alteration - Name Request related changes 16577 - Alteration - Name Request changes Jul 24, 2023
@seeker25 seeker25 marked this pull request as ready for review July 24, 2023 22:55
Copy link
Collaborator

@Jxio Jxio left a comment

Choose a reason for hiding this comment

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

LGTM

`${GetCorpFullDescription(nr.entity_type_cd)} to ${GetCorpFullDescription(nr.legalType)} ` +
`does not match the current business type ` +
`<b>${GetCorpFullDescription(this.getEntityType)}</b>.\n\n` +
`The Name Request type must match the business type before you can continue.</p>`
Copy link
Collaborator

Choose a reason for hiding this comment

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

When using the back-tick for strings, you need to add lines together. You can do this:

 dialogContent = `line 1
    line 2
    line 3`

See if it looks cleaner.

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'll try this one in the next PR - after QA

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

LGTM Travis!

@seeker25
Copy link
Collaborator Author

Thank you both

@seeker25
Copy link
Collaborator Author

image

image

@seeker25 seeker25 merged commit 7f2c6f4 into bcgov:main Jul 25, 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