feat(front): Implement front-end for replacetext step TCTC-4368 #1556
Conversation
TCTC-4368 Signed-off-by: Luka Peschke <luka.peschke@toucantoco.com>
Signed-off-by: Luka Peschke <luka.peschke@toucantoco.com>
|
Your Render PR Server URL is https://weaverbird-playground-pr-1556.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cdclg3sgqg4811u7a440. |
| from weaverbird.pipeline.steps.utils.render_variables import StepWithVariablesMixin | ||
| from weaverbird.pipeline.types import ColumnName, TemplatedVariable | ||
|
|
||
|
|
||
| class ReplaceTextStep(BaseStep): | ||
| class Config: | ||
| alias_generator = to_camelcase |
There was a problem hiding this comment.
isn't it something we should have by default on all base steps ?
There was a problem hiding this comment.
I agree, but It'll probably be a breaking change. I'm preparing another PR for that, to be merged after the LTS branches have been created
| oldStr: { | ||
| type: 'string', | ||
| description: 'String to replace', | ||
| minLength: 1, |
There was a problem hiding this comment.
Why min length? If I want to replace empty strings with something else
There was a problem hiding this comment.
Then you'd use the normal already existing replace step, which replaces values
There was a problem hiding this comment.
I don't know the codebase as well as you but from a user perspective it makes no sense to have two different steps if I want to replace '' or 'a'
There was a problem hiding this comment.
At first, we were wondering if replacing text and values should both be done in the same step or not. After discussion with product, we've decided to to it in two separate steps.
The difference between the two steps is:
- I want to replace all values in column
colcontaining the literal value"a"with42-> that's areplace, the type can change. Only cells with"a"in them will be modified -> Cells with "baba" are left untouched. - In my string columns, I want to replace the string "guys" with "people" -> That's a
replacetext. All cells containing "guys" will be modified:"guys"will become"people","hello guys"will become"hello people".
In your example the expeected behaviour of a replacetext("", "a") would be "foo" -> "afaoaoa", which is probably not what the end user wants
There was a problem hiding this comment.
Too bad it wasn't called "replace substring"
I didn't realize what this would do until I read your last example 😅
There was a problem hiding this comment.
Yes, naming is a bit frustrating for devs, but with product we thought that it was better to have "text" everywhere rather than let end users figure out by themselves wtf a "substring" is supposed to be 😄
Signed-off-by: Luka Peschke <luka.peschke@toucantoco.com>
Codecov ReportBase: 98.43% // Head: 98.44% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #1556 +/- ##
=======================================
Coverage 98.43% 98.44%
=======================================
Files 216 218 +2
Lines 6138 6167 +29
Branches 961 962 +1
=======================================
+ Hits 6042 6071 +29
Misses 96 96
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
TCTC-4368