feat: con 592 update job batches v2 endpoint #109
Conversation
alubenskyi
requested review from
AlexArcher,
PavelLoparev,
dimitrystd and
rischenko
| + "\"authorize\":true," | ||
| + "\"fileUris\":[\"fileUri.json\"]," | ||
| + "\"localeWorkflows\":[{\"targetLocaleId\":\"fr-FR\",\"workflowUid\":\"workflowUid\"}]" | ||
| + "\"localeWorkflows\":[{\"targetLocaleId\":\"fr-FR\",\"workflowUid\":\"workflowUid\",\"contentAssignments\":null}]" |
There was a problem hiding this comment.
What happens if [{\"targetLocaleId\":\"fr-FR\",\"workflowUid\":\"workflowUid\"}] is passed (without \"contentAssignments\":null)? It must be possible to pass without it for backward compatibility.
There was a problem hiding this comment.
"contentAssignments" field is not required and nullable.
So I think it will be ok to use it without contentAssignments as now.
Tests were added in another PR to cover this case: https://github.com/Smartling/jobs-facade/pull/135
There was a problem hiding this comment.
agree with pavel, I'd remain this test data as is, and add another with contentAssignements field.
There was a problem hiding this comment.
Tests were added in another PR for job-facade service to cover this case: https://github.com/Smartling/jobs-facade/pull/135
There was a problem hiding this comment.
Here I setuped input pto without contentAssignments array, as is on prod for now.
requestString contains contentAssignments = null because request.getBody().readUtf8() prints all fields from pto's with default values if such field explicitly is not defined.
PavelLoparev
left a comment
PavelLoparev
left a comment
There was a problem hiding this comment.
I'm ok but let's wait other's review
dimitrystd
left a comment
dimitrystd
left a comment
There was a problem hiding this comment.
Looks good. But waiting for one more proposed test case.
alubenskyi
changed the title
Replace this text with a description of your proposed change.
JIRA Ticket