-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Feature: Rework vaultDetails logic #271
Conversation
* add upgradeToOwner button * group in different divs
and related fields/properties
Only block addMember/grantAccess, otherwise allow all features
... and allow removing/editing vault role
WalkthroughThe update primarily enhances the management and interaction with vaults in a software application. It revises roles, introduces license status checks, and improves error messaging, ensuring a more intuitive and compliant user experience. Changes
Possibly related issues
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (4)
Files skipped from review as they are similar to previous changes (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Co-authored-by: Sebastian Stenzel <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just did a code review for now. Is my assumption right that you didn't really change anything in the UI? Or is there a noteworthy change?
Co-authored-by: Tobias Hagemann <[email protected]>
Nothing apparently visible. As wrote in the opening comment, i cleaned the license logic, such that only grant access and addMember are blocked. All other features are enabled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't test, so take this review with a grain of salt. Code change lgtm though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this change but I still need to run some tests...
Only I do not really understand the reason behind the change from RecoveryKeyDialog
to DisplayRecoveryKeyDialog
? For me a dialog is already something to display? Things get wild at the latest with showDisplayRecoveryKeyDialog
😅
If you are looking at the other I just fitted it into the scheme and from my perspective, it is now clearer and additionally easier to distinc from the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Out of diff range and nitpick comments (1)
frontend/src/components/VaultDetails.vue (1)
63-68
: Consider adding a confirmation dialog before changing a member's role to 'OWNER'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In
hub/frontend/src/components/VaultDetails.vue
Line 290 in 3a7a132
const vaultKeyJwe = await backend.vaults.accessToken(props.vaultId, true); |
when the license has expired, the backend returns a 402 for /vaults/${vaultId}/access-token?evenIfArchived=true
and is mapped to a PaymentRequiredError
which is caught in this method, but results in no vaultKeys
being set, and in turn the Show Recovery Key
button cannot show the DisplayRecoveryKeyDialog
but does nothing due to
hub/frontend/src/components/VaultDetails.vue
Line 191 in 3a7a132
<DisplayRecoveryKeyDialog v-if="displayingRecoveryKey && vault && vaultKeys" ref="displayRecoveryKeyDialog" :vault="vault" :vault-keys="vaultKeys" @close="displayingRecoveryKey = false" /> |
We should not show this button if vaultKeys
is missing.
This PR reworks/fixes the logic in vault details.
Changes:
NONE
). An admin can access all vaults, but does not have theMEMBER
role. This prevents errors done in future changes and keeps the model clean.Summary by CodeRabbit
New Features
Enhancements
VaultDetails
component for better user experience based on user roles and license status.VaultList
component.Bug Fixes
Refactor
Documentation