From c67d1a7a856624554915754ff7bfe181d4258e59 Mon Sep 17 00:00:00 2001 From: Tomas Bjerre Date: Tue, 18 Aug 2015 21:45:25 +0200 Subject: [PATCH] Hide buttons in PR if no notification configured #51 --- CHANGELOG.md | 1 + .../java/se/bjurr/prnfs/ManualResource.java | 91 +++++++++++++------ .../PrnfsPullRequestEventListener.java | 32 ++++--- .../se/bjurr/prnfs/ManualResourceTest.java | 32 +++---- .../PrnfsPullRequestEventListenerTest.java | 41 ++++++++- .../prnfs/admin/utils/PrnfsTestBuilder.java | 28 ++++-- 6 files changed, 160 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d22a80d..b27171d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Changelog of Pull Request Notifier for Stash. ## 1.21 +* Hiding buttons in pull request view, if no notification will be fired when it is clicked * Using label without ID:s in admin GUI * To avoid using same ID:s multiple times diff --git a/src/main/java/se/bjurr/prnfs/ManualResource.java b/src/main/java/se/bjurr/prnfs/ManualResource.java index 9c2a7d31..284f17d4 100644 --- a/src/main/java/se/bjurr/prnfs/ManualResource.java +++ b/src/main/java/se/bjurr/prnfs/ManualResource.java @@ -63,6 +63,8 @@ public class ManualResource { private final PrnfsPullRequestEventListener prnfsPullRequestEventListener; private final SecurityService securityService; private final PluginSettingsFactory pluginSettingsFactory; + private final ApplicationPropertiesService propertiesService; + private final RepositoryService repositoryService; private static final List adminOk = newArrayList(); private static final List systemAdminOk = newArrayList(); static { @@ -80,34 +82,39 @@ public ManualResource(UserManager userManager, UserService userService, PluginSe this.prnfsPullRequestEventListener = prnfsPullRequestEventListener; this.securityService = securityService; this.pluginSettingsFactory = pluginSettingsFactory; + this.propertiesService = propertiesService; + this.repositoryService = repositoryService; } @GET @Produces(APPLICATION_JSON) - public Response get(@Context HttpServletRequest request) throws Exception { + public Response get(@Context HttpServletRequest request, @QueryParam("repositoryId") Integer repositoryId, + @QueryParam("pullRequestId") Long pullRequestId) throws Exception { if (userManager.getRemoteUser(request) == null) { return status(UNAUTHORIZED).build(); } List buttons = newArrayList(); - for (PrnfsButton candidate : getSettings().getButtons()) { + final PrnfsSettings settings = getSettings(); + for (PrnfsButton candidate : settings.getButtons()) { UserKey userKey = userManager.getRemoteUserKey(); - if (canUseButton(candidate, userManager.isAdmin(userKey), userManager.isSystemAdmin(userKey))) { + PrnfsPullRequestAction pullRequestAction = PrnfsPullRequestAction.valueOf(BUTTON_TRIGGER); + final PullRequest pullRequest = pullRequestService.getById(repositoryId, pullRequestId); + Map> variables = getVariables(settings, candidate.getFormIdentifier()); + if (allowedUseButton(candidate, userManager.isAdmin(userKey), userManager.isSystemAdmin(userKey)) + && triggeredByAction(settings, pullRequestAction, pullRequest, variables, request)) { buttons.add(candidate); } } return ok(gson.toJson(buttons), APPLICATION_JSON).build(); } - static boolean canUseButton(PrnfsButton candidate, boolean isAdmin, boolean isSystemAdmin) { - if (candidate.getVisibility().equals(EVERYONE)) { - return TRUE; - } - if (isSystemAdmin && systemAdminOk.contains(candidate.getVisibility())) { - return TRUE; - } else if (isAdmin && adminOk.contains(candidate.getVisibility())) { - return TRUE; - } else if (candidate.getVisibility().equals(EVERYONE)) { - return TRUE; + private boolean triggeredByAction(PrnfsSettings settings, PrnfsPullRequestAction pullRequestAction, + PullRequest pullRequest, Map> variables, HttpServletRequest request) { + for (PrnfsNotification prnfsNotification : settings.getNotifications()) { + PrnfsRenderer renderer = getRenderer(pullRequest, prnfsNotification, pullRequestAction, variables, request); + if (prnfsPullRequestEventListener.notificationTriggeredByAction(prnfsNotification, pullRequestAction, renderer)) { + return TRUE; + } } return FALSE; } @@ -121,28 +128,56 @@ public Response post(@Context HttpServletRequest request, @QueryParam("repositor return status(UNAUTHORIZED).build(); } - final PullRequest pullRequest = pullRequestService.getById(repositoryId, pullRequestId); final PrnfsSettings settings = getSettings(); for (PrnfsNotification prnfsNotification : settings.getNotifications()) { PrnfsPullRequestAction pullRequestAction = PrnfsPullRequestAction.valueOf(BUTTON_TRIGGER); - StashUser stashUser = userService.getUserBySlug(userManager.getRemoteUser(request).getUsername()); - Map> variables = new HashMap>(); - variables.put(BUTTON_TRIGGER_TITLE, new Supplier() { - @Override - public String get() { - return find(settings.getButtons(), new Predicate() { - @Override - public boolean apply(PrnfsButton input) { - return input.getFormIdentifier().equals(formIdentifier); - } - }).getTitle(); - } - }); - prnfsPullRequestEventListener.notify(prnfsNotification, pullRequestAction, pullRequest, stashUser, variables); + final PullRequest pullRequest = pullRequestService.getById(repositoryId, pullRequestId); + Map> variables = getVariables(settings, formIdentifier); + PrnfsRenderer renderer = getRenderer(pullRequest, prnfsNotification, pullRequestAction, variables, request); + if (prnfsPullRequestEventListener.notificationTriggeredByAction(prnfsNotification, pullRequestAction, renderer)) { + prnfsPullRequestEventListener.notify(prnfsNotification, pullRequestAction, pullRequest, variables, renderer); + } } return status(OK).build(); } + private Map> getVariables(final PrnfsSettings settings, final String formIdentifier) { + Map> variables = new HashMap>(); + variables.put(BUTTON_TRIGGER_TITLE, new Supplier() { + @Override + public String get() { + return find(settings.getButtons(), new Predicate() { + @Override + public boolean apply(PrnfsButton input) { + return input.getFormIdentifier().equals(formIdentifier); + } + }).getTitle(); + } + }); + return variables; + } + + private PrnfsRenderer getRenderer(final PullRequest pullRequest, PrnfsNotification prnfsNotification, + PrnfsPullRequestAction pullRequestAction, Map> variables, HttpServletRequest request) { + StashUser stashUser = userService.getUserBySlug(userManager.getRemoteUser(request).getUsername()); + return new PrnfsRenderer(pullRequest, pullRequestAction, stashUser, repositoryService, propertiesService, + prnfsNotification, variables); + } + + static boolean allowedUseButton(PrnfsButton candidate, boolean isAdmin, boolean isSystemAdmin) { + if (candidate.getVisibility().equals(EVERYONE)) { + return TRUE; + } + if (isSystemAdmin && systemAdminOk.contains(candidate.getVisibility())) { + return TRUE; + } else if (isAdmin && adminOk.contains(candidate.getVisibility())) { + return TRUE; + } else if (candidate.getVisibility().equals(EVERYONE)) { + return TRUE; + } + return FALSE; + } + private PrnfsSettings getSettings() throws Exception { final PrnfsSettings settings = securityService.withPermission(ADMIN, "Getting config").call( new Operation() { diff --git a/src/main/java/se/bjurr/prnfs/listener/PrnfsPullRequestEventListener.java b/src/main/java/se/bjurr/prnfs/listener/PrnfsPullRequestEventListener.java index 3429cbec..98027363 100644 --- a/src/main/java/se/bjurr/prnfs/listener/PrnfsPullRequestEventListener.java +++ b/src/main/java/se/bjurr/prnfs/listener/PrnfsPullRequestEventListener.java @@ -3,6 +3,8 @@ import static com.google.common.base.Charsets.UTF_8; import static com.google.common.base.Optional.absent; import static com.google.common.collect.Maps.newHashMap; +import static java.lang.Boolean.FALSE; +import static java.lang.Boolean.TRUE; import static java.util.regex.Pattern.compile; import static javax.ws.rs.core.HttpHeaders.AUTHORIZATION; import static javax.xml.bind.DatatypeConverter.printBase64Binary; @@ -142,7 +144,9 @@ public String get() { } }); } - notify(notification, action, pullRequestEvent.getPullRequest(), pullRequestEvent.getUser(), variables); + PrnfsRenderer renderer = new PrnfsRenderer(pullRequestEvent.getPullRequest(), action, pullRequestEvent.getUser(), repositoryService, + propertiesService, notification, variables); + notify(notification, action, pullRequestEvent.getPullRequest(), variables, renderer); } } catch (final ValidationException e) { logger.error("", e); @@ -151,16 +155,8 @@ public String get() { @SuppressWarnings("deprecation") public void notify(final PrnfsNotification notification, PrnfsPullRequestAction pullRequestAction, - PullRequest pullRequest, StashUser stashUser, Map> variables) { - PrnfsRenderer renderer = new PrnfsRenderer(pullRequest, pullRequestAction, stashUser, repositoryService, - propertiesService, notification, variables); - if (!notification.getTriggers().contains(pullRequestAction)) { - return; - } - if (notification.getFilterRegexp().isPresent() - && notification.getFilterString().isPresent() - && !compile(notification.getFilterRegexp().get()).matcher(renderer.render(notification.getFilterString().get())) - .find()) { + PullRequest pullRequest, Map> variables, PrnfsRenderer renderer) { + if (!notificationTriggeredByAction(notification, pullRequestAction, renderer)) { return; } Optional postContent = absent(); @@ -188,4 +184,18 @@ public void notify(final PrnfsNotification notification, PrnfsPullRequestAction urlInvoker.withProxyPassword(notification.getProxyPassword()); invoker.invoke(urlInvoker); } + + public boolean notificationTriggeredByAction(PrnfsNotification notification, PrnfsPullRequestAction pullRequestAction, + PrnfsRenderer renderer) { + if (!notification.getTriggers().contains(pullRequestAction)) { + return FALSE; + } + if (notification.getFilterRegexp().isPresent() + && notification.getFilterString().isPresent() + && !compile(notification.getFilterRegexp().get()).matcher(renderer.render(notification.getFilterString().get())) + .find()) { + return FALSE; + } + return TRUE; + } } \ No newline at end of file diff --git a/src/test/java/se/bjurr/prnfs/ManualResourceTest.java b/src/test/java/se/bjurr/prnfs/ManualResourceTest.java index 239f715d..941e65ae 100644 --- a/src/test/java/se/bjurr/prnfs/ManualResourceTest.java +++ b/src/test/java/se/bjurr/prnfs/ManualResourceTest.java @@ -4,7 +4,7 @@ import static java.lang.Boolean.TRUE; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static se.bjurr.prnfs.ManualResource.canUseButton; +import static se.bjurr.prnfs.ManualResource.allowedUseButton; import static se.bjurr.prnfs.admin.AdminFormValues.BUTTON_VISIBILITY.ADMIN; import static se.bjurr.prnfs.admin.AdminFormValues.BUTTON_VISIBILITY.EVERYONE; import static se.bjurr.prnfs.admin.AdminFormValues.BUTTON_VISIBILITY.NONE; @@ -23,23 +23,23 @@ public void testThatButtonIsOnlyDisplayedForPrivilegedUsers() { PrnfsButton systemAdmin = new PrnfsButton("", SYSTEM_ADMIN, ""); PrnfsButton none = new PrnfsButton("", NONE, ""); - assertTrue(canUseButton(everyone, TRUE, TRUE)); - assertTrue(canUseButton(everyone, TRUE, FALSE)); - assertTrue(canUseButton(everyone, FALSE, TRUE)); + assertTrue(allowedUseButton(everyone, TRUE, TRUE)); + assertTrue(allowedUseButton(everyone, TRUE, FALSE)); + assertTrue(allowedUseButton(everyone, FALSE, TRUE)); - assertTrue(canUseButton(admin, TRUE, TRUE)); - assertFalse(canUseButton(admin, FALSE, TRUE)); - assertTrue(canUseButton(admin, TRUE, FALSE)); - assertFalse(canUseButton(admin, FALSE, FALSE)); + assertTrue(allowedUseButton(admin, TRUE, TRUE)); + assertFalse(allowedUseButton(admin, FALSE, TRUE)); + assertTrue(allowedUseButton(admin, TRUE, FALSE)); + assertFalse(allowedUseButton(admin, FALSE, FALSE)); - assertTrue(canUseButton(systemAdmin, TRUE, TRUE)); - assertTrue(canUseButton(systemAdmin, FALSE, TRUE)); - assertTrue(canUseButton(systemAdmin, TRUE, FALSE)); - assertFalse(canUseButton(systemAdmin, FALSE, FALSE)); + assertTrue(allowedUseButton(systemAdmin, TRUE, TRUE)); + assertTrue(allowedUseButton(systemAdmin, FALSE, TRUE)); + assertTrue(allowedUseButton(systemAdmin, TRUE, FALSE)); + assertFalse(allowedUseButton(systemAdmin, FALSE, FALSE)); - assertFalse(canUseButton(none, FALSE, FALSE)); - assertFalse(canUseButton(none, FALSE, TRUE)); - assertFalse(canUseButton(none, TRUE, FALSE)); - assertFalse(canUseButton(none, FALSE, FALSE)); + assertFalse(allowedUseButton(none, FALSE, FALSE)); + assertFalse(allowedUseButton(none, FALSE, TRUE)); + assertFalse(allowedUseButton(none, TRUE, FALSE)); + assertFalse(allowedUseButton(none, FALSE, FALSE)); } } diff --git a/src/test/java/se/bjurr/prnfs/admin/PrnfsPullRequestEventListenerTest.java b/src/test/java/se/bjurr/prnfs/admin/PrnfsPullRequestEventListenerTest.java index 1baa82a5..d528e7ba 100644 --- a/src/test/java/se/bjurr/prnfs/admin/PrnfsPullRequestEventListenerTest.java +++ b/src/test/java/se/bjurr/prnfs/admin/PrnfsPullRequestEventListenerTest.java @@ -857,7 +857,46 @@ public void testThatButtonCanBeUsedForTriggeringEvent() throws Exception { .withFieldValue(AdminFormValues.FIELDS.button_title, "Trigger notification") .withFieldValue(AdminFormValues.FIELDS.button_visibility, AdminFormValues.BUTTON_VISIBILITY.EVERYONE.name()) .build()).store().triggerButton("Button Form").invokedOnlyUrl("http://bjurr.se/Trigger%20notification") - .hasButtonEnabled("Button Form"); + .hasButtonsEnabled("Button Form"); + } + + @Test + public void testThatButtonIsHiddenIfNoConfiguredNotificationForItWhenNotificationSetToOpened() throws Exception { + prnfsTestBuilder() + .isLoggedInAsAdmin() + .withNotification( + notificationBuilder() + .withFieldValue(AdminFormValues.FIELDS.url, "http://bjurr.se/${" + BUTTON_TRIGGER_TITLE + "}") + .withFieldValue(AdminFormValues.FIELDS.events, OPENED.name()) + .withFieldValue(AdminFormValues.FIELDS.FORM_TYPE, AdminFormValues.FORM_TYPE.TRIGGER_CONFIG_FORM.name()).build()) + .store() + .withNotification( + notificationBuilder().withFieldValue(AdminFormValues.FIELDS.FORM_IDENTIFIER, "Button Form") + .withFieldValue(AdminFormValues.FIELDS.FORM_TYPE, AdminFormValues.FORM_TYPE.BUTTON_CONFIG_FORM.name()) + .withFieldValue(AdminFormValues.FIELDS.button_title, "Trigger notification") + .withFieldValue(AdminFormValues.FIELDS.button_visibility, AdminFormValues.BUTTON_VISIBILITY.EVERYONE.name()) + .build()).store().triggerButton("Button Form").hasNoButtonsEnabled(); + } + + @Test + public void testThatButtonIsHiddenIfNoConfiguredNotificationForItWhenNotificationSetToButtonTriggered() + throws Exception { + prnfsTestBuilder() + .isLoggedInAsAdmin() + .withNotification( + notificationBuilder() + .withFieldValue(AdminFormValues.FIELDS.url, "http://bjurr.se/${" + BUTTON_TRIGGER_TITLE + "}") + .withFieldValue(AdminFormValues.FIELDS.events, BUTTON_TRIGGER) + .withFieldValue(AdminFormValues.FIELDS.filter_string, "${" + BUTTON_TRIGGER_TITLE + "}") + .withFieldValue(AdminFormValues.FIELDS.filter_regexp, "123") + .withFieldValue(AdminFormValues.FIELDS.FORM_TYPE, AdminFormValues.FORM_TYPE.TRIGGER_CONFIG_FORM.name()).build()) + .store() + .withNotification( + notificationBuilder().withFieldValue(AdminFormValues.FIELDS.FORM_IDENTIFIER, "Button Form") + .withFieldValue(AdminFormValues.FIELDS.FORM_TYPE, AdminFormValues.FORM_TYPE.BUTTON_CONFIG_FORM.name()) + .withFieldValue(AdminFormValues.FIELDS.button_title, "Trigger notification") + .withFieldValue(AdminFormValues.FIELDS.button_visibility, AdminFormValues.BUTTON_VISIBILITY.EVERYONE.name()) + .build()).store().triggerButton("Button Form").hasNoButtonsEnabled(); } @Test diff --git a/src/test/java/se/bjurr/prnfs/admin/utils/PrnfsTestBuilder.java b/src/test/java/se/bjurr/prnfs/admin/utils/PrnfsTestBuilder.java index 4eac6554..18cae869 100644 --- a/src/test/java/se/bjurr/prnfs/admin/utils/PrnfsTestBuilder.java +++ b/src/test/java/se/bjurr/prnfs/admin/utils/PrnfsTestBuilder.java @@ -95,6 +95,7 @@ public static PrnfsTestBuilder prnfsTestBuilder() { private final ConfigResource configResource; private PrnfsPullRequestEventListener listener; + private final ManualResource manualResouce; private final PluginSettings pluginSettings; @@ -333,16 +334,25 @@ public void invoke(UrlInvoker urlInvoker) { return this; } - public PrnfsTestBuilder hasButtonEnabled(final String formIdentifier) throws Exception { - List enabledButtons = new Gson().fromJson((String) manualResouce.get(request).getEntity(), - new TypeToken>() { + public PrnfsTestBuilder hasNoButtonsEnabled() throws Exception { + return hasButtonsEnabled(); + } + + public PrnfsTestBuilder hasButtonsEnabled(final String... formIdentifiers) throws Exception { + Integer repositoryId = 0; + Long pullRequestId = 0L; + List enabledButtons = new Gson().fromJson( + (String) manualResouce.get(request, repositoryId, pullRequestId).getEntity(), new TypeToken>() { }.getType()); - assertTrue(tryFind(enabledButtons, new Predicate() { - @Override - public boolean apply(PrnfsButton input) { - return input.getFormIdentifier().equals(formIdentifier); - } - }).isPresent()); + assertEquals(formIdentifiers.length, enabledButtons.size()); + for (final String formIdentifier : formIdentifiers) { + assertTrue(tryFind(enabledButtons, new Predicate() { + @Override + public boolean apply(PrnfsButton input) { + return input.getFormIdentifier().equals(formIdentifier); + } + }).isPresent()); + } return this; }