feat: Draft PR Support Edit Values for JSON operator #1132 #761
Conversation
Prototype4988
requested review from
donch1989,
jvallesm and
chuang8511
as code owners
Prototype4988
changed the title
There was a problem hiding this comment.
Thanks for your contribution!
Before next code review, could you do a few things for us?
- We will assume you have done end-to-end test when your PR is ready. So, please provide your recipe for us.
- If you have time, please also add the unit testing code to ensure the functionality.
Thank you again!
| @@ -198,5 +198,47 @@ | |||
| "title": "Output", | |||
| "type": "object" | |||
| } | |||
| }, | |||
| "TASK_EDIT_VALUES": { | |||
There was a problem hiding this comment.
Sorry, I found I put the wrong JSON schema in the issue.
Could you fetch this JSON schema?
So, it means your Golang reader will change the key name.
e.g. conflictResolution -> conflict-resolution
| @@ -155,3 +162,167 @@ func (e *execution) jq(in *structpb.Struct) (*structpb.Struct, error) { | |||
| func (e *execution) Execute(ctx context.Context, jobs []*base.Job) error { | |||
| return base.SequentialExecutor(ctx, jobs, e.execute) | |||
| } | |||
|
|
|||
| func (e *execution) updateJson(in *structpb.Struct) (*structpb.Struct, error) { | |||
There was a problem hiding this comment.
updateJson
There is another function called TASK_RENAME_FIELDS in the near future.
So, let's make this function name specific. How about just editValue?
| data := in.Fields["data"].AsInterface() | ||
| updates := in.Fields["updates"].GetListValue().AsSlice() | ||
| conflictResolution := in.Fields["conflictResolution"].GetStringValue() | ||
| supportDotNotation := in.Fields["supportDotNotation"].GetBoolValue() |
There was a problem hiding this comment.
In other tasks in JSON, there are only 1 or 2 fields, so they don't use Struct to convert data from schema.
How about we make it as struct? It can convert data easily.
You can take this as sample.
data := in.Fields["data"].AsInterface()
updates := in.Fields["updates"].GetListValue().AsSlice()
conflictResolution := in.Fields["conflictResolution"].GetStringValue()
supportDotNotation := in.Fields["supportDotNotation"].GetBoolValue()
| // Process each object in the array | ||
| for i, obj := range data { | ||
| if err := applyUpdatesToObject(obj, updates, supportDotNotation, conflictResolution); err != nil { | ||
| msg := fmt.Sprintf("Error in object %d: %v\n", i, err) |
There was a problem hiding this comment.
| msg := fmt.Sprintf("Error in object %d: %v\n", i, err) | |
| msg := fmt.Sprintf("apply updates to object %d: %v\n", i, err) |
Now, we follow the pattern of describing what it does but not error statement when error handling.
| case map[string]interface{}: | ||
| // Process the single object | ||
| if err := applyUpdatesToObject(data, updates, supportDotNotation, conflictResolution); err != nil { | ||
| msg := fmt.Sprintf("Error in single object: %v\n", err) |
There was a problem hiding this comment.
| msg := fmt.Sprintf("Error in single object: %v\n", err) | |
| msg := fmt.Sprintf("apply update to single object: %v\n", err) |
| conflictResolution := in.Fields["conflictResolution"].GetStringValue() | ||
| supportDotNotation := in.Fields["supportDotNotation"].GetBoolValue() | ||
|
|
||
| // Perform deep copy of the data before updates |
There was a problem hiding this comment.
I think good comments are for
- Exported document
- Reason why the code is like this
- If it is hard to understand, add what the code does
The code explains itself well, so I'd remove the comments here.
Same as other places.
Because
This commit closes #1132