Skip to content
This repository was archived by the owner on Mar 12, 2026. It is now read-only.

Commit 9cca2b0

Browse files
briglebclaude
brigleb
and
claude
committed
refactor: improve timesheet command conventions and add tests
Use modular API interface (client.Timesheets(), client.People()) instead of calling methods directly on the client. Reuse existing api.Bucket type, remove unused models/timesheet.go, add timesheet methods to APIClient interface and mock, fix silent parseSince error propagation, deduplicate table rendering, and sort grouped output by total hours descending. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a98fd52 commit 9cca2b0

7 files changed

Lines changed: 342 additions & 139 deletions

File tree

‎cmd/timesheet/list.go‎

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,16 @@ func newListCmd(f *factory.Factory) *cobra.Command {
6363
f = f.WithProject(projectID)
6464
}
6565

66+
// Parse --since before fetching so invalid input fails early
67+
var sinceDate time.Time
68+
if sinceStr != "" {
69+
var err error
70+
sinceDate, err = parseSince(sinceStr)
71+
if err != nil {
72+
return fmt.Errorf("invalid --since value: %w", err)
73+
}
74+
}
75+
6676
// Get API client from factory
6777
client, err := f.ApiClient()
6878
if err != nil {
@@ -79,25 +89,25 @@ func newListCmd(f *factory.Factory) *cobra.Command {
7989
var entries []api.TimesheetEntry
8090
if recordingID > 0 {
8191
// Get entries for a specific recording
82-
entries, err = client.GetRecordingTimesheet(cmd.Context(), resolvedProjectID, recordingID)
92+
entries, err = client.Timesheets().GetRecordingTimesheet(cmd.Context(), resolvedProjectID, recordingID)
8393
} else {
8494
// Get entries for the project
85-
entries, err = client.GetProjectTimesheet(cmd.Context(), resolvedProjectID)
95+
entries, err = client.Timesheets().GetProjectTimesheet(cmd.Context(), resolvedProjectID)
8696
}
8797
if err != nil {
8898
return err
8999
}
90100

91101
// Apply filters
92-
entries = filterEntries(entries, personStr, sinceStr)
102+
entries = filterEntries(entries, personStr, sinceDate)
93103

94104
// Output format
95105
if formatStr == "json" {
96106
return outputJSON(entries)
97107
}
98108

99109
// Display as table
100-
return displayTable(entries)
110+
return renderEntryTable(entries, false)
101111
},
102112
}
103113

@@ -112,18 +122,9 @@ func newListCmd(f *factory.Factory) *cobra.Command {
112122
}
113123

