fix time entry rate override behavior and add tests

3 files changed, 269 insertions, 18 deletions

diff --git a/internal/commands/billable_rate_test.go b/internal/commands/billable_rate_test.go
index 184d51f..335eb07 100644
--- a/internal/commands/billable_rate_test.go
+++ b/internal/commands/billable_rate_test.go
@@ -27,19 +27,19 @@ func TestTimeEntryBillableRateCoalescing(t *testing.T) {
 			expectError:  false,
 		},
 		{
-			name:         "only client rate - should use client rate",
+			name:         "only client rate - should be NULL (no explicit override)",
 			clientRate:   func() *int64 { f := int64(10000); return &f }(), // $100.00
 			projectRate:  nil,
 			explicitRate: nil,
-			expectedRate: func() *int64 { f := int64(10000); return &f }(),
+			expectedRate: nil, // NULL because no explicit rate was provided
 			expectError:  false,
 		},
 		{
-			name:         "client and project rates - should use project rate",
+			name:         "client and project rates - should be NULL (no explicit override)",
 			clientRate:   func() *int64 { f := int64(10000); return &f }(), // $100.00
 			projectRate:  func() *int64 { f := int64(15000); return &f }(), // $150.00
 			explicitRate: nil,
-			expectedRate: func() *int64 { f := int64(15000); return &f }(),
+			expectedRate: nil, // NULL because no explicit rate was provided
 			expectError:  false,
 		},
 		{
@@ -59,11 +59,11 @@ func TestTimeEntryBillableRateCoalescing(t *testing.T) {
 			expectError:  false,
 		},
 		{
-			name:         "only project rate with no client rate - should use project rate",
+			name:         "only project rate with no client rate - should be NULL (no explicit override)",
 			clientRate:   nil,
 			projectRate:  func() *int64 { f := int64(12500); return &f }(), // $125.00
 			explicitRate: nil,
-			expectedRate: func() *int64 { f := int64(12500); return &f }(),
+			expectedRate: nil, // NULL because no explicit rate was provided
 			expectError:  false,
 		},
 	}
@@ -185,9 +185,9 @@ func TestTimeEntryWithTimesCoalescing(t *testing.T) {
 		t.Fatalf("Failed to create time entry: %v", err)
 	}
 
-	// Should use project rate (15000 cents = $150.00)
-	if !timeEntry.BillableRate.Valid || timeEntry.BillableRate.Int64 != 15000 {
-		t.Errorf("Expected billable_rate 15000, got %v", timeEntry.BillableRate)
+	// Should be NULL since no explicit rate was provided (even though client/project have rates)
+	if timeEntry.BillableRate.Valid {
+		t.Errorf("Expected NULL billable_rate (no explicit override), got %d", timeEntry.BillableRate.Int64)
 	}
 }
 
@@ -217,8 +217,8 @@ func TestTimeEntryCoalescingWithoutProject(t *testing.T) {
 		t.Fatalf("Failed to create time entry: %v", err)
 	}
 
-	// Should use client rate (7550 cents = $75.50)
-	if !timeEntry.BillableRate.Valid || timeEntry.BillableRate.Int64 != 7550 {
-		t.Errorf("Expected billable_rate 7550, got %v", timeEntry.BillableRate)
+	// Should be NULL since no explicit rate was provided (even though client has a rate)
+	if timeEntry.BillableRate.Valid {
+		t.Errorf("Expected NULL billable_rate (no explicit override), got %d", timeEntry.BillableRate.Int64)
 	}
 }
