Skip to content
This repository was archived by the owner on Jun 9, 2021. It is now read-only.

Commit 445b66b

Browse files
Itay Neemantomasbjerre
Itay Neeman
authored and
tomasbjerre
committed
Add Button Trigger Confirmation Dialog
When clicking the various trigger buttons, there is no feedback to the user that the button was clicked. Even worse, the button may have been clicked but there was an error doing the trigger itself (either in the PRNFB code itself or when it actually does the final HTTP call to the backing service). This change adds an optional confirmation dialog (with a default of it being disabled) that will report, after the button press is complete and we have a response from the server, whether each trigger was successful (or if no triggers were hit).
1 parent da83990 commit 445b66b

20 files changed

+331
-54
lines changed

‎src/main/java/se/bjurr/prnfb/http/HttpResponse.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package se.bjurr.prnfb.http;
2+
3+
public class HttpResponse {
4+
public HttpResponse(int status, String content) {
5+
this.status = status;
6+
this.content = content;
7+
}
8+
9+
private int status;
10+
private String content;
11+
12+
public int getStatus() {
13+
return status;
14+
}
15+
16+
public String getContent() {
17+
return content;
18+
}
19+
}

‎src/main/java/se/bjurr/prnfb/http/Invoker.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
package se.bjurr.prnfb.http;
22

33
public interface Invoker {
4-
void invoke(UrlInvoker urlInvoker);
4+
HttpResponse invoke(UrlInvoker urlInvoker);
55
}

