From ec8f934c49a92319b6fa639ffa0e6dd5850e5d73 Mon Sep 17 00:00:00 2001 From: Greg Gibeling Date: Fri, 13 Jun 2025 12:13:50 -0700 Subject: [PATCH] Add support for status adjustments in comments, and improve change back propagation --- pj-report/pom.xml | 5 ++ .../com/g2forge/project/report/Billing.java | 43 +++++++++-- .../com/g2forge/project/report/Change.java | 73 ++++++++++++++----- .../com/g2forge/project/report/Request.java | 5 ++ .../project/report/StatusAdjustment.java | 56 ++++++++++++++ .../g2forge/project/report/TestChange.java | 20 +++-- .../project/report/TestStatusAdjustment.java | 35 +++++++++ 7 files changed, 206 insertions(+), 31 deletions(-) create mode 100644 pj-report/src/main/java/com/g2forge/project/report/StatusAdjustment.java create mode 100644 pj-report/src/test/java/com/g2forge/project/report/TestStatusAdjustment.java diff --git a/pj-report/pom.xml b/pj-report/pom.xml index 233e411..8570c5f 100644 --- a/pj-report/pom.xml +++ b/pj-report/pom.xml @@ -29,5 +29,10 @@ gb-csv ${gearbox.version} + + com.g2forge.alexandria + ax-parse + ${alexandria.version} + diff --git a/pj-report/src/main/java/com/g2forge/project/report/Billing.java b/pj-report/src/main/java/com/g2forge/project/report/Billing.java index 8b69127..8ae82b7 100644 --- a/pj-report/src/main/java/com/g2forge/project/report/Billing.java +++ b/pj-report/src/main/java/com/g2forge/project/report/Billing.java @@ -11,6 +11,7 @@ import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -25,6 +26,9 @@ import com.atlassian.jira.rest.client.api.IssueRestClient; import com.atlassian.jira.rest.client.api.domain.BasicComponent; import com.atlassian.jira.rest.client.api.domain.ChangelogGroup; +import com.atlassian.jira.rest.client.api.domain.ChangelogItem; +import com.atlassian.jira.rest.client.api.domain.Comment; +import com.atlassian.jira.rest.client.api.domain.FieldType; import com.atlassian.jira.rest.client.api.domain.Issue; import com.atlassian.jira.rest.client.api.domain.SearchResult; import com.atlassian.jira.rest.client.api.domain.User; @@ -47,6 +51,7 @@ import com.g2forge.gearbox.argparse.ArgumentParser; import com.g2forge.gearbox.jira.ExtendedJiraRestClient; import com.g2forge.gearbox.jira.JiraAPI; +import com.g2forge.gearbox.jira.fields.KnownField; import com.g2forge.project.core.HConfig; import com.g2forge.project.core.Server; @@ -176,9 +181,20 @@ public static void main(String[] args) throws Throwable { protected final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy/MM/dd"); - protected List computeChanges(ExtendedJiraRestClient client, Server server, IFunction1 userToFriendly, String issueKey, ZonedDateTime start, ZonedDateTime end) throws InterruptedException, ExecutionException { + protected List computeChanges(ExtendedJiraRestClient client, Server server, Request request, IFunction1 userToFriendly, String issueKey, ZonedDateTime start, ZonedDateTime end) throws InterruptedException, ExecutionException { final Issue issue = client.getIssueClient().getIssue(issueKey, HCollection.asList(IssueRestClient.Expandos.CHANGELOG)).get(); - final Iterable changelog = issue.getChangelog(); + final List changelog = new ArrayList<>(HCollection.asListIterable(issue.getChangelog())); + for (Comment comment : issue.getComments()) { + final String body = comment.getBody(); + final List adjustments = StatusAdjustment.parse(body); + if (!adjustments.isEmpty()) { + final ZoneId zone = request.getZone(comment.getAuthor().getName()); + for (StatusAdjustment adjustment : adjustments) { + final ZonedDateTime when = adjustment.getWhen().atZone(zone); + changelog.add(new ChangelogGroup(comment.getAuthor(), convert(when), HCollection.asList(new ChangelogItem(FieldType.JIRA, KnownField.Status.getName(), adjustment.getFrom(), adjustment.getFrom(), adjustment.getTo(), adjustment.getTo())))); + } + } + } final IFunction1 users = new Cache<>(primaryKey -> { if (primaryKey == null) return null; @@ -217,7 +233,7 @@ protected List examineIssue(final ExtendedJiraRestClient client, Server final Set billableComponents = HCollection.asListIterable(issue.getComponents()).stream().map(BasicComponent::getName).distinct().filter(isComponentBillable).collect(Collectors.toSet()); if (billableComponents.isEmpty()) return null; - final List changes = computeChanges(client, server, userToFriendly, issue.getKey(), request.getStart().atStartOfDay(ZoneId.systemDefault()), request.getEnd().atStartOfDay(ZoneId.systemDefault())); + final List changes = computeChanges(client, server, request, userToFriendly, issue.getKey(), request.getStart().atStartOfDay(ZoneId.systemDefault()), request.getEnd().atStartOfDay(ZoneId.systemDefault())); final Map billableHoursByUser = computeBillableHoursByUser(changes, isStatusBillable, request.getUsers()::get); final Map billableHoursByUserDividedByComponents = billableHoursByUser.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue() / billableComponents.size())); for (String billableComponent : billableComponents) { @@ -228,7 +244,7 @@ protected List examineIssue(final ExtendedJiraRestClient client, Server return changes; } - protected static final DateTimeFormatter RANGE_FORMAT = DateTimeFormatter.ofPattern("yyyy/MM/dd/ HH:mm:ss"); + public static final DateTimeFormatter DATETIME_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm[:ss]"); @Override public IExit invoke(CommandInvocation invocation) throws Throwable { @@ -257,8 +273,14 @@ public IExit invoke(CommandInvocation invocation) thro issues = relevantIssues.stream().collect(Collectors.toMap(Issue::getKey, IFunction1.identity(), (i0, i1) -> i0)); } log.info("Found: {}", issues.keySet().stream().collect(HCollector.joining(", ", ", & "))); + final Map errors = new LinkedHashMap<>(); for (Issue issue : issues.values()) { - changes.put(issue.getKey(), examineIssue(client, server, request, isStatusBillable, isComponentBillable, userToFriendly, issue, billBuilder)); + try { + changes.put(issue.getKey(), examineIssue(client, server, request, isStatusBillable, isComponentBillable, userToFriendly, issue, billBuilder)); + } catch (Throwable throwable) { + log.error("Failed to incorporate {} into billing report: {}", issue.getKey(), throwable); + errors.put(issue, throwable); + } } final Bill bill = billBuilder.build(); @@ -284,9 +306,9 @@ public IExit invoke(CommandInvocation invocation) thro final boolean newBillable = isStatusBillable.test(change.getStatus()); if (newBillable != currentBillable) { currentBillable = newBillable; - final ZoneId zone = change.getAssignee() == null ? ZoneId.systemDefault() : request.getUsers().get(change.getAssignee()).getZone(); + final ZoneId zone = request.getZone(change.getAssignee()); final LocalDateTime local = change.getStart().withZoneSameInstant(zone).toLocalDateTime(); - ranges.append(RANGE_FORMAT.format(local)).append(" (@").append(change.getAssignee() == null ? zone : change.getAssignee()).append(')'); + ranges.append(DATETIME_FORMAT.format(local)).append(" (@").append(change.getAssignee() == null ? zone : change.getAssignee()).append(')'); ranges.append(' ').append(((issueChanges.size() - 1) == i) ? "End" : (newBillable ? "Start" : "Stop")).append('\n'); } } @@ -307,6 +329,13 @@ public IExit invoke(CommandInvocation invocation) thro } } + if (!errors.isEmpty()) { + log.error("One or more issues could not be incorporated into the report (please see above for complete errors):"); + for (Map.Entry entry : errors.entrySet()) { + log.error("\t{}: {}", entry.getKey().getKey(), entry.getValue().getMessage()); + } + return IStandardCommand.FAIL; + } } // TODO: Report on any times where a person was not billing to anything, but was working diff --git a/pj-report/src/main/java/com/g2forge/project/report/Change.java b/pj-report/src/main/java/com/g2forge/project/report/Change.java index 0a60c66..48f44a4 100644 --- a/pj-report/src/main/java/com/g2forge/project/report/Change.java +++ b/pj-report/src/main/java/com/g2forge/project/report/Change.java @@ -21,15 +21,40 @@ @Builder(toBuilder = true) @RequiredArgsConstructor public class Change { - protected final ZonedDateTime start; - - protected final String assignee; + /** + * Back propagate the from assignee and status for a newly found change log to a previous change. This allows each change to have from "from" and "to" + * information. This is made slightly more complex because not every change affects assignee, and not ever change affects status. + * + * @param retVal The list of changes being built up. + * @param fromAssignee The "from" assignee of the newly found change. + * @param fromStatus The "from" status of the newly found change. + */ + protected static void backPropagate(final List retVal, String fromAssignee, String fromStatus) { + for (int i = retVal.size() - 1; i >= 0; i--) { + final Change prev = retVal.get(i); + if ((prev.getAssignee() != null) && (prev.getStatus() != null)) break; + if ((fromAssignee == null) && (fromStatus == null)) break; - protected final String status; + final Change.ChangeBuilder builder = prev.toBuilder(); + if (fromAssignee != null) { + if (prev.getAssignee() == null) builder.assignee(fromAssignee); + else if (!prev.getAssignee().equals(fromAssignee)) { + throw new IllegalArgumentException("Cannot back propogate change to assignee to change at " + prev.getStart() + ", because previous assignee (" + fromAssignee + ") does not match expected previous assignee (" + prev.getAssignee() + ")"); + } else fromAssignee = null; + } + if (fromStatus != null) { + if (prev.getStatus() == null) builder.status(fromStatus); + else if (!prev.getStatus().equals(fromStatus)) { + throw new IllegalArgumentException("Cannot back propogate change to status to change at " + prev.getStart() + ", because previous status (" + fromStatus + ") does not match expected previous status (" + prev.getStatus() + ")"); + } else fromStatus = null; + } + retVal.set(i, builder.build()); + } + } public static List toChanges(final Iterable changelog, ZonedDateTime start, ZonedDateTime end, String assignee, String status, IFunction1 assigneeResolver) { final List retVal = new ArrayList<>(); - String finalAssignee = assignee, finalStatus = status; + String finalAssignee = assignee, finalStatus = status, prevFromStatus = null, prevToStatus = null; boolean foundFinalAssignee = false, foundFinalStatus = false; final List sorted = HCollection.asListIterable(changelog).stream().sorted(new MappedComparator<>(ChangelogGroup::getCreated, ComparableComparator.create())).collect(Collectors.toList()); for (ChangelogGroup changelogGroup : sorted) { @@ -50,6 +75,9 @@ public static List toChanges(final Iterable changelog, Z } } + // Skip duplicate status changes, which are generally caused by adjustments + if ((toAssignee == null) && (toStatus != null) && (prevFromStatus != null) && prevFromStatus.equals(fromStatus) && prevToStatus.equals(toStatus)) continue; + // IF the status changed (not all change log groups include a chance to the status), then... if ((toAssignee != null) || (toStatus != null)) { if (created.isAfter(end)) { @@ -65,28 +93,39 @@ public static List toChanges(final Iterable changelog, Z } else { // If this is the first change, record the starting info, otherwise back propagate any new information we just learned if (retVal.isEmpty()) retVal.add(new Change(start, fromAssignee, fromStatus)); - else backPropagate(retVal, fromAssignee, fromStatus); + else { + try { + backPropagate(retVal, fromAssignee, fromStatus); + } catch (Throwable throwable) { + throw new RuntimeException("Failed to backpropogate information about changelog group at " + changelogGroup.getCreated() + " by " + (changelogGroup.getAuthor() != null ? changelogGroup.getAuthor().getDisplayName() : "No Author"), throwable); + } + } retVal.add(new Change(created, toAssignee, toStatus)); } } + + if (toStatus != null) { + prevFromStatus = fromStatus; + prevToStatus = toStatus; + } } // Add a start marker if we didn't get a chance to already if (retVal.isEmpty()) retVal.add(new Change(start, finalAssignee, finalStatus)); - else backPropagate(retVal, finalAssignee, finalStatus); + else { + try { + backPropagate(retVal, finalAssignee, finalStatus); + } catch (Throwable throwable) { + throw new RuntimeException("Failed to backpropogate information for final assignee and status", throwable); + } + } // Add an end marker if we didn't get a chance at exactly the right time if (!retVal.get(retVal.size() - 1).getStart().isEqual(end)) retVal.add(new Change(end, finalAssignee, finalStatus)); return retVal; } - protected static void backPropagate(final List retVal, String fromAssignee, String fromStatus) { - for (int i = retVal.size() - 1; i >= 0; i--) { - final Change prev = retVal.get(i); - if (prev.getAssignee() != null && prev.getStatus() != null) break; + protected final ZonedDateTime start; - final Change.ChangeBuilder builder = prev.toBuilder(); - if (prev.getAssignee() == null) builder.assignee(fromAssignee); - if (prev.getStatus() == null) builder.status(fromStatus); - retVal.set(i, builder.build()); - } - } + protected final String assignee; + + protected final String status; } \ No newline at end of file diff --git a/pj-report/src/main/java/com/g2forge/project/report/Request.java b/pj-report/src/main/java/com/g2forge/project/report/Request.java index 025be9c..c07ae34 100644 --- a/pj-report/src/main/java/com/g2forge/project/report/Request.java +++ b/pj-report/src/main/java/com/g2forge/project/report/Request.java @@ -1,6 +1,7 @@ package com.g2forge.project.report; import java.time.LocalDate; +import java.time.ZoneId; import java.util.Map; import java.util.Set; @@ -27,4 +28,8 @@ public class Request { protected final LocalDate start; protected final LocalDate end; + + public ZoneId getZone(String user) { + return user == null ? ZoneId.systemDefault() : getUsers().get(user).getZone(); + } } \ No newline at end of file diff --git a/pj-report/src/main/java/com/g2forge/project/report/StatusAdjustment.java b/pj-report/src/main/java/com/g2forge/project/report/StatusAdjustment.java new file mode 100644 index 0000000..50dba2d --- /dev/null +++ b/pj-report/src/main/java/com/g2forge/project/report/StatusAdjustment.java @@ -0,0 +1,56 @@ +package com.g2forge.project.report; + +import java.time.LocalDateTime; +import java.util.ArrayList; +import java.util.List; + +import com.g2forge.alexandria.java.core.helpers.HCollection; +import com.g2forge.alexandria.java.fluent.optional.IOptional; +import com.g2forge.alexandria.parse.IMatch; +import com.g2forge.alexandria.parse.IMatcher; +import com.g2forge.alexandria.parse.IMatcherBuilder; +import com.g2forge.alexandria.parse.NamedCharacterClass; +import com.g2forge.alexandria.parse.regex.Regex; +import com.g2forge.alexandria.parse.regex.RegexMatcher; +import com.g2forge.alexandria.parse.regex.RegexMatcher.Flag; + +import lombok.Builder; +import lombok.Data; +import lombok.RequiredArgsConstructor; + +@Data +@Builder(toBuilder = true) +@RequiredArgsConstructor +public class StatusAdjustment { + protected static final IMatcher GAP = pattern().charClass(false, cc -> cc.named(NamedCharacterClass.Space)).star().build(); + + protected static final IMatcher LOCALDATETIME = pattern().digit(10).repeat(4).text("-").digit(10).repeat(2).text("-").digit(10).repeat(2).with(GAP).digit(10).repeat(1, 2).text(":").digit(10).repeat(2).group(g -> g.text(":").digit(10).plus()).opt().build(); + + protected static final IMatcher STATUS = pattern().charClass(false, cc -> cc.range('a', 'z')).plus().build(); + + @SuppressWarnings("unchecked") + protected static final IMatcher ADJUSTMENT = RegexMatcher.builder(Flag.CASE_INSENSITIVE).group(g -> g.text("*").with(GAP)).opt().text("status").with(GAP).opt().text("adjustment").group(g -> g.with(GAP).opt().text(":").opt()).with(GAP).group(StatusAdjustment::getFrom, g -> g.with(STATUS).buildReq(IMatch::getAsString)).with(GAP).group(g -> g.alt(pattern().text("to").build(), pattern().text("->").build(), pattern().text("→").build())).opt().with(GAP) + .group(StatusAdjustment::getTo, g -> g.with(STATUS).buildReq(IMatch::getAsString)).with(GAP).group(g -> g.text("at").with(GAP)).opt().group(StatusAdjustment::getWhen, g -> g.with(LOCALDATETIME).buildReq(match -> LocalDateTime.parse(match.getAsString(), Billing.DATETIME_FORMAT))).buildReq(match -> new StatusAdjustment(match.getAsObject(StatusAdjustment::getWhen), match.getAsObject(StatusAdjustment::getFrom), match.getAsObject(StatusAdjustment::getTo))); + + public static List parse(String text) { + if ((text == null) || text.isBlank()) return HCollection.emptyList(); + + final String[] lines = text.split("\n"); + final List retVal = new ArrayList<>(); + for (String line : lines) { + final IOptional match = ADJUSTMENT.match(line); + if (match.isNotEmpty()) retVal.add(match.get()); + } + return retVal; + } + + protected static IMatcherBuilder pattern() { + return RegexMatcher.builder(Flag.CASE_INSENSITIVE); + } + + protected final LocalDateTime when; + + protected final String from; + + protected final String to; +} \ No newline at end of file diff --git a/pj-report/src/test/java/com/g2forge/project/report/TestChange.java b/pj-report/src/test/java/com/g2forge/project/report/TestChange.java index 8c68cef..eb77afc 100644 --- a/pj-report/src/test/java/com/g2forge/project/report/TestChange.java +++ b/pj-report/src/test/java/com/g2forge/project/report/TestChange.java @@ -31,43 +31,49 @@ protected ChangelogGroup changeStatus(ZonedDateTime when, String fromStatus, Str } @Test - public void testToChangesAllAfter() { + public void toChangesAllAfter() { final List actual = Change.toChanges(HCollection.asList(changeStatus(END_PLUS20, "State", "Ignored")), START, END, "user", "Ignored", IFunction1.identity()); HAssert.assertEquals(HCollection.asList(new Change(START, "user", "State"), new Change(END, "user", "State")), actual); } @Test - public void testToChangesDoubleAfter() { + public void toChangesDoubleAfter() { final List actual = Change.toChanges(HCollection.asList(changeStatus(END_PLUS20, "State", "Ignored1"), changeStatus(END_PLUS40, "Ignored1", "Ignored2")), START, END, "user", "Ignored", IFunction1.identity()); HAssert.assertEquals(HCollection.asList(new Change(START, "user", "State"), new Change(END, "user", "State")), actual); } @Test - public void testToChangesAllBefore() { + public void toChangesAllBefore() { final List actual = Change.toChanges(HCollection.asList(changeStatus(START_MINUS20, "Ignored", "State")), START, END, "user", "State", IFunction1.identity()); HAssert.assertEquals(HCollection.asList(new Change(START, "user", "State"), new Change(END, "user", "State")), actual); } @Test - public void testToChangesEmpty() { + public void toChangesEmpty() { final List actual = Change.toChanges(HCollection.emptyList(), START, END, "user", "State", IFunction1.identity()); HAssert.assertEquals(HCollection.asList(new Change(START, "user", "State"), new Change(END, "user", "State")), actual); } @Test - public void testToChangesOne() { + public void toChangesOne() { final List actual = Change.toChanges(HCollection.asList(changeStatus(START_PLUS15, "Initial", "Final")), START, END, "user", "Final", IFunction1.identity()); HAssert.assertEquals(HCollection.asList(new Change(START, "user", "Initial"), new Change(START_PLUS15, "user", "Final"), new Change(END, "user", "Final")), actual); } @Test - public void testToChangesThree() { + public void toChangesAdjusted() { + final List actual = Change.toChanges(HCollection.asList(changeStatus(START_PLUS15, "Initial", "Final"), changeStatus(START_PLUS30, "Initial", "Final")), START, END, "user", "Final", IFunction1.identity()); + HAssert.assertEquals(HCollection.asList(new Change(START, "user", "Initial"), new Change(START_PLUS15, "user", "Final"), new Change(END, "user", "Final")), actual); + } + + @Test + public void toChangesThree() { final List actual = Change.toChanges(HCollection.asList(changeStatus(START_PLUS15, "Initial", "Middle"), changeAssignee(START_PLUS30, "user1", "user2"), changeStatus(START_PLUS45, "Middle", "Final")), START, END, "user2", "Final", IFunction1.identity()); HAssert.assertEquals(HCollection.asList(new Change(START, "user1", "Initial"), new Change(START_PLUS15, "user1", "Middle"), new Change(START_PLUS30, "user2", "Middle"), new Change(START_PLUS45, "user2", "Final"), new Change(END, "user2", "Final")), actual); } @Test - public void testToChangesTwo() { + public void toChangesTwo() { final List actual = Change.toChanges(HCollection.asList(changeStatus(START_PLUS15, "Initial", "Middle"), changeStatus(START_PLUS45, "Middle", "Final")), START, END, "user", "Final", IFunction1.identity()); HAssert.assertEquals(HCollection.asList(new Change(START, "user", "Initial"), new Change(START_PLUS15, "user", "Middle"), new Change(START_PLUS45, "user", "Final"), new Change(END, "user", "Final")), actual); } diff --git a/pj-report/src/test/java/com/g2forge/project/report/TestStatusAdjustment.java b/pj-report/src/test/java/com/g2forge/project/report/TestStatusAdjustment.java new file mode 100644 index 0000000..022abbf --- /dev/null +++ b/pj-report/src/test/java/com/g2forge/project/report/TestStatusAdjustment.java @@ -0,0 +1,35 @@ +package com.g2forge.project.report; + +import java.time.LocalDateTime; +import java.util.List; + +import org.junit.Test; + +import com.g2forge.alexandria.java.core.helpers.HCollection; +import com.g2forge.alexandria.test.HAssert; + +public class TestStatusAdjustment { + @Test + public void empty() { + HAssert.assertEquals(HCollection.emptyList(), StatusAdjustment.parse(null)); + HAssert.assertEquals(HCollection.emptyList(), StatusAdjustment.parse("")); + HAssert.assertEquals(HCollection.emptyList(), StatusAdjustment.parse(" ")); + } + + @Test + public void multiple() { + final List expected = HCollection.asList(new StatusAdjustment(LocalDateTime.of(2025, 6, 12, 13, 59), "statusA", "statusB"), new StatusAdjustment(LocalDateTime.of(2025, 6, 12, 14, 22), "statusB", "statusC")); + HAssert.assertEquals(expected, StatusAdjustment.parse("* Status adjustment: statusA -> statusB at 2025-06-12 13:59\n* Status adjustment: statusB → statusC 2025-06-12 14:22")); + } + + @Test + public void simple() { + final List expected = HCollection.asList(new StatusAdjustment(LocalDateTime.of(2025, 6, 12, 13, 59), "statusA", "statusB")); + HAssert.assertEquals(expected, StatusAdjustment.parse("Status adjustment: statusA -> statusB at 2025-06-12 13:59")); + // Many spaces are optional, matches are case insensitive, seconds are optional, to and -> are both valid + HAssert.assertEquals(expected, StatusAdjustment.parse("StatusAdjustMENT statusA to statusB at2025-06-12 13:59:00")); + // You can also use →, and both the arrow/"to" and "at" are optional + HAssert.assertEquals(expected, StatusAdjustment.parse("Status adjustment: statusA → statusB 2025-06-12 13:59")); + HAssert.assertEquals(expected, StatusAdjustment.parse("Status adjustment: statusA statusB 2025-06-12 13:59")); + } +}