From f11592770694e9d0a391a31fa23f455bc05756c1 Mon Sep 17 00:00:00 2001 From: James Moger <james.moger@gitblit.com> Date: Fri, 31 Oct 2014 09:22:01 -0400 Subject: [PATCH] Merged #212 "Gracefully handle missing integration branch in ticket page" --- src/main/java/com/gitblit/tickets/TicketNotifier.java | 131 ++++++++++++++++++++++++++----------------- 1 files changed, 79 insertions(+), 52 deletions(-) diff --git a/src/main/java/com/gitblit/tickets/TicketNotifier.java b/src/main/java/com/gitblit/tickets/TicketNotifier.java index b4c3bae..5979cf2 100644 --- a/src/main/java/com/gitblit/tickets/TicketNotifier.java +++ b/src/main/java/com/gitblit/tickets/TicketNotifier.java @@ -135,6 +135,7 @@ StringBuilder html = new StringBuilder(); html.append("<head>"); html.append(readStyle()); + html.append(readViewTicketAction(ticket)); html.append("</head>"); html.append("<body>"); html.append(MarkdownUtils.transformGFM(settings, markdown, ticket.repository)); @@ -174,52 +175,13 @@ fieldExclusions.addAll(Arrays.asList(Field.watchers, Field.voters)); StringBuilder sb = new StringBuilder(); - boolean newTicket = false; + boolean newTicket = lastChange.isStatusChange() && Status.New == lastChange.getStatus(); boolean isFastForward = true; List<RevCommit> commits = null; DiffStat diffstat = null; String pattern; - if (lastChange.isStatusChange()) { - Status state = lastChange.getStatus(); - switch (state) { - case New: - // new ticket - newTicket = true; - fieldExclusions.add(Field.status); - fieldExclusions.add(Field.title); - fieldExclusions.add(Field.body); - if (lastChange.hasPatchset()) { - pattern = "**{0}** is proposing a change."; - } else { - pattern = "**{0}** created this ticket."; - } - sb.append(MessageFormat.format(pattern, user.getDisplayName())); - break; - default: - // some form of resolved - if (lastChange.hasField(Field.mergeSha)) { - // closed by push (merged patchset) - pattern = "**{0}** closed this ticket by pushing {1} to {2}."; - - // identify patch that closed the ticket - String merged = ticket.mergeSha; - for (Patchset patchset : ticket.getPatchsets()) { - if (patchset.tip.equals(ticket.mergeSha)) { - merged = patchset.toString(); - break; - } - } - sb.append(MessageFormat.format(pattern, user.getDisplayName(), merged, ticket.mergeTo)); - } else { - // workflow status change by user - pattern = "**{0}** changed the status of this ticket to **{1}**."; - sb.append(MessageFormat.format(pattern, user.getDisplayName(), lastChange.getStatus().toString().toUpperCase())); - } - break; - } - sb.append(HARD_BRK); - } else if (lastChange.hasPatchset()) { + if (lastChange.hasPatchset()) { // patchset uploaded Patchset patchset = lastChange.patchset; String base = ""; @@ -243,19 +205,32 @@ } catch (Exception e) { Logger.getLogger(getClass()).error("failed to get changed paths", e); } finally { - repo.close(); + if (repo != null) { + repo.close(); + } } - // describe the patchset String compareUrl = ticketService.getCompareUrl(ticket, base, patchset.tip); - if (patchset.isFF()) { - pattern = "**{0}** added {1} {2} to patchset {3}."; - sb.append(MessageFormat.format(pattern, user.getDisplayName(), patchset.added, patchset.added == 1 ? "commit" : "commits", patchset.number)); + + if (newTicket) { + // new proposal + pattern = "**{0}** is proposing a change."; + sb.append(MessageFormat.format(pattern, user.getDisplayName())); + fieldExclusions.add(Field.status); + fieldExclusions.add(Field.title); + fieldExclusions.add(Field.body); } else { - pattern = "**{0}** uploaded patchset {1}. *({2})*"; - sb.append(MessageFormat.format(pattern, user.getDisplayName(), patchset.number, patchset.type.toString().toUpperCase())); + // describe the patchset + if (patchset.isFF()) { + pattern = "**{0}** added {1} {2} to patchset {3}."; + sb.append(MessageFormat.format(pattern, user.getDisplayName(), patchset.added, patchset.added == 1 ? "commit" : "commits", patchset.number)); + } else { + pattern = "**{0}** uploaded patchset {1}. *({2})*"; + sb.append(MessageFormat.format(pattern, user.getDisplayName(), patchset.number, patchset.type.toString().toUpperCase())); + } } sb.append(HARD_BRK); + sb.append(MessageFormat.format("{0} {1}, {2} {3}, <span style=\"color:darkgreen;\">+{4} insertions</span>, <span style=\"color:darkred;\">-{5} deletions</span> from {6}. [compare]({7})", commits.size(), commits.size() == 1 ? "commit" : "commits", diffstat.paths.size(), @@ -275,6 +250,32 @@ break; default: break; + } + sb.append(HARD_BRK); + } else if (lastChange.isStatusChange()) { + if (newTicket) { + fieldExclusions.add(Field.status); + fieldExclusions.add(Field.title); + fieldExclusions.add(Field.body); + pattern = "**{0}** created this ticket."; + sb.append(MessageFormat.format(pattern, user.getDisplayName())); + } else if (lastChange.hasField(Field.mergeSha)) { + // closed by merged + pattern = "**{0}** closed this ticket by merging {1} to {2}."; + + // identify patchset that closed the ticket + String merged = ticket.mergeSha; + for (Patchset patchset : ticket.getPatchsets()) { + if (patchset.tip.equals(ticket.mergeSha)) { + merged = patchset.toString(); + break; + } + } + sb.append(MessageFormat.format(pattern, user.getDisplayName(), merged, ticket.mergeTo)); + } else { + // workflow status change by user + pattern = "**{0}** changed the status of this ticket to **{1}**."; + sb.append(MessageFormat.format(pattern, user.getDisplayName(), lastChange.getStatus().toString().toUpperCase())); } sb.append(HARD_BRK); } else if (lastChange.hasReview()) { @@ -491,6 +492,7 @@ instructions = instructions.replace("${ticketRef}", ticketBranch); instructions = instructions.replace("${patchsetRef}", patchsetBranch); instructions = instructions.replace("${reviewBranch}", reviewBranch); + instructions = instructions.replace("${ticketBranch}", ticketBranch); return instructions; } @@ -521,11 +523,18 @@ // // Direct TO recipients + // reporter & responsible // + Set<String> tos = new TreeSet<String>(); + tos.add(ticket.createdBy); + if (!StringUtils.isEmpty(ticket.responsible)) { + tos.add(ticket.responsible); + } + Set<String> toAddresses = new TreeSet<String>(); - for (String name : ticket.getParticipants()) { + for (String name : tos) { UserModel user = userManager.getUserModel(name); - if (user != null) { + if (user != null && !user.disabled) { if (!StringUtils.isEmpty(user.emailAddress)) { if (user.canView(repository)) { toAddresses.add(user.emailAddress); @@ -537,12 +546,16 @@ } } } - mailing.setRecipients(toAddresses); // // CC recipients // Set<String> ccs = new TreeSet<String>(); + + // repository owners + if (!ArrayUtils.isEmpty(repository.owners)) { + ccs.addAll(repository.owners); + } // cc users mentioned in last comment Change lastChange = ticket.changes.get(ticket.changes.size() - 1); @@ -563,7 +576,7 @@ Set<String> ccAddresses = new TreeSet<String>(); for (String name : ccs) { UserModel user = userManager.getUserModel(name); - if (user != null) { + if (user != null && !user.disabled) { if (!StringUtils.isEmpty(user.emailAddress)) { if (user.canView(repository)) { ccAddresses.add(user.emailAddress); @@ -582,6 +595,14 @@ } ccAddresses.addAll(settings.getStrings(Keys.mail.mailingLists)); + // respect the author's email preference + UserModel lastAuthor = userManager.getUserModel(lastChange.author); + if (lastAuthor != null && !lastAuthor.getPreferences().isEmailMeOnMyTicketChanges()) { + toAddresses.remove(lastAuthor.emailAddress); + ccAddresses.remove(lastAuthor.emailAddress); + } + + mailing.setRecipients(toAddresses); mailing.setCCs(ccAddresses); } @@ -593,6 +614,12 @@ return sb.toString(); } + protected String readViewTicketAction(TicketModel ticket) { + String action = readResource("viewTicket.html"); + action = action.replace("${url}", ticketService.getTicketUrl(ticket)); + return action; + } + protected String readResource(String resource) { StringBuilder sb = new StringBuilder(); InputStream is = null; -- Gitblit v1.9.1