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

26014 - auth web permissions updates #3290

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

ochiu
Copy link
Collaborator

@ochiu ochiu commented Mar 6, 2025

Issue #:
bcgov/entity#26014

Description of changes:

  • staff / external UI permissions moved to DB
  • refactor staff dashboard loading to handle different staff types
  • fix existing refresh bug where account info is set to the current account and not what the url account is (was causing permissions issues with the current updates)
  • update account settings components to work with new permissions
  • Depends on PR 26014 - auth-api permissions updates for staff / external staff users #3287

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

@ochiu ochiu force-pushed the 26014-auth-web-permissions-updates branch from 282fb32 to 3c1a8f0 Compare March 6, 2025 16:11
@@ -131,7 +131,7 @@
class="value"
aria-labelledby="adminContact"
>
<OrgAdminContact />
<OrgAdminContact :orgId="orgId"/>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fix page refresh where the contact refreshes to the current accounts contact rather than the account on the current url

@@ -591,8 +590,6 @@ export default defineComponent({
}

onMounted(async () => {
const accountSettings = getAccountFromSession()
await orgStore.syncOrganization(accountSettings?.id)
Copy link
Collaborator Author

@ochiu ochiu Mar 6, 2025

Choose a reason for hiding this comment

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

Let the higher level sync from account settings(takes orgId path param now) take care of this or it conflicts with the orgId on the url on refresh.

sortable: false
}
]
export default defineComponent({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To composition-api which handled refresh / syncing issues

@@ -88,10 +87,6 @@ export default class PendingMemberDataTable extends Vue {
}
]

private canApproveOrDeny (): boolean {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

can be consistent now with some of the current invitation updates to use permissions.

@@ -188,13 +188,6 @@ export default defineComponent({
})

const credit = ref(0)

const isTransactionsAllowed = computed((): boolean => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no longer needed. Permissions will hide/show this based on 'transaction_history' permission, this has been updated for the auth-api portion. API blocks if there are no appropriate security roles.

v-for="tab in tabs"
:key="tab.code"
:data-test="tab.code"
:to="tab.page"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lot of weird artifacts with all the explicit v-tabs along with v-can permission updates. Refactored to use tab config that is built on permissions when loading up and iterates over to render them

this.currentOrganization?.statusCode === AccountStatus.INACTIVE
)
}

private async mounted () {
await this.syncOrganization(this.orgId)
await this.syncMembership(this.orgId)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Refresh bug fix, based on path param rather that currentOrg in the state

@@ -154,7 +154,8 @@

<!-- Continuation Applications -->
<v-card
v-can:VIEW_CONTINUATION_AUTHORIZATION_REVIEWS.hide
v-if="canViewContinuationAuthorization"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fix where legal api is still called even though this is hidden

@ochiu ochiu marked this pull request as ready for review March 6, 2025 21:43
Copy link
Collaborator

@seeker25 seeker25 left a comment

Choose a reason for hiding this comment

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

LGTM some solid stuff in here

@ochiu
Copy link
Collaborator Author

ochiu commented Mar 6, 2025

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-test-3290-2juml0kn.web.app

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-3290-a2e8xhf3.web.app

@ochiu
Copy link
Collaborator Author

ochiu commented Mar 6, 2025

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-test-3290-2juml0kn.web.app

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-3290-a2e8xhf3.web.app

@ochiu
Copy link
Collaborator Author

ochiu commented Mar 6, 2025

Holding off on merging. Discussed with Ken and he will do some initial regression on Staff permissions so he can use the same data set to compare against in DEV and gcbrun DEV

Copy link

sonarqubecloud bot commented Mar 7, 2025

@ochiu
Copy link
Collaborator Author

ochiu commented Mar 7, 2025

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-3290-a2e8xhf3.web.app

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-test-3290-2juml0kn.web.app

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants