-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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: PKI config via import #18504
UI: PKI config via import #18504
Conversation
e8b7724
to
db3a6ac
Compare
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.
Nice job!! Some comments and questions, but shouldn't be anything blocking. 😄
@attr() importedIssuers; | ||
@attr() importedKeys; | ||
|
||
get backend() { |
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.
😍
@@ -12,6 +12,8 @@ export default class PkiOverviewRoute extends Route { | |||
} | |||
|
|||
hasConfig() { | |||
// When the engine is configured, it creates a default issuer. |
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.
thank you for the comment! 🤩
this.configModel = model; | ||
} | ||
|
||
willDestroy() { |
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 haven't seen much use of the lifecycle hooks in glimmer components, aside from the constructor, so I'm curious about the reasoning behind using willDestroy
here 💭
The form pattern I've seen us use lately is a cancel action that unloads the record and then calls something like this.args.onCancel
which is usually the transition, if there is one
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.
unloading with onCancel
is fine for when the user explicitly leaves the page, but in a case where, for example, the user clicks "back" or on another link, we also want the model to reset which is where the willDestroy
hook comes in handy.
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.
Oh, one other reason -- since this form is being rendered inline on the page, we want the import config model to reset if the user clicks on a different option, so there aren't multiple config models floating around
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.
Ohhh, yes! That makes sense!
bdc3acc
to
9964c4d
Compare
Allows user to configure the pki engine via the /pki/issuers/import/bundle endpoint, with
pem_bundle
on the payload.(Issuer details view is on a different PR which is not merged at the time of the screenshot)