-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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/create edit transformation fixes #9668
Ui/create edit transformation fixes #9668
Conversation
… text for create transformation
…TODO: turn into components.
…ormation-itme list accordingly
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.
Really great work so far! A few questions, happy to chat about it (especially the serializer thing, which I'm sure I'm missing some context on)
ui/app/serializers/transform.js
Outdated
|
||
if (payload.data.templates && payload.data.templates.length === 1) { | ||
// add space after comma in returned array length one of a string of templates | ||
payload.data.templates[0] = payload.data.templates[0].replace(/,/g, ', '); |
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.
If we're adding a comma for display purposes I would expect the controller to add that in, not the serializer. I'm also a little confused why we're adding it only to the first template? Maybe an example of the before and after here would be helpful.
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.
Let's discuss during our sync. This was a request by design, but becomes a problem with Roles because the data is structured differently. And yeah, I originally thought of using a serializer because it was changing the data, but it's more of a display thing. It should go in a controller.
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.
Removing for now based on discussion
ui/app/styles/core/forms.scss
Outdated
@@ -73,6 +73,7 @@ label { | |||
.sub-text { | |||
color: $grey; | |||
margin-bottom: 0.25rem; | |||
font-size: 12px; |
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.
Here too. If the designs don't match the sizing variables we have we need to revisit with the design team
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.
See comment above.
</div> | ||
<div class="copy-text level"> | ||
<code>vault write transform/encode/payments value=<enter your value here></code> | ||
<CopyButton class="button is-transparent level-right" @clipboardText='blahblah' |
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.
The @clipboardText
should probably be the command above, which makes me think this is a perfect use case for the handlebars {{let}} helper
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.
You may have to walk me through that one. Let's touch base in the sync later today
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.
That's sweet!!
</div> | ||
<div class="copy-text level"> | ||
<code>vault write transform/decode/payments value=<enter your value here></code> | ||
<CopyButton class="button is-transparent level-right" @clipboardText='blahblah' |
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.
Here too! :D
> | ||
Delete role | ||
Delete transformation | ||
</ConfirmAction> | ||
{{/if}} | ||
{{#if (or model.canUpdate model.canDelete)}} |
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.
Someone with only delete capabilities can edit the transformation, is that right?
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.
Ah, good catch. I was using a pattern from the template earlier, but they to have canUpdate in order to edit, not canDelete. I'll amend.
@@ -0,0 +1,62 @@ | |||
{{#linked-block | |||
(concat |
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.
Probably don't need concat if it's all magic strings
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.
Good catch, another carry over from copying from a different template.
* Ui/transform enable (#9647) * Show Transform on engines list if enterprise * Add box-radio component * Add is-disabled styling for box-radio and fix tooltip styling when position: above * Add KMIP and Transform to possible features on has feature helper * Sidebranch: Transform Secret Engine Initial setup (#9625) * WIP // list transforms, console.logs and all * setup LIST transformations ajax request and draft out options-for-backend options * change from plural to singluar and add transform to secret-edit * create two transform edit components * modify transform model with new attrs * add adapterFor to connect transform adapter to transform-edit-form component * setup Allowed roles searchSelect component to search over new transform/role adapter and model. * clean up for PR * clean up linting errors * restructure adapter call, now it works. * remove console * setup template model for SearchSelect component * add props to form field and search select for styling Co-authored-by: Chelsea Shaw <[email protected]> * Ui/transform language fixes (#9666) * Update casing and wording on Transform list route. Use generic list item for transformations * Add back js file for transformation-edit * Set up transform for tabs * Ui/create edit transformation fixes (#9668) * add conditional for masking vs tweak source based on type, and update text for create transformation * change order * fix error with stringArray * setup the edit/delete transformation view * clean up toolbar links * setup serializer to change response of mask character from keycode to character * change styling of label and sub-text size, confirmed with design * temp fix on templates vs template * add clickable list item * add space between template list * setup styling and structure for the rest of the show transformation. TODO: turn into components. * create transform-show-transformation component * add attachCapabilities to transform model and update transform-transformation-itme list accordingly * clean up liniting errors * address pr comments * remove leftover * clean up * Sidebranch: UI transform create and edit clean up (#9778) * clean up some of the TODOs * setup edit view with read only attributes for name and template * setup initial selected for search select component * fixes * hide templates form field for now * set selectLimit for search select component * hide power select if the select limit is greater than or equal to the selectedOptions length * clean up failing linting * address pr comments * Ui/fix list roles transformation (#9788) * Update search-select to pass backend to query if exists * Update role and template adapters * cleanup * Fix replace with static string * Ui/transform cleanup 2 (#9789) * amend encode/decode commands for now until design gets back with more details * restrict character count on masking input field * clean up selectLimit * show backend instead of transform in cli copy command * Show KMIP un-selectable if enterprise but no ADP module (#9780) * New component transform-edit-base * Duplicate RoleEdit as TransformEditBase and swap in all transform components * Roll back role-edit changes * Update to transform edit base * Remove extraeneous set backend type on transform components * formatting * Revert search-select changes * Update template/templates data on transformation (#9838) Co-authored-by: Angel Garbarino <[email protected]>
This PR:
stringArray
errortweak_source
ormasking field
based on the type selected.Here is a gif run through: