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

ui: add capabilities to pki key model #18412

Merged
merged 14 commits into from
Dec 16, 2022

Conversation

hellobontempo
Copy link
Contributor

@hellobontempo hellobontempo commented Dec 15, 2022

Adds capabilities to the key model and wraps actions in the key route

@hellobontempo hellobontempo added this to the 1.13.0-rc1 milestone Dec 15, 2022
@hellobontempo hellobontempo changed the title add capabilities to pki key model ui: add capabilities to pki key model Dec 15, 2022
@hellobontempo hellobontempo marked this pull request as ready for review December 15, 2022 20:56
Copy link
Collaborator

@hashishaw hashishaw left a comment

Choose a reason for hiding this comment

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

✨ gorgeous! A couple minor comments

<ToolbarLink @route="keys.key.edit" @model={{@key.model.name}}>
Edit key
</ToolbarLink>
{{#if @canDelete}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Super excited about this pattern 🤩

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same! great idea 🥇

<nav class="menu">
<ul class="menu-list">
<li>
<LinkTo @route="keys.key.details" @model={{pkiKey.keyId}} @disabled={{eq pkiKey.canRead false}}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since the boolean is calculated on the model you could use not instead of eq ... false. Also do we want to used a passed arg here?

Suggested change
<LinkTo @route="keys.key.details" @model={{pkiKey.keyId}} @disabled={{eq pkiKey.canRead false}}>
<LinkTo @route="keys.key.details" @model={{pkiKey.keyId}} @disabled={{not @canRead}}>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah - I was wondering about doing something similar to what I did for @canGenerateKey so like the following. If we are okay with reading those capabilities just based on the first model?

@canReadKey={{this.model.keyModels.firstObject.canReadKey}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought we didn't want to use not in case the capability is undefined for some reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oohhh ^ disregard, forgot its explicitly checking for false in the model getter now

Copy link
Collaborator

@hashishaw hashishaw left a comment

Choose a reason for hiding this comment

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

🎉

@hellobontempo hellobontempo enabled auto-merge (squash) December 16, 2022 17:10
@vercel
Copy link

vercel bot commented Dec 16, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
vault 🔄 Building (Inspect) Dec 16, 2022 at 6:09PM (UTC)

@hellobontempo hellobontempo merged commit 6413a7a into main Dec 16, 2022
@hellobontempo hellobontempo deleted the ui/VAULT-12125/add-capabilities-pki-key branch December 16, 2022 23:00
AnPucel pushed a commit that referenced this pull request Jan 14, 2023
* add capabilities to pki key model

* move key list from route into component

* rename test file

* rename test file

* add tests

* pass capabilities directly to key list componente

* add test for key list component

* rename test files

* remove href assertion
dhuckins pushed a commit that referenced this pull request Jan 19, 2023
* add capabilities to pki key model

* move key list from route into component

* rename test file

* rename test file

* add tests

* pass capabilities directly to key list componente

* add test for key list component

* rename test files

* remove href assertion
dhuckins pushed a commit that referenced this pull request Jan 19, 2023
* add capabilities to pki key model

* move key list from route into component

* rename test file

* rename test file

* add tests

* pass capabilities directly to key list componente

* add test for key list component

* rename test files

* remove href assertion
AnPucel pushed a commit that referenced this pull request Jan 25, 2023
* add capabilities to pki key model

* move key list from route into component

* rename test file

* rename test file

* add tests

* pass capabilities directly to key list componente

* add test for key list component

* rename test files

* remove href assertion
AnPucel pushed a commit that referenced this pull request Feb 3, 2023
* add capabilities to pki key model

* move key list from route into component

* rename test file

* rename test file

* add tests

* pass capabilities directly to key list componente

* add test for key list component

* rename test files

* remove href assertion
jayant07-yb pushed a commit to jayant07-yb/hashicorp-vault-integrations that referenced this pull request Mar 15, 2023
* add capabilities to pki key model

* move key list from route into component

* rename test file

* rename test file

* add tests

* pass capabilities directly to key list componente

* add test for key list component

* rename test files

* remove href assertion
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.

None yet

2 participants