Support edit transformation name #27
Conversation
annie3431
requested review from
thalurur,
adityaj1107,
bowenlan-amzn and
lobdelle
| @@ -0,0 +1,78 @@ | |||
| /* | |||
There was a problem hiding this comment.
Can you use the new license header?
There was a problem hiding this comment.
Sure, will refactor that.
| @@ -74,3 +74,7 @@ export const wrapQuotesAroundTransformId = (transformId: string, stringToSearch: | |||
| const idWrappedWithQuotes = `"${transformId}"`; | |||
| return stringToSearch.replace(regex, idWrappedWithQuotes); | |||
| }; | |||
|
|
|||
| export const isGroupBy = (type: string) => { | |||
| @@ -9,7 +9,7 @@ | |||
| * GitHub history for details. | |||
| */ | |||
|
|
|||
| import { FieldItem } from "../../../../models/interfaces"; | |||
| import { FieldItem, TRANSFORM_AGG_TYPE } from "../../../../models/interfaces"; | |||
|
|
|||
| export const parseTimeunit = (timeunit: string): string => { | |||
There was a problem hiding this comment.
Should we refactor these utility methods (parseTimeUnit and isNumericMapping etc.) present in CreateRollups as well to a common utility file? We can then use them across both modules and reduce code duplication. WDYT?
There was a problem hiding this comment.
I agree. I think we can do it after the transforms jest unit tests are implemented to avoid errors.
| closePopover: () => void; | ||
| } | ||
|
|
||
| export default function EditTransformPanel({ name, aggList, onEditTransformation, closePopover }: EditTransformPanelProps) { |
There was a problem hiding this comment.
Should we add jest tests for these?
There was a problem hiding this comment.
Yes, I believe the unit tests are currently being written.
| onClick={() => { | ||
| // Update transform name if new value specified | ||
| if (name !== transformName) onEditTransformation(name, transformName); | ||
| closePopover(); |
There was a problem hiding this comment.
Should we move this to a separate function? Same question for the onChange variable in the EuiFieldText.
There was a problem hiding this comment.
I think either way should be fine. I guess the benefit of separating as a function would be good for readability. I believe there are many more places that can do the same. Will take a look. Thanks for the suggestion!
There was a problem hiding this comment.
Yeah if there are other places, we should do that cleanup. We should take it up as a separate PR. :) :)
Issue #, if available: N/A
Description of changes:
isGroupByto determine the type of transformation.Screenshots:
On step 2 of create transform:
When clicking the edit transform icon button:
After selecting "Edit transformation":
Validation of duplicate name:
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.