-
Notifications
You must be signed in to change notification settings - Fork 8
Triggercamapign #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Triggercamapign #44
Changes from all commits
893c117
7f07279
1afc03a
7956ca2
a24f918
fc385ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
package com.smartling.marketo.sdk; | ||
|
||
import com.smartling.marketo.sdk.rest.MarketoTriggerCampaignClient; | ||
|
||
public interface MarketoClientManager { | ||
MarketoFolderClient getMarketoFolderClient(); | ||
|
||
|
@@ -18,4 +20,6 @@ public interface MarketoClientManager { | |
MarketoProgramClient getMarketoProgramClient(); | ||
|
||
MarketoTokenClient getMarketoTokenClient(); | ||
|
||
MarketoTriggerCampaignClient getMarketoTriggerCampaignClient(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Naming convention of the Marketo and this sdk is not followed. I suggest to rename this client to SmartCampaignClient or CampaignClient. It will be able to be extended with other smart campaign endpoints. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
package com.smartling.marketo.sdk.domain.campaign; | ||
|
||
|
||
import lombok.AllArgsConstructor; | ||
import lombok.Getter; | ||
import lombok.Setter; | ||
import lombok.ToString; | ||
|
||
@Getter | ||
@Setter | ||
@ToString | ||
@AllArgsConstructor | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Data lombok's annotation can be used instead of a combination of Getter,Setter,ToString |
||
public class TriggerCampaignRequest { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm, it is confusing with RequestTriggerCampaign There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Request data (leads and tokens) should be moved to the appropriate command (RequestTriggerCampaignCommand) |
||
|
||
private LeadId[] leads; | ||
private Token[] tokens; | ||
|
||
@Getter | ||
@Setter | ||
@ToString | ||
@AllArgsConstructor | ||
public static class LeadId { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According Marketo https://developers.marketo.com/rest-api/endpoint-reference/lead-database-endpoint-reference/#/Campaigns/triggerCampaignUsingPOST |
||
|
||
private int id; | ||
} | ||
|
||
@Getter | ||
@Setter | ||
@ToString | ||
@AllArgsConstructor | ||
public static class Token { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like in the previous comment it should be separate domain class Token |
||
|
||
private String name; | ||
private String value; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package com.smartling.marketo.sdk.domain.campaign; | ||
|
||
import lombok.Getter; | ||
import lombok.Setter; | ||
import lombok.ToString; | ||
|
||
@Getter @Setter @ToString | ||
public class TriggerCampaignResult { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Result of request trigger command should be Array of Campaign domain classes |
||
private int id; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package com.smartling.marketo.sdk.rest; | ||
|
||
import com.smartling.marketo.sdk.MarketoApiException; | ||
import com.smartling.marketo.sdk.domain.campaign.TriggerCampaignRequest; | ||
import com.smartling.marketo.sdk.domain.campaign.TriggerCampaignResult; | ||
import com.smartling.marketo.sdk.rest.command.triggercampaign.ActivateSmartCampaign; | ||
import com.smartling.marketo.sdk.rest.command.triggercampaign.RequestTriggerCampaign; | ||
|
||
import java.util.List; | ||
|
||
public class MarketoTriggerCampaignClient { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add some tests for the new client |
||
|
||
private final HttpCommandExecutor httpCommandExecutor; | ||
|
||
MarketoTriggerCampaignClient(HttpCommandExecutor httpCommandExecutor) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why it isn't a public? |
||
this.httpCommandExecutor = httpCommandExecutor; | ||
} | ||
|
||
public TriggerCampaignResult requestTriggerCampaign(int campaignId, | ||
TriggerCampaignRequest.LeadId[] leadIds) | ||
throws MarketoApiException { | ||
return this.requestTriggerCampaign(campaignId, leadIds, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. omit redundant 'this' |
||
new TriggerCampaignRequest.Token[]{}); | ||
} | ||
|
||
public TriggerCampaignResult requestTriggerCampaign(int campaignId, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd have only one method: in your case, your split constructing TriggerCampaignRequest into 2 places, outside client your construct leadIds and tokens, inside client ( in RequestTriggerCampaign ) - wrap them in TriggerCampaignRequest There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, maybe it is just the bad model for TriggerCampaignRequest.
|
||
TriggerCampaignRequest.LeadId[] leadIds, TriggerCampaignRequest.Token[] tokens) | ||
throws MarketoApiException { | ||
List<TriggerCampaignResult> triggerCampaignResults = httpCommandExecutor.execute( | ||
new RequestTriggerCampaign(campaignId, leadIds, tokens)); | ||
return triggerCampaignResults.get(0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know the context, but it is an unsafe call. check size isn't empty at least. the same in next method |
||
} | ||
|
||
public TriggerCampaignResult activateSmartCampaign(int campaignId) | ||
throws MarketoApiException { | ||
List<TriggerCampaignResult> triggerCampaignResults = httpCommandExecutor.execute( | ||
new ActivateSmartCampaign(campaignId)); | ||
return triggerCampaignResults.get(0); | ||
} | ||
|
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package com.smartling.marketo.sdk.rest.command.triggercampaign; | ||
|
||
import com.smartling.marketo.sdk.domain.campaign.TriggerCampaignResult; | ||
import com.smartling.marketo.sdk.rest.command.BaseMarketoCommand; | ||
|
||
public class ActivateSmartCampaign extends BaseMarketoCommand<TriggerCampaignResult> { | ||
private final int campaignId; | ||
|
||
public ActivateSmartCampaign(int campaignId) { | ||
super(TriggerCampaignResult.class); | ||
this.campaignId = campaignId; | ||
} | ||
|
||
@Override | ||
public String getPath() { | ||
return "/asset/v1/smartCampaign/" + campaignId + "/activate.json"; | ||
} | ||
|
||
@Override | ||
public String getMethod() { | ||
return "POST_BODY"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
package com.smartling.marketo.sdk.rest.command.triggercampaign; | ||
|
||
import com.google.common.collect.ImmutableMap; | ||
import com.smartling.marketo.sdk.domain.campaign.TriggerCampaignRequest; | ||
import com.smartling.marketo.sdk.domain.campaign.TriggerCampaignResult; | ||
import com.smartling.marketo.sdk.rest.command.BaseMarketoCommand; | ||
|
||
import java.util.Map; | ||
|
||
public class RequestTriggerCampaign extends BaseMarketoCommand<TriggerCampaignResult> { | ||
|
||
private final int campaignId; | ||
private final TriggerCampaignRequest triggerCampaignRequest; | ||
|
||
public RequestTriggerCampaign(int campaignId, TriggerCampaignRequest.LeadId[] leadIds, | ||
TriggerCampaignRequest.Token[] tokens) { | ||
super(TriggerCampaignResult.class); | ||
this.campaignId = campaignId; | ||
this.triggerCampaignRequest = new TriggerCampaignRequest(leadIds, tokens); | ||
} | ||
|
||
@Override | ||
public String getPath() { | ||
return "/v1/campaigns/" + campaignId + "/trigger.json"; | ||
} | ||
|
||
@Override | ||
public String getMethod() { | ||
return "POST_BODY"; | ||
} | ||
|
||
@Override | ||
public Map<String, Object> getParameters() { | ||
ImmutableMap.Builder<String, Object> builder = ImmutableMap.<String, Object>builder() | ||
.put("input", this.triggerCampaignRequest); | ||
return builder.build(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redundant comment