Abuse reports getters #6
Conversation
|
Hi @apocheau , It seems that you have not applied the new gradle version by calling './gradlew wrapper' since gradle/wrapper/gradle-wrapper.properties still refers to the old version. Haven't had a look at the other changes, will do it later. |
| @@ -1,4 +1,4 @@ | |||
| #Thu Aug 04 16:09:12 SAMT 2016 | |||
| #Tue Oct 04 22:38:57 CEST 2016 | |||
| distributionBase=GRADLE_USER_HOME | |||
| distributionPath=wrapper/dists | |||
| zipStoreBase=GRADLE_USER_HOME | |||
There was a problem hiding this comment.
Notice that the line below still refers to the old gradle version. You should run ./gradlew wrapper to complete Gradle upgrade
|
|
||
| @JvmField | ||
| @Field | ||
| var _links: List<LinkInfo>? = null |
There was a problem hiding this comment.
These "links" are included with every response and I find them useless. Do they really needed? If not, I'd prefer this field be removed.
There was a problem hiding this comment.
The main goal of the connector is to give access to the data offered by the API.
As we are working on an opensource connector to MailChimp API, we shouldn't filter data.
Everyone using this connector would be able to choose whether to use this field or not.
| @Field | ||
| var _links: List<LinkInfo>? = null | ||
|
|
||
| fun setId(id: String): AbuseReportInfo{ |
There was a problem hiding this comment.
It seems that you've added a kind of "builder" methods. This is not a Kotlin-way to build objects. In Kotlin you build objects as follows:
val abuseReport = AbuseReportInfo().apply {
id = "1234"
...
}
Please remove these methods.
There was a problem hiding this comment.
Yes it was made for chaining methods and build object in test method more easily. For example:
AbuseReportInfo().setId("2345").setCampaign_id("6789");
The "Kotlin way" is also nice and i didn't know it, thanks. I will follow it.
|
|
||
| @JvmField | ||
| @Field | ||
| var date: String? = null |
|
|
||
| @JvmField | ||
| @Field | ||
| var merge_fields: Object? = null |
There was a problem hiding this comment.
This field should be of type MailchimpObject (so that the info could be taken from mapping).
| // var spiedConnector = spy(connector) | ||
| // | ||
| // var callMock = mock<HttpClientConnector>() | ||
| // `when`(spiedConnector.call()).thenReturn(response) |
There was a problem hiding this comment.
The response could be mocked this way:
class MockMailchimpClient : MailchimpClient(apiKey, object : Connector {
override fun call(request: Connector.Request): Connector.Response {
val json = "{\"id\": 42,\"campaign_id\": \"839488a60b\",\"list_id\": \"1a2df69511\",\"email_id\": \"62eeb292278cc15f5817cb78f7790b08\",\"email_address\": \"[email protected]\",\"date\": \"2015-07-15T19:19:31+00:00\",\"links\": [{\"rel\": \"self\",\"href\": \"https://usX.api.mailchimp.com/3.0/lists/1a2df69511/abuse-reports/42\",\"method\": \"GET\",\"targetSchema\": \"https://api.mailchimp.com/schema/3.0/Lists/Abuse/Instance.json\" },{ \"rel\": \"parent\",\"href\": \"https://usX.api.mailchimp.com/3.0/lists/1a2df69511/abuse-reports\",\"method\": \"GET\",\"targetSchema\": \"https://api.mailchimp.com/schema/3.0/Lists/Abuse/Collection.json\",\"schema\": \"https://api.mailchimp.com/schema/3.0/CollectionLinks/Lists/Abuse.json\" }]}"
return Connector.Response(200, "", json)
}
override fun close() { }
})
| @@ -27,4 +27,5 @@ dependencies { | |||
| compile 'joda-time:joda-time:2.9.4' | |||
|
|
|||
| testCompile 'org.testng:testng:6.8.21' | |||
| testCompile 'org.powermock:powermock-mockito-release-full:1.5.4' | |||
There was a problem hiding this comment.
There is a way to mock the response without this lib (see below), so please remove it
|
Thanks for your review and your help ! |
| @Field | ||
| var schema: String? = null | ||
|
|
||
| fun setRel(rel: String): LinkInfo{ |
There was a problem hiding this comment.
Let's remove these chain methods as well
|
|
||
| @JvmField | ||
| @QueryStringParam | ||
| var offset: Int? = null |
There was a problem hiding this comment.
The count/offset params seem to be useless, as this method returns a single item. Perhaps, there is an error in the documentation.
Hey,
I added getters for abuse reports.
As we cannot add an abuse report, i tried to mock the call method to retreive a fake json (found in the doc) but i didn't make it work.
So, i commented the test for the getAbuseReportMethod with hope you can help.
I also updated gradle version to last one.