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

Commit

Permalink
Fine tuning notification confirmation feature
Browse files Browse the repository at this point in the history
 * Using AUI flag.
 * Showing invoked URL and response content.
 * Also logging error when variable cant be resolved. Was giving up entirely. Will now log and continue trying to resolve other variables.
  • Loading branch information
tomasbjerre committed Jul 29, 2016
1 parent 445b66b commit aee524c
Show file tree
Hide file tree
Showing 24 changed files with 442 additions and 262 deletions.
26 changes: 26 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,32 @@ Changelog of Pull Request Notifier for Bitbucket.

## Unreleased
### No issue
Fine tuning notification confirmation feature
* Using AUI flag.
* Showing invoked URL and response content.
* Also logging error when variable cant be resolved. Was giving up entirely. Will now log and continue trying to resolve other variables.

[cd772334597de1f](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/cd772334597de1f) Tomas Bjerre *2016-07-29 22:25:58*

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).

[445b66bd464144b](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/445b66bd464144b) Itay Neeman *2016-07-29 20:39:28*

doc

[da839907caf3a40](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/da839907caf3a40) Tomas Bjerre *2016-07-28 21:01:31*

doc

[fbdec988218c8d9](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/fbdec988218c8d9) Tomas Bjerre *2016-07-18 21:19:06*
Expand Down
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ Changelog of Pull Request Notifier for Bitbucket.
</profiles>

<properties>
<bitbucket.version>4.7.1</bitbucket.version>
<bitbucket.version>4.8.1</bitbucket.version>
<bitbucket.data.version>${bitbucket.version}</bitbucket.data.version>
<amps.version>6.1.0</amps.version>
</properties>
</project>
</project>
87 changes: 72 additions & 15 deletions src/main/java/se/bjurr/prnfb/http/HttpResponse.java
Original file line number Diff line number Diff line change
@@ -1,19 +1,76 @@
package se.bjurr.prnfb.http;

import java.net.URI;

public class HttpResponse {
public HttpResponse(int status, String content) {
this.status = status;
this.content = content;
}

private int status;
private String content;

public int getStatus() {
return status;
}

public String getContent() {
return content;
}
private final String content;

private final int status;

private final URI uri;

public HttpResponse(URI uri, int status, String content) {
this.uri = uri;
this.status = status;
this.content = content;
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
HttpResponse other = (HttpResponse) obj;
if (this.content == null) {
if (other.content != null) {
return false;
}
} else if (!this.content.equals(other.content)) {
return false;
}
if (this.status != other.status) {
return false;
}
if (this.uri == null) {
if (other.uri != null) {
return false;
}
} else if (!this.uri.equals(other.uri)) {
return false;
}
return true;
}

public String getContent() {
return this.content;
}

public int getStatus() {
return this.status;
}

public URI getUri() {
return this.uri;
}

@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ((this.content == null) ? 0 : this.content.hashCode());
result = prime * result + this.status;
result = prime * result + ((this.uri == null) ? 0 : this.uri.hashCode());
return result;
}

@Override
public String toString() {
return "HttpResponse [content=" + this.content + ", status=" + this.status + ", uri=" + this.uri + "]";
}
}
80 changes: 80 additions & 0 deletions src/main/java/se/bjurr/prnfb/http/NotificationResponse.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package se.bjurr.prnfb.http;

import java.util.UUID;

public class NotificationResponse {
private final HttpResponse httpResponse;
private final UUID notification;
private final String notificationName;

public NotificationResponse(UUID notification, String notificationName, HttpResponse httpResponse) {
this.notification = notification;
this.notificationName = notificationName;
this.httpResponse = httpResponse;
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
NotificationResponse other = (NotificationResponse) obj;
if (this.httpResponse == null) {
if (other.httpResponse != null) {
return false;
}
} else if (!this.httpResponse.equals(other.httpResponse)) {
return false;
}
if (this.notification == null) {
if (other.notification != null) {
return false;
}
} else if (!this.notification.equals(other.notification)) {
return false;
}
if (this.notificationName == null) {
if (other.notificationName != null) {
return false;
}
} else if (!this.notificationName.equals(other.notificationName)) {
return false;
}
return true;
}

public HttpResponse getHttpResponse() {
return this.httpResponse;
}

public UUID getNotification() {
return this.notification;
}

public String getNotificationName() {
return this.notificationName;
}

@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ((this.httpResponse == null) ? 0 : this.httpResponse.hashCode());
result = prime * result + ((this.notification == null) ? 0 : this.notification.hashCode());
result = prime * result + ((this.notificationName == null) ? 0 : this.notificationName.hashCode());
return result;
}

@Override
public String toString() {
return "NotificationResponse [httpResponse=" + this.httpResponse + ", notification=" + this.notification
+ ", notificationName=" + this.notificationName + "]";
}

}
7 changes: 3 additions & 4 deletions src/main/java/se/bjurr/prnfb/http/UrlInvoker.java
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public HttpResponse invoke() {

this.response = doInvoke(httpRequestBase, builder);
LOG.debug(this.response.getContent());

return this.response;
}

