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

24659 - Implement Withdraw Action in Ledger #119

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

meawong
Copy link
Collaborator

@meawong meawong commented Jan 13, 2025

*Issue:*bcgov/entity#24659

Description of changes:

  • Implement withdraw action on ledger

Try:
Regular Businesss: https://business-dashboard-dev--pr-119-aql2sz14.web.app/BC0883803/?accountid=3627&accountid=3627
Bootstrap Business: https://business-dashboard-dev--pr-119-aql2sz14.web.app/TJes1p4Er5/?accountid=3627&accountid=3627

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

@meawong
Copy link
Collaborator Author

meawong commented Jan 13, 2025

/gcbrun

1 similar comment
@ketaki-deodhar
Copy link
Collaborator

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-dashboard-dev--pr-119-aql2sz14.web.app

pnpm-lock.yaml Outdated Show resolved Hide resolved
@@ -167,7 +172,7 @@ const correctionFormSubmit = async function () {
filingError.value = 'Unable to get correction filing id'
return
}
const path = `/${currentBusiness.value.identifier}/correction/`
const path = `/${currentBusinessIdentifier.value}/correction/`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please call out this fix in your ticket so that QA knows to verify this.

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Jan 13, 2025

Did you determine that it's possible to withdraw an address change? see response below

image

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Jan 13, 2025

Did you determine that the action should be disabled, instead of not rendered, when it's not applicable? see response below

image

@severinbeauvais
Copy link
Collaborator

@patrickpeinanw Any idea why the setup-job check failed?

@meawong
Copy link
Collaborator Author

meawong commented Jan 14, 2025

@patrickpeinanw Any idea why the setup-job check failed?

related to: https://www.githubstatus.com/?
image

@meawong
Copy link
Collaborator Author

meawong commented Jan 14, 2025

Did you determine that the action should be disabled, instead of not rendered, when it's not applicable?

image

confirmed with Jacqueline that the design is implemented as expected

@meawong
Copy link
Collaborator Author

meawong commented Jan 14, 2025

Did you determine that it's possible to withdraw an address change?

image

yes, Jacqueline confirmed that we can withdraw any FE filing

@meawong
Copy link
Collaborator Author

meawong commented Jan 14, 2025

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-dashboard-dev--pr-119-aql2sz14.web.app

@patrickpeinanw
Copy link
Collaborator

@severinbeauvais I recommend reverting back to the old dependencies just like in PR #118. If you want to update the lock files, it can be done in a separate PR and we need to ensure all tests are passing locally.

@meawong
Copy link
Collaborator Author

meawong commented Jan 14, 2025

/gcbrun

@bcregistry-sre
Copy link
Collaborator

bcregistry-sre commented Jan 14, 2025

Temporary Url for review: https://business-dashboard-dev--pr-119-aql2sz14.web.app

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. Arwen's PR is now merged, so please rebase and update the app version.

Also, please get another approval.

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.

Great stuff! 👍

@meawong meawong force-pushed the 24659-Implement-Withdraw-File-Action branch from 022f82f to cb83a0b Compare January 14, 2025 22:07
@meawong
Copy link
Collaborator Author

meawong commented Jan 14, 2025

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-dashboard-dev--pr-119-aql2sz14.web.app

Copy link
Collaborator

@patrickpeinanw patrickpeinanw left a comment

Choose a reason for hiding this comment

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

Nice work

package.json Outdated
@@ -2,7 +2,7 @@
"name": "bcros-business-dashboard",
"private": true,
"type": "module",
"version": "1.0.1",
"version": "1.0.2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please rebase again. The version is already 1.0.2.

https://github.com/bcgov/business-dashboard-ui/blob/main/package.json

@meawong
Copy link
Collaborator Author

meawong commented Jan 14, 2025

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-dashboard-dev--pr-119-aql2sz14.web.app

@meawong meawong merged commit 4f5d4db into bcgov:main Jan 14, 2025
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