Pki Generate Intermediate CSR #18807
Conversation
| @@ -23,6 +24,8 @@ const validations = { | |||
| export default class PkiActionModel extends Model { | |||
| @service secretMountPath; | |||
|
|
|||
| @tracked actionType; // used to toggle between different form fields when creating configuration | |||
There was a problem hiding this comment.
Needed to move this to the model for conditional validation since issuerName is not a required field when generating a CSR.
| const { isValid, state, invalidFormMessage } = model.validate(); | ||
| if (isValid) { | ||
| yield model.save({ | ||
| adapterOptions: { actionType: 'generate-csr', useIssuer: model.canGenerateIssuerIntermediate }, |
There was a problem hiding this comment.
Since the model uses lazyCapabilities I found that if we only call it during the save action it doesn't always get the response back in time to be useful to this method. So I recommend calculating it as you pass in useIssuer and then replace this with that arg. That way the parent can easily override it as well.
| adapterOptions: { actionType: 'generate-csr', useIssuer: model.canGenerateIssuerIntermediate }, | |
| adapterOptions: { actionType: 'generate-csr', useIssuer: this.args.useIssuer }, |
There was a problem hiding this comment.
I'm glad you pointed that out! I was working on tests for the component and noticed that it wasn't picking up the capabilities but it was when I went through the same steps in the app.
I thought we wanted to get away from evaluating form validation on change for UX purposes since the errors show up immediately and can be distracting. I can make the change but since generateIssuerCsrPath is a promise proxy, what if I were to await that before calling save?
const issuerCapabilities = yield model.generateIssuerCsrPath;
yield model.save({
adapterOptions: { actionType: 'generate-csr', useIssuer: issuerCapabilities.get('canCreate') }
});With the above change the test is now passing!
There was a problem hiding this comment.
Interesting. I don't love having to do all the manual jumps to get it to Just Work, but it's good to know that yielding the path manually does work! One other thing to note is that if we go with your proposed code, that does differ from the patterns we have already established in the other PKI forms. If we go with what you propose I would ask that we remove useIssuer from the component args and maybe document this usage in the lazyCapabilities definition?
There was a problem hiding this comment.
Sorry I enabled auto merge since I stepped out to walk the dog. I reverted those changes and my test is failing again. I updated the pki-generate-csr to pass in adapterOptions instead and the capabilities are resolving after the save request. The same is true if I pass just the property as useIssuer as suggested. Even though canGenerateIssuerIntermediate is passed in as an arg the computed is not triggered until the property is accessed. I suspect a similar test would fail in the generate-root component.
There was a problem hiding this comment.
The useIssuer arg in the interface should have been removed since it was from an early iteration of the component so I will get that updated. I can add a test for saving the config from the generate-root component and see if it fails and then align the capabilities access between the 2 components. I can also add some comments somewhere to note that we should be waiting for the promises if not accessed from a template.
| @task | ||
| @waitFor | ||
| *save(event: Event) { | ||
| *save(event: Event): Generator<Promise<boolean | PkiActionModel>> { |
There was a problem hiding this comment.
Interesting, is the task "returning" whatever is yielded, that's why you need both the boolean and the PkiActionModel in the promise value?
There was a problem hiding this comment.
Ya it's the IterableIterator returned from yield which in this case is 2 different things. I had to look it up 😁.
* adds pki generate csr component * adds keyParamsByType helper to pki-generate-toggle-groups component * removes unused router service from pki-generate-csr component * updates common pki generate form fields * addresses feedback and adds tests
Adds component for generating intermediate CSR in config and issuer views.
Tests to come in follow up PR to keep things moving!Update: tests were ready by the time this was initially reviewed so I added them!