feat(review): reorder single-goal details display (#229)

review.go

@@ -318,9 +318,27 @@ func formatRecentDatapoints(datapoints []Datapoint) string {
 func formatGoalDetails(goal *Goal, config *Config) string {
 	var details string
 
-	// Display title only if not empty
-	if goal.Title != "" {
-		details += fmt.Sprintf("Title:       %s\n", goal.Title)
+	// Field order follows issue #229: the goal's commitment (rate, autoratchet)
+	// first, then urgency (limsum, deadline, due time), then stakes (pledge),
+	// then reference info (title, url). Fields the issue didn't enumerate
+	// (autodata, fine print, recent datapoints) follow, preserving that order.
+
+	// Display rate (n / unit). When the current rate differs from the end
+	// rate (a non-flat road), show both so the user sees what they're held to
+	// today versus where the goal is heading.
+	if goal.Rate != nil && goal.Runits != "" {
+		rateStr := formatRate(*goal.Rate, goal.Runits, goal.Gunits)
+		if cur := goal.CurrentRate(); cur != nil && formatRateValue(*cur) != formatRateValue(*goal.Rate) {
+			rateStr = fmt.Sprintf("%s (current), %s (end)",
+				formatRate(*cur, goal.Runits, goal.Gunits),
+				formatRateValue(*goal.Rate))
+		}
+		details += fmt.Sprintf("Rate:        %s\n", rateStr)
+	}
+
+	// Display autoratchet only if set (not nil)
+	if goal.Autoratchet != nil {
+		details += fmt.Sprintf("Autoratchet: %.0f\n", *goal.Autoratchet)
 	}
 
 	// Display limsum with color coding based on urgency
@@ -343,35 +361,22 @@ func formatGoalDetails(goal *Goal, config *Config) string {
 	}
 	details += fmt.Sprintf("Pledge:      %s\n", pledgeDisplay)
 
-	// Display rate (n / unit). When the current rate differs from the end
-	// rate (a non-flat road), show both so the user sees what they're held to
-	// today versus where the goal is heading.
-	if goal.Rate != nil && goal.Runits != "" {
-		rateStr := formatRate(*goal.Rate, goal.Runits, goal.Gunits)
-		if cur := goal.CurrentRate(); cur != nil && formatRateValue(*cur) != formatRateValue(*goal.Rate) {
-			rateStr = fmt.Sprintf("%s (current), %s (end)",
-				formatRate(*cur, goal.Runits, goal.Gunits),
-				formatRateValue(*goal.Rate))
-		}
-		details += fmt.Sprintf("Rate:        %s\n", rateStr)
-	}
-
-	// Display autodata only if not empty
-	if goal.Autodata != "" {
-		details += fmt.Sprintf("Autodata:    %s\n", goal.Autodata)
-	}
-
-	// Display autoratchet only if set (not nil)
-	if goal.Autoratchet != nil {
-		details += fmt.Sprintf("Autoratchet: %.0f\n", *goal.Autoratchet)
+	// Display title only if not empty
+	if goal.Title != "" {
+		details += fmt.Sprintf("Title:       %s\n", goal.Title)
 	}
 
 	// Generate and display goal URL
 	baseURL := getBaseURL(config)
 	goalURL := fmt.Sprintf("%s/%s/%s", baseURL, url.PathEscape(config.Username), url.PathEscape(goal.Slug))
 	details += fmt.Sprintf("URL:         %s\n", goalURL)
 
-	// Display fine print if it exists (at the end)
+	// Display autodata only if not empty
+	if goal.Autodata != "" {
+		details += fmt.Sprintf("Autodata:    %s\n", goal.Autodata)
+	}
+
+	// Display fine print if it exists
 	if goal.Fineprint != "" {
 		details += fmt.Sprintf("Fine print:  %s\n", goal.Fineprint)
 	}

review_test.go

@@ -1013,3 +1013,76 @@ func TestFormatRecentDatapointsFallsBackToTimestamp(t *testing.T) {
 		t.Errorf("Expected timestamp fallback date '2021-01-01', got:\n%s", result)
 	}
 }
+
+// TestGoalDetailsFieldOrder locks in the field ordering from issue #229:
+// Rate, Autoratchet, Limsum, Deadline, Due time, Pledge, Title, URL — with the
+// fields the issue didn't enumerate following after.
+func TestGoalDetailsFieldOrder(t *testing.T) {
+	rate := 0.5
+	autoratchet := 7.0
+	goal := &Goal{
+		Slug:        "clean",
+		Title:       "#autodialMax=0.5",
+		Limsum:      "+0.07 in 6 days",
+		Pledge:      5.0,
+		Rate:        &rate,
+		Runits:      "d",
+		Gunits:      "hours",
+		Autoratchet: &autoratchet,
+		Autodata:    "ifttt",
+		Fineprint:   "be honest",
+		Losedate:    4102444800, // fixed future timestamp; only the label's position matters here
+	}
+	config := &Config{Username: "narthur"}
+
+	out := formatGoalDetails(goal, config)
+
+	// Each label must appear, and in this exact relative order.
+	want := []string{
+		"Rate:", "Autoratchet:", "Limsum:", "Deadline:", "Due time:",
+		"Pledge:", "Title:", "URL:", "Autodata:", "Fine print:",
+	}
+	prev := -1
+	for _, label := range want {
+		idx := strings.Index(out, label)
+		if idx == -1 {
+			t.Fatalf("output missing %q\n%s", label, out)
+		}
+		if idx < prev {
+			t.Errorf("%q appears out of order (want sequence %v)\n%s", label, want, out)
+		}
+		prev = idx
+	}
+}
+
+// TestGoalDetailsFieldOrderMinimal verifies the #229 order still holds — and
+// the conditional fields are omitted — when Rate, Autoratchet, Title, Autodata,
+// and Fine print are all unset.
+func TestGoalDetailsFieldOrderMinimal(t *testing.T) {
+	goal := &Goal{
+		Slug:     "spark",
+		Limsum:   "+1 in 3 days",
+		Pledge:   5.0,
+		Losedate: 4102444800, // fixed future timestamp; only label positions matter
+		// Rate nil, Autoratchet nil, Title/Autodata/Fineprint empty → all omitted.
+	}
+	out := formatGoalDetails(goal, &Config{Username: "narthur"})
+
+	for _, absent := range []string{"Rate:", "Autoratchet:", "Title:", "Autodata:", "Fine print:"} {
+		if strings.Contains(out, absent) {
+			t.Errorf("expected %q to be omitted when unset\n%s", absent, out)
+		}
+	}
+	// The always-present fields keep their #229 relative order.
+	prev := -1
+	for _, label := range []string{"Limsum:", "Deadline:", "Due time:", "Pledge:", "URL:"} {
+		idx := strings.Index(out, label)
+		if idx == -1 {
+			t.Fatalf("output missing %q\n%s", label, out)
+		}
+		if idx < prev {
+			t.Errorf("%q appears out of order\n%s", label, out)
+		}
+		prev = idx
+	}
+}