114124
// filterEntries applies person and date filters
115-
func filterEntries(entries []api.TimesheetEntry, personStr, sinceStr string) []api.TimesheetEntry {
125+
func filterEntries(entries []api.TimesheetEntry, personStr string, sinceDate time.Time) []api.TimesheetEntry {
116126
var filtered []api.TimesheetEntry
117127

118-
// Parse since filter
119-
var sinceDate time.Time
120-
if sinceStr != "" {
121-
since, err := parseSince(sinceStr)
122-
if err == nil {
123-
sinceDate = since
124-
}
125-
}
126-
127128
for _, entry := range entries {
128129
// Filter by person
129130
if personStr != "" {
@@ -146,7 +147,7 @@ func filterEntries(entries []api.TimesheetEntry, personStr, sinceStr string) []a
146147
return filtered
147148
}
148149

149-
// parseSince parses a "since" string (e.g., "7d", "2024-01-01")
150+
// parseSince parses a "since" string (e.g., "7d", "24h", "2024-01-01")
150151
func parseSince(since string) (time.Time, error) {
151152
// Try parsing as duration (e.g., "7d", "24h")
152153
if strings.HasSuffix(since, "d") {
@@ -167,7 +168,7 @@ func parseSince(since string) (time.Time, error) {
167168
// Try parsing as ISO date
168169
t, err := time.Parse("2006-01-02", since)
169170
if err != nil {
170-
return time.Time{}, fmt.Errorf("invalid date format: %s", since)
171+
return time.Time{}, fmt.Errorf("invalid date format: %s (expected Nd, Nh, or YYYY-MM-DD)", since)
171172
}
172173
return t, nil
173174
}
@@ -179,17 +180,15 @@ func outputJSON(entries []api.TimesheetEntry) error {
179180
return enc.Encode(entries)
180181
}
181182

182-
// displayTable displays entries in a table format
183-
func displayTable(entries []api.TimesheetEntry) error {
183+
// renderEntryTable displays entries in a table format, optionally with a total summary line
184+
func renderEntryTable(entries []api.TimesheetEntry, showTotal bool) error {
184185
if len(entries) == 0 {
185186
fmt.Println("No timesheet entries found")
186187
return nil
187188
}
188189

189-
// Create table
190190
tp := tableprinter.New(os.Stdout)
191191

192-
// Add headers
193192
tp.AddField("DATE")
194193
tp.AddField("HOURS")
195194
tp.AddField("PERSON")
@@ -198,22 +197,31 @@ func displayTable(entries []api.TimesheetEntry) error {
198197
tp.AddField("DESCRIPTION")
199198
tp.EndRow()
200199

201-
// Add rows
200+
var totalHours float64
202201
for _, entry := range entries {
203202
tp.AddField(entry.Date)
204203
tp.AddField(fmt.Sprintf("%.2f", entry.Hours))
205204
tp.AddField(entry.Creator.Name)
206205
tp.AddField(entry.Bucket.Name)
207206
tp.AddField(entry.Parent.Title)
208207

209-
// Truncate description if too long
210208
desc := entry.Description
211209
if len(desc) > 50 {
212210
desc = desc[:47] + "..."
213211
}
214212
tp.AddField(desc)
215213
tp.EndRow()
214+
215+
totalHours += entry.Hours
216+
}
217+
218+
if err := tp.Render(); err != nil {
219+
return err
220+
}
221+
222+
if showTotal {
223+
fmt.Printf("\nTotal: %d entries, %.2f hours\n", len(entries), totalHours)
216224
}
217225

218-
return tp.Render()
226+
return nil
219227
}

‎cmd/timesheet/report.go‎

Lines changed: 43 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package timesheet
33
import (
44
"fmt"
55
"os"
6+
"sort"
67
"strconv"
78
"strings"
89
"time"
@@ -75,7 +76,7 @@ Note: Without date filters, the API returns entries from the last month by defau
7576
// Parse person filter
7677
if personStr != "" {
7778
// We need to convert name to ID - fetch all people first
78-
people, err := client.GetAllPeople(cmd.Context())
79+
people, err := client.People().GetAllPeople(cmd.Context())
7980
if err != nil {
8081
return fmt.Errorf("failed to fetch people: %w", err)
8182
}
@@ -99,7 +100,7 @@ Note: Without date filters, the API returns entries from the last month by defau
99100
}
100101

101102
// Fetch report
102-
entries, err := client.GetTimesheetReport(cmd.Context(), opts)
103+
entries, err := client.Timesheets().GetTimesheetReport(cmd.Context(), opts)
103104
if err != nil {
104105
return err
105106
}
@@ -116,7 +117,7 @@ Note: Without date filters, the API returns entries from the last month by defau
116117
case "project":
117118
return displayGroupedByProject(entries)
118119
default:
119-
return displayReportTable(entries)
120+
return renderEntryTable(entries, true)
120121
}
121122
},
122123
}
@@ -132,55 +133,11 @@ Note: Without date filters, the API returns entries from the last month by defau
132133
return cmd
133134
}
134135

135-
// displayReportTable displays the report as a flat table
136-
func displayReportTable(entries []api.TimesheetEntry) error {
137-
if len(entries) == 0 {
138-
fmt.Println("No timesheet entries found")
139-
return nil
140-
}
141-
142-
// Create table
143-
tp := tableprinter.New(os.Stdout)
144-
145-
// Add headers
146-
tp.AddField("DATE")
147-
tp.AddField("HOURS")
148-
tp.AddField("PERSON")
149-
tp.AddField("PROJECT")
150-
tp.AddField("PARENT")
151-
tp.AddField("DESCRIPTION")
152-
tp.EndRow()
153-
154-
// Calculate total hours
155-
var totalHours float64
156-
157-
// Add rows
158-
for _, entry := range entries {
159-
tp.AddField(entry.Date)
160-
tp.AddField(fmt.Sprintf("%.2f", entry.Hours))
161-
tp.AddField(entry.Creator.Name)
162-
tp.AddField(entry.Bucket.Name)
163-
tp.AddField(entry.Parent.Title)
164-
165-
// Truncate description if too long
166-
desc := entry.Description
167-
if len(desc) > 50 {
168-
desc = desc[:47] + "..."
169-
}
170-
tp.AddField(desc)
171-
tp.EndRow()
172-
173-
totalHours += entry.Hours
174-
}
175-
176-
if err := tp.Render(); err != nil {
177-
return err
178-
}
179-
180-
// Print summary
181-
fmt.Printf("\nTotal: %d entries, %.2f hours\n", len(entries), totalHours)
182-
183-
return nil
136+
type groupEntry struct {
137+
key string
138+
name string
139+
hours float64
140+
count int
184141
}
185142

186143
// displayGroupedByPerson groups entries by person and shows totals
@@ -191,21 +148,27 @@ func displayGroupedByPerson(entries []api.TimesheetEntry) error {
191148
}
192149

193150
// Group by person
194-
grouped := make(map[string]struct {
195-
name string
196-
hours float64
197-
count int
198-
})
199-
151+
grouped := make(map[string]*groupEntry)
200152
for _, entry := range entries {
201153
key := strconv.FormatInt(entry.Creator.ID, 10)
202-
g := grouped[key]
203-
g.name = entry.Creator.Name
154+
g, ok := grouped[key]
155+
if !ok {
156+
g = &groupEntry{key: key, name: entry.Creator.Name}
157+
grouped[key] = g
158+
}
204159
g.hours += entry.Hours
205160
g.count++
206-
grouped[key] = g
207161
}
208162

163+
// Sort by total hours descending
164+
sorted := make([]*groupEntry, 0, len(grouped))
165+
for _, g := range grouped {
166+
sorted = append(sorted, g)
167+
}
168+
sort.Slice(sorted, func(i, j int) bool {
169+
return sorted[i].hours > sorted[j].hours
170+
})
171+
209172
// Create table
210173
tp := tableprinter.New(os.Stdout)
211174
tp.AddField("PERSON")
@@ -217,7 +180,7 @@ func displayGroupedByPerson(entries []api.TimesheetEntry) error {
217180
var totalHours float64
218181
var totalCount int
219182

220-
for _, g := range grouped {
183+
for _, g := range sorted {
221184
tp.AddField(g.name)
222185
tp.AddField(fmt.Sprintf("%d", g.count))
223186
tp.AddField(fmt.Sprintf("%.2f", g.hours))
@@ -232,7 +195,7 @@ func displayGroupedByPerson(entries []api.TimesheetEntry) error {
232195
return err
233196
}
234197

235-
fmt.Printf("\nTotal: %d entries, %.2f hours across %d people\n", totalCount, totalHours, len(grouped))
198+
fmt.Printf("\nTotal: %d entries, %.2f hours across %d people\n", totalCount, totalHours, len(sorted))
236199

237200
return nil
238201
}
@@ -245,21 +208,27 @@ func displayGroupedByProject(entries []api.TimesheetEntry) error {
245208
}
246209

247210
// Group by project
248-
grouped := make(map[string]struct {
249-
name string
250-
hours float64
251-
count int
252-
})
253-
211+
grouped := make(map[string]*groupEntry)
254212
for _, entry := range entries {
255213
key := strconv.FormatInt(entry.Bucket.ID, 10)
256-
g := grouped[key]
257-
g.name = entry.Bucket.Name
214+
g, ok := grouped[key]
215+
if !ok {
216+
g = &groupEntry{key: key, name: entry.Bucket.Name}
217+
grouped[key] = g
218+
}
258219
g.hours += entry.Hours
259220
g.count++
260-
grouped[key] = g
261221
}
262222

223+
// Sort by total hours descending
224+
sorted := make([]*groupEntry, 0, len(grouped))
225+
for _, g := range grouped {
226+
sorted = append(sorted, g)
227+
}
228+
sort.Slice(sorted, func(i, j int) bool {
229+
return sorted[i].hours > sorted[j].hours
230+
})
231+
263232
// Create table
264233
tp := tableprinter.New(os.Stdout)
265234
tp.AddField("PROJECT")
@@ -271,7 +240,7 @@ func displayGroupedByProject(entries []api.TimesheetEntry) error {
271240
var totalHours float64
272241
var totalCount int
273242

274-
for _, g := range grouped {
243+
for _, g := range sorted {
275244
tp.AddField(g.name)
276245
tp.AddField(fmt.Sprintf("%d", g.count))
277246
tp.AddField(fmt.Sprintf("%.2f", g.hours))
@@ -286,7 +255,7 @@ func displayGroupedByProject(entries []api.TimesheetEntry) error {
286255
return err
287256
}
288257

289-
fmt.Printf("\nTotal: %d entries, %.2f hours across %d projects\n", totalCount, totalHours, len(grouped))
258+
fmt.Printf("\nTotal: %d entries, %.2f hours across %d projects\n", totalCount, totalHours, len(sorted))
290259

291260
return nil
292261
}

0 commit comments

Comments
 (0)