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

Fix for non staff user view/change business info modal #84

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

BrandonSharratt
Copy link
Collaborator

*Issue:*bcgov/entity#24459

Description of changes:
Fix the text that shows for non staff user.

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
Copy link
Collaborator

Could you please trigger the preview site build and/or provide before and after screenshots to show what this change did? Thanks.

@BrandonSharratt
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

bcregistry-sre commented Nov 27, 2024

@severinbeauvais
Copy link
Collaborator

My test results...

Alteration

As a regular user

image

As staff

I am redirected to Edit UI to perform the alteration. This is consistent with Filings UI.

Voluntary Dissolution

As a regular user

This is not consistent with Filings UI, which shows you the action in the More Actions dropdown, and then a dialog when you select it.

image

As staff

image

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.

If you don't have a UI design to be different from Filings UI then please update this.

Also, please use a more verbose PR title :)

@BrandonSharratt BrandonSharratt changed the title tweak Fix for non staff user view/change business info modal Nov 27, 2024
@BrandonSharratt
Copy link
Collaborator Author

BrandonSharratt commented Nov 27, 2024

@severinbeauvais

This PR is not addressing visibility or enabled state of actions.

However, in the case demonstrated it should be disabled. The business is not in good standing, so it does not show in the allowable actions for this business which is considered a blocker in the legal api.

@@ -63,7 +63,7 @@ const setShowDissolutionDialog = (show: boolean) => {
}

const dialogTitle = computed<string>(() => {
return (!currentBusiness?.value?.goodStanding && hasRoleStaff)
return showChangeNotInGoodStandingDialog.value
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand how a variable for whether to show a dialog can be used to select one other other string (or template, below).

I cannot verify this as I cannot get to the dissolution version of this dialog.

@severinbeauvais
Copy link
Collaborator

However, in the case demonstrated it should be disabled. The business is not in good standing, so it does not show in the allowable actions for this business which is considered a blocker in the legal api.

So... the dissolution version of this dialog will NEVER show?

@BrandonSharratt
Copy link
Collaborator Author

However, in the case demonstrated it should be disabled. The business is not in good standing, so it does not show in the allowable actions for this business which is considered a blocker in the legal api.

So... the dissolution version of this dialog will NEVER show?

No it'll show you just need a business in good standing

@hfekete
Copy link
Collaborator

hfekete commented Nov 28, 2024

Here is example of Voluntary dissolution dialog from the PR.

image

Lets close this PR if it fixes what the ticket asks for, and lets create new verify or bug ticket if we are having some issues in a flow.

@hfekete
Copy link
Collaborator

hfekete commented Nov 28, 2024

ticket to verify dissolve flow for not in good standing business:
https://app.zenhub.com/workspaces/beneficial-ownership-2023-6557a7db7ce6e464af821855/issues/gh/bcgov/entity/24637

@severinbeauvais
Copy link
Collaborator

However, in the case demonstrated it should be disabled. The business is not in good standing, so it does not show in the allowable actions for this business which is considered a blocker in the legal api.

So... the dissolution version of this dialog will NEVER show?

No it'll show you just need a business in good standing

Is there still a "Business is not in good standing -- voluntary dissolution" dialog? How do I get to it?

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.

Lets close this PR if it fixes what the ticket asks for, and lets create new verify or bug ticket if we are having some issues in a flow.

Go for it.

@BrandonSharratt BrandonSharratt merged commit 975e0ed into bcgov:main Nov 28, 2024
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.

4 participants