‎src/main/java/se/bjurr/prnfb/http/UrlInvoker.java

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public static UrlInvoker urlInvoker() {
8787
private Optional<String> proxyPassword = absent();
8888
private Optional<Integer> proxyPort = absent();
8989
private Optional<String> proxyUser = absent();
90-
private String responseString;
90+
private HttpResponse response;
9191

9292
private boolean shouldAcceptAnyCertificate;
9393

@@ -137,19 +137,19 @@ public Optional<String> getProxyUser() {
137137
return this.proxyUser;
138138
}
139139

140-
public String getResponseString() {
141-
return this.responseString;
140+
public HttpResponse getResponse() {
141+
return this.response;
142142
}
143143

144144
public InputStream getResponseStringStream() {
145-
return new ByteArrayInputStream(getResponseString().getBytes(UTF_8));
145+
return new ByteArrayInputStream(getResponse().getContent().getBytes(UTF_8));
146146
}
147147

148148
public String getUrlParam() {
149149
return this.urlParam;
150150
}
151151

152-
public void invoke() {
152+
public HttpResponse invoke() {
153153
LOG.info("Url: \"" + this.urlParam + "\"");
154154

155155
HttpRequestBase httpRequestBase = newHttpRequestBase();
@@ -160,12 +160,14 @@ public void invoke() {
160160
configureSsl(builder);
161161
configureProxy(builder);
162162

163-
this.responseString = doInvoke(httpRequestBase, builder);
164-
LOG.debug(this.responseString);
163+
this.response = doInvoke(httpRequestBase, builder);
164+
LOG.debug(this.response.getContent());
165+
166+
return this.response;
165167
}
166168

167-
public void setResponseString(String responseString) {
168-
this.responseString = responseString;
169+
public void setResponse(HttpResponse response) {
170+
this.response = response;
169171
}
170172

171173
@VisibleForTesting
@@ -330,15 +332,17 @@ HttpClientBuilder configureSsl(HttpClientBuilder builder) {
330332
}
331333

332334
@VisibleForTesting
333-
String doInvoke(HttpRequestBase httpRequestBase, HttpClientBuilder builder) {
335+
HttpResponse doInvoke(HttpRequestBase httpRequestBase, HttpClientBuilder builder) {
334336
CloseableHttpResponse httpResponse = null;
335337
try {
336338
httpResponse = builder//
337339
.build()//
338340
.execute(httpRequestBase);
339341

340342
HttpEntity entity = httpResponse.getEntity();
341-
return EntityUtils.toString(entity, UTF_8);
343+
return new HttpResponse(
344+
httpResponse.getStatusLine().getStatusCode(),
345+
EntityUtils.toString(entity, UTF_8));
342346
} catch (final Exception e) {
343347
LOG.error("", e);
344348
} finally {

‎src/main/java/se/bjurr/prnfb/listener/PrnfbPullRequestEventListener.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.slf4j.Logger;
2222

2323
import se.bjurr.prnfb.http.ClientKeyStore;
24+
import se.bjurr.prnfb.http.HttpResponse;
2425
import se.bjurr.prnfb.http.Invoker;
2526
import se.bjurr.prnfb.http.UrlInvoker;
2627
import se.bjurr.prnfb.service.PrnfbRenderer;
@@ -127,11 +128,11 @@ public boolean isNotificationTriggeredByAction(PrnfbNotification notification,
127128
return TRUE;
128129
}
129130

130-
public void notify(final PrnfbNotification notification, PrnfbPullRequestAction pullRequestAction,
131+
public HttpResponse notify(final PrnfbNotification notification, PrnfbPullRequestAction pullRequestAction,
131132
PullRequest pullRequest, PrnfbRenderer renderer, ClientKeyStore clientKeyStore, Boolean shouldAcceptAnyCertificate) {
132133
if (!isNotificationTriggeredByAction(notification, pullRequestAction, renderer, pullRequest, clientKeyStore,
133134
shouldAcceptAnyCertificate)) {
134-
return;
135+
return null;
135136
}
136137

137138
Optional<String> postContent = absent();
@@ -155,7 +156,7 @@ public void notify(final PrnfbNotification notification, PrnfbPullRequestAction
155156
.withHeader(header.getName(),
156157
renderer.render(header.getValue(), FALSE, clientKeyStore, shouldAcceptAnyCertificate));
157158
}
158-
createInvoker().invoke(urlInvoker//
159+
return createInvoker().invoke(urlInvoker//
159160
.withProxyServer(notification.getProxyServer()) //
160161
.withProxyPort(notification.getProxyPort())//
161162
.withProxyUser(notification.getProxyUser())//
@@ -219,8 +220,8 @@ private Invoker createInvoker() {
219220
}
220221
return new Invoker() {
221222
@Override
222-
public void invoke(UrlInvoker urlInvoker) {
223-
urlInvoker.invoke();
223+
public HttpResponse invoke(UrlInvoker urlInvoker) {
224+
return urlInvoker.invoke();
224225
}
225226
};
226227
}

‎src/main/java/se/bjurr/prnfb/presentation/ButtonServlet.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@
55
import static javax.ws.rs.core.Response.status;
66
import static javax.ws.rs.core.Response.Status.OK;
77
import static javax.ws.rs.core.Response.Status.UNAUTHORIZED;
8+
import static se.bjurr.prnfb.transformer.ButtonTransformer.toTriggerResultDto;
89
import static se.bjurr.prnfb.transformer.ButtonTransformer.toButtonDto;
910
import static se.bjurr.prnfb.transformer.ButtonTransformer.toButtonDtoList;
1011
import static se.bjurr.prnfb.transformer.ButtonTransformer.toPrnfbButton;
1112

1213
import java.util.Collections;
1314
import java.util.List;
15+
import java.util.Map;
1416
import java.util.UUID;
1517

1618
import javax.ws.rs.Consumes;
@@ -22,7 +24,9 @@
2224
import javax.ws.rs.Produces;
2325
import javax.ws.rs.core.Response;
2426

27+
import se.bjurr.prnfb.http.HttpResponse;
2528
import se.bjurr.prnfb.presentation.dto.ButtonDTO;
29+
import se.bjurr.prnfb.presentation.dto.TriggerResultDTO;
2630
import se.bjurr.prnfb.service.ButtonsService;
2731
import se.bjurr.prnfb.service.SettingsService;
2832
import se.bjurr.prnfb.service.UserCheckService;
@@ -148,9 +152,10 @@ public Response press(@PathParam("repositoryId") Integer repositoryId, @PathPara
148152
if (!this.userCheckService.isAllowedUseButton(button)) {
149153
return status(UNAUTHORIZED).build();
150154
}
151-
this.buttonsService.handlePressed(repositoryId, pullRequestId, buttionUuid);
155+
Map<String, HttpResponse> results = this.buttonsService.handlePressed(repositoryId, pullRequestId, buttionUuid);
152156

153-
return status(OK).build();
157+
TriggerResultDTO dto = toTriggerResultDto(button, results);
158+
return ok(dto, APPLICATION_JSON).build();
154159
}
155160

156161
}

‎src/main/java/se/bjurr/prnfb/presentation/dto/ButtonDTO.java

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public class ButtonDTO implements Comparable<ButtonDTO> {
1818
private String name;
1919
private String projectKey;
2020
private String repositorySlug;
21+
private String confirmation;
2122
private USER_LEVEL userLevel;
2223
private UUID uuid;
2324

@@ -62,6 +63,13 @@ public boolean equals(Object obj) {
6263
if (this.userLevel != other.userLevel) {
6364
return false;
6465
}
66+
if (this.confirmation == null) {
67+
if (other.confirmation != null) {
68+
return false;
69+
}
70+
} else if (!this.confirmation.equals(other.confirmation)) {
71+
return false;
72+
}
6573
if (this.uuid == null) {
6674
if (other.uuid != null) {
6775
return false;
@@ -88,6 +96,10 @@ public USER_LEVEL getUserLevel() {
8896
return this.userLevel;
8997
}
9098

99+
public String getConfirmation() {
100+
return this.confirmation;
101+
}
102+
91103
public UUID getUuid() {
92104
return this.uuid;
93105
}
@@ -105,6 +117,7 @@ public int hashCode() {
105117
result = prime * result + ((this.name == null) ? 0 : this.name.hashCode());
106118
result = prime * result + ((this.userLevel == null) ? 0 : this.userLevel.hashCode());
107119
result = prime * result + ((this.uuid == null) ? 0 : this.uuid.hashCode());
120+
result = prime * result + ((this.confirmation == null) ? 0 : this.confirmation.hashCode());
108121
return result;
109122
}
110123

@@ -128,10 +141,22 @@ public void setUuid(UUID uuid) {
128141
this.uuid = uuid;
129142
}
130143

144+
public void setConfirmation(String confirmation) {
145+
if (confirmation == null) {
146+
this.confirmation = "off";
147+
}
148+
else if (confirmation.equalsIgnoreCase("on")) {
149+
this.confirmation = "on";
150+
}
151+
else {
152+
this.confirmation = "off";
153+
}
154+
}
155+
131156
@Override
132157
public String toString() {
133158
return "ButtonDTO [name=" + this.name + ", userLevel=" + this.userLevel + ", uuid=" + this.uuid + ", repositorySlug="
134-
+ this.repositorySlug + ", projectKey=" + this.projectKey + "]";
159+
+ this.repositorySlug + ", projectKey=" + this.projectKey + ", confirmation=" + this.confirmation + "]";
135160
}
136161

137162
}

‎src/main/java/se/bjurr/prnfb/presentation/dto/TriggerResultDTO.java

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package se.bjurr.prnfb.presentation.dto;
2+
3+
import static javax.xml.bind.annotation.XmlAccessType.FIELD;
4+
5+
import java.util.HashMap;
6+
import java.util.Map;
7+
8+
import javax.xml.bind.annotation.XmlAccessorType;
9+
import javax.xml.bind.annotation.XmlRootElement;
10+
11+
import com.google.common.base.Optional;
12+
13+
import se.bjurr.prnfb.http.HttpResponse;
14+
15+
@XmlRootElement
16+
@XmlAccessorType(FIELD)
17+
public class TriggerResultDTO implements Comparable<TriggerResultDTO> {
18+
19+
private String name;
20+
private Map<String, Map<String, Object>> results;
21+
22+
@Override
23+
public int compareTo(TriggerResultDTO o) {
24+
return this.name.compareTo(o.name);
25+
}
26+
27+
@Override
28+
public boolean equals(Object obj) {
29+
if (this == obj) {
30+
return true;
31+
}
32+
if (obj == null) {
33+
return false;
34+
}
35+
if (getClass() != obj.getClass()) {
36+
return false;
37+
}
38+
TriggerResultDTO other = (TriggerResultDTO) obj;
39+
if (this.results == null) {
40+
if (other.results != null) {
41+
return false;
42+
}
43+
} else if (!this.results.equals(other.results)) {
44+
return false;
45+
}
46+
if (this.name == null) {
47+
if (other.name != null) {
48+
return false;
49+
}
50+
} else if (!this.name.equals(other.name)) {
51+
return false;
52+
}
53+
return true;
54+
}
55+
56+
public String getName() {
57+
return this.name;
58+
}
59+
60+
public Optional<Map<String, Map<String, Object>>> getResults() {
61+
return Optional.fromNullable(this.results);
62+
}
63+
64+
@Override
65+
public int hashCode() {
66+
final int prime = 31;
67+
int result = 1;
68+
result = prime * result + ((this.results == null) ? 0 : this.results.hashCode());
69+
result = prime * result + ((this.name == null) ? 0 : this.name.hashCode());
70+
return result;
71+
}
72+
73+
public void setName(String name) {
74+
this.name = name;
75+
}
76+
77+
public void setResults(Map<String, HttpResponse> results) {
78+
this.results = new HashMap<String, Map<String, Object>>();
79+
for(String key : results.keySet()) {
80+
Map<String, Object> data = new HashMap<String, Object>();
81+
data.put("status", results.get(key).getStatus());
82+
data.put("response", results.get(key).getContent());
83+
this.results.put(key, data);
84+
}
85+
}
86+
87+
@Override
88+
public String toString() {
89+
return "TriggerResultDTO [name=" + this.name + ", results=" + this.results + "]";
90+
}
91+
92+
}

‎src/main/java/se/bjurr/prnfb/service/ButtonsService.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.UUID;
1414

1515
import se.bjurr.prnfb.http.ClientKeyStore;
16+
import se.bjurr.prnfb.http.HttpResponse;
1617
import se.bjurr.prnfb.listener.PrnfbPullRequestAction;
1718
import se.bjurr.prnfb.listener.PrnfbPullRequestEventListener;
1819
import se.bjurr.prnfb.settings.PrnfbButton;
@@ -53,12 +54,12 @@ public List<PrnfbButton> getButtons(Integer repositoryId, Long pullRequestId) {
5354
return doGetButtons(notifications, clientKeyStore, pullRequest, shouldAcceptAnyCertificate);
5455
}
5556

56-
public void handlePressed(Integer repositoryId, Long pullRequestId, UUID buttonUuid) {
57+
public Map<String, HttpResponse> handlePressed(Integer repositoryId, Long pullRequestId, UUID buttonUuid) {
5758
final PrnfbSettingsData prnfbSettingsData = this.settingsService.getPrnfbSettingsData();
5859
ClientKeyStore clientKeyStore = new ClientKeyStore(prnfbSettingsData);
5960
boolean shouldAcceptAnyCertificate = prnfbSettingsData.isShouldAcceptAnyCertificate();
6061
final PullRequest pullRequest = this.pullRequestService.getById(repositoryId, pullRequestId);
61-
doHandlePressed(buttonUuid, clientKeyStore, shouldAcceptAnyCertificate, pullRequest);
62+
return doHandlePressed(buttonUuid, clientKeyStore, shouldAcceptAnyCertificate, pullRequest);
6263
}
6364

6465
private boolean isTriggeredByAction(ClientKeyStore clientKeyStore, List<PrnfbNotification> notifications,
@@ -126,19 +127,25 @@ && isTriggeredByAction(clientKeyStore, notifications, shouldAcceptAnyCertificate
126127
}
127128

128129
@VisibleForTesting
129-
void doHandlePressed(UUID buttonUuid, ClientKeyStore clientKeyStore, boolean shouldAcceptAnyCertificate,
130+
Map<String, HttpResponse> doHandlePressed(UUID buttonUuid, ClientKeyStore clientKeyStore, boolean shouldAcceptAnyCertificate,
130131
final PullRequest pullRequest) {
131132
Map<PrnfbVariable, Supplier<String>> variables = getVariables(buttonUuid);
133+
Map<String, HttpResponse> successes = new HashMap<String, HttpResponse>();
132134
for (PrnfbNotification prnfbNotification : this.settingsService.getNotifications()) {
133135
PrnfbPullRequestAction pullRequestAction = BUTTON_TRIGGER;
134136
PrnfbRenderer renderer = this.prnfbRendererFactory.create(pullRequest, pullRequestAction, prnfbNotification,
135137
variables);
136138
if (this.prnfbPullRequestEventListener.isNotificationTriggeredByAction(prnfbNotification, pullRequestAction,
137139
renderer, pullRequest, clientKeyStore, shouldAcceptAnyCertificate)) {
138-
this.prnfbPullRequestEventListener.notify(prnfbNotification, pullRequestAction, pullRequest, renderer,
140+
HttpResponse response = this.prnfbPullRequestEventListener.notify(prnfbNotification, pullRequestAction, pullRequest, renderer,
139141
clientKeyStore, shouldAcceptAnyCertificate);
142+
if (response != null) {
143+
successes.put(prnfbNotification.getName(), response);
144+
}
140145
}
141146
}
147+
148+
return successes;
142149
}
143150

144151
@VisibleForTesting

0 commit comments

Comments
 (0)