feat: Strings API v2 #38
Conversation
AlexanderNovak
requested review from
AlexArcher,
bloy-smartling and
vlinetskiysml
as code owners
pom.xml
Outdated
| @@ -30,6 +31,17 @@ | |||
| <org.jboss.resteasy.version>3.11.0.Final</org.jboss.resteasy.version> | |||
| </properties> | |||
|
|
|||
| <!-- [+] TODO: ALX Revert this --> | |||
|
|
||
| import java.util.List; | ||
|
|
||
| public class StringsCreationPTO |
There was a problem hiding this comment.
how do I do create+authorize?
you need to add into this class
private StringsAuthorizationPTO authorization;
There was a problem hiding this comment.
from public doc.
Note: You cannot authorize a string via API. Once you have created the strings, they will need to be authorized by a content owner in the Smartling Dashboard.
so did this by intend
pom.xml
Outdated
| <!-- [+] TODO: ALX Revert this --> | ||
| <repositories> | ||
| <repository> | ||
| <id>resolve-parent</id> | ||
| <url>https://artifactory.smartling.net/artifactory/repo</url> | ||
| <releases><enabled>true</enabled></releases> | ||
| <snapshots><enabled>true</enabled></snapshots> | ||
| </repository> | ||
| </repositories> | ||
| <!-- [-] TODO: ALX Revert this --> | ||
|
|
There was a problem hiding this comment.
You have to revert this, right?
There was a problem hiding this comment.
yep, will do. was fighting with dependencies but it turned into java 11 problem
| package com.smartling.api.strings.v2.pto; | ||
|
|
||
|
|
||
| public class CreatedStringPTO |
There was a problem hiding this comment.
You can use Project Lombok for your DTOs.
|
|
||
| @GET | ||
| @Path("/projects/{projectUid}/processes/{processUid}") | ||
| AsyncStatusResponsePTO asyncProcess(@PathParam("projectUid") String projectUid, @PathParam("processUid") String processUid); |
There was a problem hiding this comment.
In our reference this method is described as "Check string status" which is not consistent with "asyncProcess" method name
https://api-reference.smartling.com/#operation/getAddStringsToProjectRequestStatus
There was a problem hiding this comment.
yeah.... going to rename this to checkStatus, any other suggestions about naming?
pom.xml
Outdated
| <snapshots><enabled>true</enabled></snapshots> | ||
| </repository> | ||
| </repositories> | ||
| <!-- [-] TODO: ALX Revert this --> |
There was a problem hiding this comment.
This is a public open-source project, it mustn't depend on any closed source library
| private String modifiedDate; | ||
| private ProcessStatisticsPTO processStatistics; | ||
|
|
||
| public String getProcessUid() |
There was a problem hiding this comment.
we use Lombok in all other SDKs
I think it is better to do so across the whole project
There was a problem hiding this comment.
will do lombok style then :)
|
|
||
| import static org.junit.Assert.*; | ||
|
|
||
| public class StringsApiTest |
There was a problem hiding this comment.
You check only method and path from the request but you never check that the payload is serialized as expected
You also don't check how the response is deserialized
This API implementation is copypasted from the legacy project which is working and well tested
tests will be very helpful when we will switch major versions of Resteasy or Jackson
There was a problem hiding this comment.
well... it actually tests json marshalling (from public doc examples) and it will fail if for instance boolean property in DTO turns into Integer. In this case I just do not check that DTO contains exact payload from json that I provide. In this case examining DTO it turns into test for test
TCM-2046 Strings API collection not included in Java SDK