Expand Down Expand Up @@ -340,9 +340,8 @@ HttpResponse doInvoke(HttpRequestBase httpRequestBase, HttpClientBuilder builder
.execute(httpRequestBase);

HttpEntity entity = httpResponse.getEntity();
return new HttpResponse(
httpResponse.getStatusLine().getStatusCode(),
EntityUtils.toString(entity, UTF_8));
return new HttpResponse(httpRequestBase.getURI(), httpResponse.getStatusLine().getStatusCode(),
EntityUtils.toString(entity, UTF_8));
} catch (final Exception e) {
LOG.error("", e);
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import se.bjurr.prnfb.http.ClientKeyStore;
import se.bjurr.prnfb.http.HttpResponse;
import se.bjurr.prnfb.http.Invoker;
import se.bjurr.prnfb.http.NotificationResponse;
import se.bjurr.prnfb.http.UrlInvoker;
import se.bjurr.prnfb.service.PrnfbRenderer;
import se.bjurr.prnfb.service.PrnfbRendererFactory;
Expand Down Expand Up @@ -128,7 +129,7 @@ public boolean isNotificationTriggeredByAction(PrnfbNotification notification,
return TRUE;
}

public HttpResponse notify(final PrnfbNotification notification, PrnfbPullRequestAction pullRequestAction,
public NotificationResponse notify(final PrnfbNotification notification, PrnfbPullRequestAction pullRequestAction,
PullRequest pullRequest, PrnfbRenderer renderer, ClientKeyStore clientKeyStore, Boolean shouldAcceptAnyCertificate) {
if (!isNotificationTriggeredByAction(notification, pullRequestAction, renderer, pullRequest, clientKeyStore,
shouldAcceptAnyCertificate)) {
Expand Down Expand Up @@ -156,12 +157,14 @@ public HttpResponse notify(final PrnfbNotification notification, PrnfbPullReques
.withHeader(header.getName(),
renderer.render(header.getValue(), FALSE, clientKeyStore, shouldAcceptAnyCertificate));
}
return createInvoker().invoke(urlInvoker//
HttpResponse httpResponse = createInvoker().invoke(urlInvoker//
.withProxyServer(notification.getProxyServer()) //
.withProxyPort(notification.getProxyPort())//
.withProxyUser(notification.getProxyUser())//
.withProxyPassword(notification.getProxyPassword())//
.shouldAcceptAnyCertificate(shouldAcceptAnyCertificate));

return new NotificationResponse(notification.getUuid(), notification.getName(), httpResponse);
}

@EventListener
Expand Down
11 changes: 5 additions & 6 deletions src/main/java/se/bjurr/prnfb/presentation/ButtonServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@
import static javax.ws.rs.core.Response.status;
import static javax.ws.rs.core.Response.Status.OK;
import static javax.ws.rs.core.Response.Status.UNAUTHORIZED;
import static se.bjurr.prnfb.transformer.ButtonTransformer.toTriggerResultDto;
import static se.bjurr.prnfb.transformer.ButtonTransformer.toButtonDto;
import static se.bjurr.prnfb.transformer.ButtonTransformer.toButtonDtoList;
import static se.bjurr.prnfb.transformer.ButtonTransformer.toPrnfbButton;
import static se.bjurr.prnfb.transformer.ButtonTransformer.toTriggerResultDto;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.UUID;

import javax.ws.rs.Consumes;
Expand All @@ -24,9 +23,9 @@
import javax.ws.rs.Produces;
import javax.ws.rs.core.Response;

import se.bjurr.prnfb.http.HttpResponse;
import se.bjurr.prnfb.http.NotificationResponse;
import se.bjurr.prnfb.presentation.dto.ButtonDTO;
import se.bjurr.prnfb.presentation.dto.TriggerResultDTO;
import se.bjurr.prnfb.presentation.dto.NotificationResponseDTO;
import se.bjurr.prnfb.service.ButtonsService;
import se.bjurr.prnfb.service.SettingsService;
import se.bjurr.prnfb.service.UserCheckService;
Expand Down Expand Up @@ -152,9 +151,9 @@ public Response press(@PathParam("repositoryId") Integer repositoryId, @PathPara
if (!this.userCheckService.isAllowedUseButton(button)) {
return status(UNAUTHORIZED).build();
}
Map<String, HttpResponse> results = this.buttonsService.handlePressed(repositoryId, pullRequestId, buttionUuid);
List<NotificationResponse> results = this.buttonsService.handlePressed(repositoryId, pullRequestId, buttionUuid);

TriggerResultDTO dto = toTriggerResultDto(button, results);
List<NotificationResponseDTO> dto = toTriggerResultDto(results);
return ok(dto, APPLICATION_JSON).build();
}

Expand Down
26 changes: 9 additions & 17 deletions src/main/java/se/bjurr/prnfb/presentation/dto/ButtonDTO.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
@XmlAccessorType(FIELD)
public class ButtonDTO implements Comparable<ButtonDTO> {

private ON_OR_OFF confirmation;
private String name;
private String projectKey;
private String repositorySlug;
private String confirmation;
private USER_LEVEL userLevel;
private UUID uuid;

Expand Down Expand Up @@ -80,6 +80,10 @@ public boolean equals(Object obj) {
return true;
}

public ON_OR_OFF getConfirmation() {
return this.confirmation;
}

public String getName() {
return this.name;
}
Expand All @@ -96,10 +100,6 @@ public USER_LEVEL getUserLevel() {
return this.userLevel;
}

public String getConfirmation() {
return this.confirmation;
}

public UUID getUuid() {
return this.uuid;
}
Expand All @@ -121,6 +121,10 @@ public int hashCode() {
return result;
}

public void setConfirmation(ON_OR_OFF confirmation) {
this.confirmation = confirmation;
}

public void setName(String name) {
this.name = name;
}
Expand All @@ -141,18 +145,6 @@ public void setUuid(UUID uuid) {
this.uuid = uuid;
}

public void setConfirmation(String confirmation) {
if (confirmation == null) {
this.confirmation = "off";
}
else if (confirmation.equalsIgnoreCase("on")) {
this.confirmation = "on";
}
else {
this.confirmation = "off";
}
}

@Override
public String toString() {
return "ButtonDTO [name=" + this.name + ", userLevel=" + this.userLevel + ", uuid=" + this.uuid + ", repositorySlug="
Expand Down
Loading

0 comments on commit aee524c

Please sign in to comment.