diff --git a/internal/commands/in_test.go b/internal/commands/in_test.go
index 884a459..eac70a2 100644
--- a/internal/commands/in_test.go
+++ b/internal/commands/in_test.go
@@ -384,7 +384,7 @@ func TestInCommandBillableRateStorage(t *testing.T) {
 			expectedRate: func() *int64 { f := int64(20050); return &f }(), // $200.50
 		},
 		{
-			name: "no explicit rate uses project rate",
+			name: "no explicit rate results in NULL (even with project rate)",
 			setupData: func(q *queries.Queries) (int64, int64) {
 				client, _ := q.CreateClient(context.Background(), queries.CreateClientParams{
 					Name:         "TestClient",
@@ -398,11 +398,11 @@ func TestInCommandBillableRateStorage(t *testing.T) {
 				})
 				return client.ID, project.ID
 			},
-			args:         []string{"in", "-c", "TestClient", "-p", "ProjectWithRate"},
-			expectedRate: func() *int64 { f := int64(12500); return &f }(), // $125.00
+			args:              []string{"in", "-c", "TestClient", "-p", "ProjectWithRate"},
+			expectExplicitNil: true, // NULL because no explicit rate override
 		},
 		{
-			name: "no explicit rate and no project uses client rate",
+			name: "no explicit rate results in NULL (even with client rate)",
 			setupData: func(q *queries.Queries) (int64, int64) {
 				client, _ := q.CreateClient(context.Background(), queries.CreateClientParams{
 					Name:         "ClientOnly",
@@ -411,8 +411,8 @@ func TestInCommandBillableRateStorage(t *testing.T) {
 				})
 				return client.ID, 0
 			},
-			args:         []string{"in", "-c", "ClientOnly"},
-			expectedRate: func() *int64 { f := int64(9000); return &f }(), // $90.00
+			args:              []string{"in", "-c", "ClientOnly"},
+			expectExplicitNil: true, // NULL because no explicit rate override
 		},
 		{
 			name: "no rates anywhere results in NULL",
diff --git a/internal/commands/report_coalescing_test.go b/internal/commands/report_coalescing_test.go
new file mode 100644
index 0000000..8f4d221
--- /dev/null
+++ b/internal/commands/report_coalescing_test.go
@@ -0,0 +1,251 @@
+package commands
+
+import (
+	"context"
+	"database/sql"
+	"testing"
+	"time"
+
+	"punchcard/internal/queries"
+)
+
+func TestReportRateCoalescing(t *testing.T) {
+	tests := []struct {
+		name               string
+		clientRate         *int64 // nil means NULL, values in cents
+		projectRate        *int64 // nil means NULL, values in cents
+		entryRate          *int64 // nil means NULL, values in cents
+		expectedRateSource string
+	}{
+		{
+			name:               "no rates anywhere - should use client source",
+			clientRate:         nil,
+			projectRate:        nil,
+			entryRate:          nil,
+			expectedRateSource: "client",
+		},
+		{
+			name:               "only client rate - should use client source",
+			clientRate:         func() *int64 { f := int64(10000); return &f }(), // $100.00
+			projectRate:        nil,
+			entryRate:          nil,
+			expectedRateSource: "client",
+		},
+		{
+			name:               "client and project rates - should use project source",
+			clientRate:         func() *int64 { f := int64(10000); return &f }(), // $100.00
+			projectRate:        func() *int64 { f := int64(15000); return &f }(), // $150.00
+			entryRate:          nil,
+			expectedRateSource: "project",
+		},
+		{
+			name:               "all rates provided - should use entry source",
+			clientRate:         func() *int64 { f := int64(10000); return &f }(), // $100.00
+			projectRate:        func() *int64 { f := int64(15000); return &f }(), // $150.00
+			entryRate:          func() *int64 { f := int64(20000); return &f }(), // $200.00
+			expectedRateSource: "entry",
+		},
+		{
+			name:               "entry rate overrides even when zero",
+			clientRate:         func() *int64 { f := int64(10000); return &f }(), // $100.00
+			projectRate:        func() *int64 { f := int64(15000); return &f }(), // $150.00
+			entryRate:          func() *int64 { f := int64(0); return &f }(),     // $0.00
+			expectedRateSource: "entry",
+		},
+		{
+			name:               "only project rate with no client rate - should use project source",
+			clientRate:         nil,
+			projectRate:        func() *int64 { f := int64(12500); return &f }(), // $125.00
+			entryRate:          nil,
+			expectedRateSource: "project",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Setup fresh database for each test
+			q, cleanup := setupTestDB(t)
+			defer cleanup()
+
+			// Create client with optional billable rate
+			var clientBillableRate sql.NullInt64
+			if tt.clientRate != nil {
+				clientBillableRate = sql.NullInt64{Int64: *tt.clientRate, Valid: true}
+			}
+
+			client, err := q.CreateClient(context.Background(), queries.CreateClientParams{
+				Name:         "TestClient",
+				Email:        sql.NullString{},
+				BillableRate: clientBillableRate,
+			})
+			if err != nil {
+				t.Fatalf("Failed to create client: %v", err)
+			}
+
+			// Create project with optional billable rate
+			var projectBillableRate sql.NullInt64
+			if tt.projectRate != nil {
+				projectBillableRate = sql.NullInt64{Int64: *tt.projectRate, Valid: true}
+			}
+
+			project, err := q.CreateProject(context.Background(), queries.CreateProjectParams{
+				Name:         "TestProject",
+				ClientID:     client.ID,
+				BillableRate: projectBillableRate,
+			})
+			if err != nil {
+				t.Fatalf("Failed to create project: %v", err)
+			}
+
+			// Create time entry with optional explicit billable rate
+			var entryBillableRate sql.NullInt64
+			if tt.entryRate != nil {
+				entryBillableRate = sql.NullInt64{Int64: *tt.entryRate, Valid: true}
+			}
+
+			// Create time entry with explicit start and end times
+			startTime := time.Now().UTC()
+			endTime := startTime.Add(2 * time.Hour) // 2 hours of work
+
+			_, err = q.CreateTimeEntryWithTimes(context.Background(), queries.CreateTimeEntryWithTimesParams{
+				StartTime:    startTime.Format(time.DateTime),
+				EndTime:      endTime.Format(time.DateTime),
+				Description:  sql.NullString{String: "Test work", Valid: true},
+				ClientID:     client.ID,
+				ProjectID:    sql.NullInt64{Int64: project.ID, Valid: true},
+				BillableRate: entryBillableRate,
+			})
+			if err != nil {
+				t.Fatalf("Failed to create time entry: %v", err)
+			}
+
+			// Test GetInvoiceDataByClient query (report coalescing logic)
+			clientData, err := q.GetInvoiceDataByClient(context.Background(), queries.GetInvoiceDataByClientParams{
+				ClientID:  client.ID,
+				StartTime: startTime.Add(-time.Hour),
+				EndTime:   endTime.Add(time.Hour),
+			})
+			if err != nil {
+				t.Fatalf("Failed to get invoice data by client: %v", err)
+			}
+
+			if len(clientData) != 1 {
+				t.Fatalf("Expected 1 client data entry, got %d", len(clientData))
+			}
+
+			entry := clientData[0]
+			if entry.RateSource != tt.expectedRateSource {
+				t.Errorf("Expected rate source %s, got %s", tt.expectedRateSource, entry.RateSource)
+			}
+
+			// Test GetInvoiceDataByProject query (report coalescing logic)
+			projectData, err := q.GetInvoiceDataByProject(context.Background(), queries.GetInvoiceDataByProjectParams{
+				ProjectID: project.ID,
+				StartTime: startTime.Add(-time.Hour),
+				EndTime:   endTime.Add(time.Hour),
+			})
+			if err != nil {
+				t.Fatalf("Failed to get invoice data by project: %v", err)
+			}
+
+			if len(projectData) != 1 {
+				t.Fatalf("Expected 1 project data entry, got %d", len(projectData))
+			}
+
+			projectEntry := projectData[0]
+			if projectEntry.RateSource != tt.expectedRateSource {
+				t.Errorf("Expected rate source %s, got %s (project query)", tt.expectedRateSource, projectEntry.RateSource)
+			}
+
+			// Verify that the appropriate rate values are present
+			switch tt.expectedRateSource {
+			case "entry":
+				if !entry.EntryBillableRate.Valid {
+					t.Errorf("Expected entry billable rate to be valid when rate source is 'entry'")
+				} else if tt.entryRate != nil && entry.EntryBillableRate.Int64 != *tt.entryRate {
+					t.Errorf("Expected entry billable rate %d, got %d", *tt.entryRate, entry.EntryBillableRate.Int64)
+				}
+			case "project":
+				if !entry.ProjectBillableRate.Valid {
+					t.Errorf("Expected project billable rate to be valid when rate source is 'project'")
+				} else if tt.projectRate != nil && entry.ProjectBillableRate.Int64 != *tt.projectRate {
+					t.Errorf("Expected project billable rate %d, got %d", *tt.projectRate, entry.ProjectBillableRate.Int64)
+				}
+			case "client":
+				// Client rate can be NULL and still be the source (fallback case)
+				if tt.clientRate != nil {
+					if !entry.ClientBillableRate.Valid {
+						t.Errorf("Expected client billable rate to be valid when rate source is 'client' and client has rate")
+					} else if entry.ClientBillableRate.Int64 != *tt.clientRate {
+						t.Errorf("Expected client billable rate %d, got %d", *tt.clientRate, entry.ClientBillableRate.Int64)
+					}
+				}
+			}
+		})
+	}
+}
+
+func TestReportRateCoalescingWithoutProject(t *testing.T) {
+	// Test coalescing when no project is specified
+	q, cleanup := setupTestDB(t)
+	defer cleanup()
+
+	// Create client with rate $75.50 (7550 cents)
+	client, err := q.CreateClient(context.Background(), queries.CreateClientParams{
+		Name:         "NoProjectClient",
+		Email:        sql.NullString{},
+		BillableRate: sql.NullInt64{Int64: 7550, Valid: true},
+	})
+	if err != nil {
+		t.Fatalf("Failed to create client: %v", err)
+	}
+
+	// Create time entry without project - should use client rate
+	startTime := time.Now().UTC()
+	endTime := startTime.Add(time.Hour)
+
+	_, err = q.CreateTimeEntryWithTimes(context.Background(), queries.CreateTimeEntryWithTimesParams{
+		StartTime:   startTime.Format(time.DateTime),
+		EndTime:     endTime.Format(time.DateTime),
+		Description: sql.NullString{String: "Client work", Valid: true},
+		ClientID:    client.ID,
+		ProjectID:   sql.NullInt64{}, // No project
+		BillableRate: sql.NullInt64{}, // No explicit rate
+	})
+	if err != nil {
+		t.Fatalf("Failed to create time entry: %v", err)
+	}
+
+	// Test GetInvoiceDataByClient query
+	clientData, err := q.GetInvoiceDataByClient(context.Background(), queries.GetInvoiceDataByClientParams{
+		ClientID:  client.ID,
+		StartTime: startTime.Add(-time.Hour),
+		EndTime:   endTime.Add(time.Hour),
+	})
+	if err != nil {
+		t.Fatalf("Failed to get invoice data by client: %v", err)
+	}
+
+	if len(clientData) != 1 {
+		t.Fatalf("Expected 1 client data entry, got %d", len(clientData))
+	}
+
+	entry := clientData[0]
+	if entry.RateSource != "client" {
+		t.Errorf("Expected rate source 'client', got %s", entry.RateSource)
+	}
+
+	if !entry.ClientBillableRate.Valid || entry.ClientBillableRate.Int64 != 7550 {
+		t.Errorf("Expected client billable rate 7550, got %v", entry.ClientBillableRate)
+	}
+
+	// Project rate should be NULL since no project
+	if entry.ProjectBillableRate.Valid {
+		t.Errorf("Expected project billable rate to be NULL when no project, got %d", entry.ProjectBillableRate.Int64)
+	}
+
+	// Entry rate should be NULL since no explicit override
+	if entry.EntryBillableRate.Valid {
+		t.Errorf("Expected entry billable rate to be NULL when no explicit override, got %d", entry.EntryBillableRate.Int64)
+	}
+}
\ No newline at end of file