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

22561 Update ledger body for FE IA / Amalgamation / Continuation In / etc #124

Merged
merged 9 commits into from
Jan 22, 2025

Conversation

kzdev420
Copy link
Collaborator

*Issue:*bcgov/entity#22561

Description of changes:

Update FE ledger section

  • spacing between text
  • FE section title
  • language

Update contact information

  • Include hours of operation

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

@severinbeauvais severinbeauvais assigned kzdev420 and unassigned kzdev420 Jan 20, 2025
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

@bcgov bcgov deleted a comment from bcregistry-sre Jan 21, 2025
@bcgov bcgov deleted a comment from bcregistry-sre Jan 21, 2025
@bcgov bcgov deleted a comment from kzdev420 Jan 21, 2025
@bcgov bcgov deleted a comment from kzdev420 Jan 21, 2025
@bcgov bcgov deleted a comment from bcregistry-sre Jan 21, 2025
@bcgov bcgov deleted a comment from kzdev420 Jan 21, 2025
@bcgov bcgov deleted a comment from kzdev420 Jan 21, 2025
@bcgov bcgov deleted a comment from bcregistry-sre Jan 21, 2025
@bcgov bcgov deleted a comment from kzdev420 Jan 21, 2025
@kzdev420
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

bcregistry-sre commented Jan 21, 2025

@bcgov bcgov deleted a comment from bcregistry-sre Jan 21, 2025
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.

The ledger item looks great, but the link to the form still doesn't work.

image

https://business-dashboard-dev--pr-124-9kx34we4.web.app/BC0886466?filing_id=161929

@kzdev420
Copy link
Collaborator Author

https://business-dashboard-dev--pr-124-9kx34we4.web.app/BC0886466?filing_id=161929

The ledger item looks great, but the link to the form still doesn't work.

image

Hi @severinbeauvais
It works locally.
I just checked the code at the preview URL.
It shows like this. I think It doesn't get the URL from the environment variables.
Screenshot 2025-01-21 at 2 44 44 PM

@severinbeauvais
Copy link
Collaborator

Sample FE COA looks good:

image

https://business-dashboard-dev--pr-124-9kx34we4.web.app/BC0887059?filing_id=161931

@kzdev420
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-dashboard-dev--pr-124-9kx34we4.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.

Looks great!

The only thing is, can we deploy this to Prod, since the rest of the NoW work is not yet complete? Is it planned to be an Ops filing until we release everything? That's a question for Jacqueline or Mihai.

@severinbeauvais
Copy link
Collaborator

PS I don't see a Feature Flag to conditionally enable the drop-down menu option.

image

Am I missing something, or do we need another ticket for that?

@kzdev420
Copy link
Collaborator Author

PS I don't see a Feature Flag to conditionally enable the drop-down menu option.

image

Am I missing something, or do we need another ticket for that?

I didn't see that requirement on the ticket.
Did I missed it?

@jacqueline-williams-549

Looks great!

The only thing is, can we deploy this to Prod, since the rest of the NoW work is not yet complete? Is it planned to be an Ops filing until we release everything? That's a question for Jacqueline or Mihai.

I will ask Mihai to confirm, but the only difference between the old text and the next text is that we are directing them to the paper form that they must currently fill out and submit to BC Registries for any withdrawal. So I think that we shouldn't have a problem deploying it.

But once that paper form is updated, then it will ask for the filing number which I think only shows on outputs for business in the modern system.

@severinbeauvais
Copy link
Collaborator

PS I don't see a Feature Flag to conditionally enable the drop-down menu option.
I didn't see that requirement on the ticket. Did I missed it?

It's related to the feature as a whole, not specifically THIS ticket/PR. It would be in the ticket that added the drop-down menu option.

If you don't see a FF check in the code for that menu option then we need to create a new ticket. Why don't you create the ticket (if needed) and I'll update it with what I know?

@severinbeauvais
Copy link
Collaborator

The only thing is, can we deploy this to Prod, since the rest of the NoW work is not yet complete? Is it planned to be an Ops filing until we release everything? That's a question for Jacqueline or Mihai.

I will ask Mihai to confirm, but the only difference between the old text and the next text is that we are directing them to the paper form that they must currently fill out and submit to BC Registries for any withdrawal. So I think that we shouldn't have a problem deploying it.

But once that paper form is updated, then it will ask for the filing number which I think only shows on outputs for business in the modern system.

Hey there! Yeah, I think the ledger items are OK, but we might want to block the "File a Withdrawal" button with a FF until the whole feature is implemented and ready to release.

See my other comment above.

@jacqueline-williams-549

The only thing is, can we deploy this to Prod, since the rest of the NoW work is not yet complete? Is it planned to be an Ops filing until we release everything? That's a question for Jacqueline or Mihai.

I will ask Mihai to confirm, but the only difference between the old text and the next text is that we are directing them to the paper form that they must currently fill out and submit to BC Registries for any withdrawal. So I think that we shouldn't have a problem deploying it.
But once that paper form is updated, then it will ask for the filing number which I think only shows on outputs for business in the modern system.

Hey there! Yeah, I think the ledger items are OK, but we might want to block the "File a Withdrawal" button with a FF until the whole feature is implemented and ready to release.

See my other comment above.

I agree we wouldn't want to allow them to "file a withdrawal" until the whole feature is implemented and ready to release.

@severinbeauvais
Copy link
Collaborator

If you don't see a FF check in the code for that menu option then we need to create a new ticket. Why don't you create the ticket (if needed) and I'll update it with what I know?

I assume there is no FF so I created this ticket already: https://app.zenhub.com/workspaces/entities---olga-65af15f59e89f5043c2911f7/issues/gh/bcgov/entity/25406

@kzdev420 kzdev420 merged commit 4315b47 into bcgov:main Jan 22, 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.

None yet

5 participants