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/create edit transformation fixes #9668

Merged

Conversation

Monkeychip
Copy link
Contributor

@Monkeychip Monkeychip commented Aug 5, 2020

This PR:

  • fixes the create transformation issue with the stringArray error
  • conditionally shows with the tweak_source or masking field based on the type selected.
  • serializes the masking data such that instead of displaying a character code, which is what is returned by the API, we display the character (e.g. we now show * and not 42).
  • Starts to layout the "show" display of a transformation. My next PR will finish this.
  • Fixes issue with template vs templates on the POST vs GET requests, respectfully. These will likely be changed again once the BE fixes the issue.

Here is a gif run through:

transformation

@Monkeychip Monkeychip added the ui label Aug 5, 2020
Copy link
Contributor

@chelshaw chelshaw left a 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)


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, ', ');
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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

@@ -73,6 +73,7 @@ label {
.sub-text {
color: $grey;
margin-bottom: 0.25rem;
font-size: 12px;
Copy link
Contributor

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

Copy link
Contributor Author

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=&lt;enter your value here&gt;</code>
<CopyButton class="button is-transparent level-right" @clipboardText='blahblah'
Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor Author

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=&lt;enter your value here&gt;</code>
<CopyButton class="button is-transparent level-right" @clipboardText='blahblah'
Copy link
Contributor

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)}}
Copy link
Contributor

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?

Copy link
Contributor Author

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
Copy link
Contributor

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

Copy link
Contributor Author

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.

@Monkeychip Monkeychip merged commit 5b3282e into ui/transform-sidebranch Aug 11, 2020
@Monkeychip Monkeychip deleted the ui/create-edit-transformation-fixes branch August 11, 2020 21:46
chelshaw added a commit that referenced this pull request Aug 26, 2020
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants