diff --git a/.surface b/.surface index 7931f86a4..a8ee1dcbd 100644 --- a/.surface +++ b/.surface @@ -410,6 +410,10 @@ ARG basecamp templates delete 00 ARG basecamp templates show 00 ARG basecamp templates update 00 ARG basecamp timeline 00 [me] +ARG basecamp timesheet entry create 00 +ARG basecamp timesheet entry delete 00 +ARG basecamp timesheet entry show 00 +ARG basecamp timesheet entry update 00 ARG basecamp timesheet item 00 ARG basecamp timesheet recording 00 ARG basecamp tlgroup copy 00 @@ -1169,6 +1173,11 @@ CMD basecamp templates show CMD basecamp templates update CMD basecamp timeline CMD basecamp timesheet +CMD basecamp timesheet entry +CMD basecamp timesheet entry create +CMD basecamp timesheet entry delete +CMD basecamp timesheet entry show +CMD basecamp timesheet entry update CMD basecamp timesheet item CMD basecamp timesheet project CMD basecamp timesheet recording @@ -15734,6 +15743,143 @@ FLAG basecamp timesheet --styled type=bool FLAG basecamp timesheet --to type=string FLAG basecamp timesheet --todolist type=string FLAG basecamp timesheet --verbose type=count +FLAG basecamp timesheet entry --account type=string +FLAG basecamp timesheet entry --agent type=bool +FLAG basecamp timesheet entry --cache-dir type=string +FLAG basecamp timesheet entry --count type=bool +FLAG basecamp timesheet entry --end type=string +FLAG basecamp timesheet entry --from type=string +FLAG basecamp timesheet entry --help type=bool +FLAG basecamp timesheet entry --hints type=bool +FLAG basecamp timesheet entry --ids-only type=bool +FLAG basecamp timesheet entry --in type=string +FLAG basecamp timesheet entry --jq type=string +FLAG basecamp timesheet entry --json type=bool +FLAG basecamp timesheet entry --markdown type=bool +FLAG basecamp timesheet entry --md type=bool +FLAG basecamp timesheet entry --no-hints type=bool +FLAG basecamp timesheet entry --no-stats type=bool +FLAG basecamp timesheet entry --person type=string +FLAG basecamp timesheet entry --profile type=string +FLAG basecamp timesheet entry --project type=string +FLAG basecamp timesheet entry --quiet type=bool +FLAG basecamp timesheet entry --start type=string +FLAG basecamp timesheet entry --stats type=bool +FLAG basecamp timesheet entry --styled type=bool +FLAG basecamp timesheet entry --to type=string +FLAG basecamp timesheet entry --todolist type=string +FLAG basecamp timesheet entry --verbose type=count +FLAG basecamp timesheet entry create --account type=string +FLAG basecamp timesheet entry create --agent type=bool +FLAG basecamp timesheet entry create --cache-dir type=string +FLAG basecamp timesheet entry create --count type=bool +FLAG basecamp timesheet entry create --date type=string +FLAG basecamp timesheet entry create --description type=string +FLAG basecamp timesheet entry create --end type=string +FLAG basecamp timesheet entry create --from type=string +FLAG basecamp timesheet entry create --help type=bool +FLAG basecamp timesheet entry create --hints type=bool +FLAG basecamp timesheet entry create --hours type=string +FLAG basecamp timesheet entry create --ids-only type=bool +FLAG basecamp timesheet entry create --in type=string +FLAG basecamp timesheet entry create --jq type=string +FLAG basecamp timesheet entry create --json type=bool +FLAG basecamp timesheet entry create --markdown type=bool +FLAG basecamp timesheet entry create --md type=bool +FLAG basecamp timesheet entry create --no-hints type=bool +FLAG basecamp timesheet entry create --no-stats type=bool +FLAG basecamp timesheet entry create --person type=string +FLAG basecamp timesheet entry create --profile type=string +FLAG basecamp timesheet entry create --project type=string +FLAG basecamp timesheet entry create --quiet type=bool +FLAG basecamp timesheet entry create --start type=string +FLAG basecamp timesheet entry create --stats type=bool +FLAG basecamp timesheet entry create --styled type=bool +FLAG basecamp timesheet entry create --to type=string +FLAG basecamp timesheet entry create --todolist type=string +FLAG basecamp timesheet entry create --verbose type=count +FLAG basecamp timesheet entry delete --account type=string +FLAG basecamp timesheet entry delete --agent type=bool +FLAG basecamp timesheet entry delete --cache-dir type=string +FLAG basecamp timesheet entry delete --count type=bool +FLAG basecamp timesheet entry delete --end type=string +FLAG basecamp timesheet entry delete --force type=bool +FLAG basecamp timesheet entry delete --from type=string +FLAG basecamp timesheet entry delete --help type=bool +FLAG basecamp timesheet entry delete --hints type=bool +FLAG basecamp timesheet entry delete --ids-only type=bool +FLAG basecamp timesheet entry delete --in type=string +FLAG basecamp timesheet entry delete --jq type=string +FLAG basecamp timesheet entry delete --json type=bool +FLAG basecamp timesheet entry delete --markdown type=bool +FLAG basecamp timesheet entry delete --md type=bool +FLAG basecamp timesheet entry delete --no-hints type=bool +FLAG basecamp timesheet entry delete --no-stats type=bool +FLAG basecamp timesheet entry delete --person type=string +FLAG basecamp timesheet entry delete --profile type=string +FLAG basecamp timesheet entry delete --project type=string +FLAG basecamp timesheet entry delete --quiet type=bool +FLAG basecamp timesheet entry delete --start type=string +FLAG basecamp timesheet entry delete --stats type=bool +FLAG basecamp timesheet entry delete --styled type=bool +FLAG basecamp timesheet entry delete --to type=string +FLAG basecamp timesheet entry delete --todolist type=string +FLAG basecamp timesheet entry delete --verbose type=count +FLAG basecamp timesheet entry show --account type=string +FLAG basecamp timesheet entry show --agent type=bool +FLAG basecamp timesheet entry show --cache-dir type=string +FLAG basecamp timesheet entry show --count type=bool +FLAG basecamp timesheet entry show --end type=string +FLAG basecamp timesheet entry show --from type=string +FLAG basecamp timesheet entry show --help type=bool +FLAG basecamp timesheet entry show --hints type=bool +FLAG basecamp timesheet entry show --ids-only type=bool +FLAG basecamp timesheet entry show --in type=string +FLAG basecamp timesheet entry show --jq type=string +FLAG basecamp timesheet entry show --json type=bool +FLAG basecamp timesheet entry show --markdown type=bool +FLAG basecamp timesheet entry show --md type=bool +FLAG basecamp timesheet entry show --no-hints type=bool +FLAG basecamp timesheet entry show --no-stats type=bool +FLAG basecamp timesheet entry show --person type=string +FLAG basecamp timesheet entry show --profile type=string +FLAG basecamp timesheet entry show --project type=string +FLAG basecamp timesheet entry show --quiet type=bool +FLAG basecamp timesheet entry show --start type=string +FLAG basecamp timesheet entry show --stats type=bool +FLAG basecamp timesheet entry show --styled type=bool +FLAG basecamp timesheet entry show --to type=string +FLAG basecamp timesheet entry show --todolist type=string +FLAG basecamp timesheet entry show --verbose type=count +FLAG basecamp timesheet entry update --account type=string +FLAG basecamp timesheet entry update --agent type=bool +FLAG basecamp timesheet entry update --cache-dir type=string +FLAG basecamp timesheet entry update --count type=bool +FLAG basecamp timesheet entry update --date type=string +FLAG basecamp timesheet entry update --description type=string +FLAG basecamp timesheet entry update --end type=string +FLAG basecamp timesheet entry update --from type=string +FLAG basecamp timesheet entry update --help type=bool +FLAG basecamp timesheet entry update --hints type=bool +FLAG basecamp timesheet entry update --hours type=string +FLAG basecamp timesheet entry update --ids-only type=bool +FLAG basecamp timesheet entry update --in type=string +FLAG basecamp timesheet entry update --jq type=string +FLAG basecamp timesheet entry update --json type=bool +FLAG basecamp timesheet entry update --markdown type=bool +FLAG basecamp timesheet entry update --md type=bool +FLAG basecamp timesheet entry update --no-hints type=bool +FLAG basecamp timesheet entry update --no-stats type=bool +FLAG basecamp timesheet entry update --person type=string +FLAG basecamp timesheet entry update --profile type=string +FLAG basecamp timesheet entry update --project type=string +FLAG basecamp timesheet entry update --quiet type=bool +FLAG basecamp timesheet entry update --start type=string +FLAG basecamp timesheet entry update --stats type=bool +FLAG basecamp timesheet entry update --styled type=bool +FLAG basecamp timesheet entry update --to type=string +FLAG basecamp timesheet entry update --todolist type=string +FLAG basecamp timesheet entry update --verbose type=count FLAG basecamp timesheet item --account type=string FLAG basecamp timesheet item --agent type=bool FLAG basecamp timesheet item --cache-dir type=string @@ -21224,6 +21370,11 @@ SUB basecamp templates show SUB basecamp templates update SUB basecamp timeline SUB basecamp timesheet +SUB basecamp timesheet entry +SUB basecamp timesheet entry create +SUB basecamp timesheet entry delete +SUB basecamp timesheet entry show +SUB basecamp timesheet entry update SUB basecamp timesheet item SUB basecamp timesheet project SUB basecamp timesheet recording diff --git a/API-COVERAGE.md b/API-COVERAGE.md index c07aadd28..b3853c0f0 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -274,7 +274,7 @@ cannot faithfully cover at least one endpoint for a reason outside the CLI. A | **Templates** | | templates | 10 | `templates` | ✅ | BC4 | - | list, show, create, update, delete, construct, construction, library, copy, copy-status | | **Time Tracking** | -| timesheets | 6 | `timesheet` | ✅ | BC4 | - | list, show, create, update, delete | +| timesheets | 7 | `timesheet` | ✅ | BC4 | - | Reports: account-wide (`timesheet report`), project (`timesheet project`), item (`timesheet item`). Entries: `timesheet entry show`, `create` (on an item), `update`, `delete` (permanent: bc3 refuses to trash or archive an entry) | | **Subscriptions** | | subscriptions | 4 | `subscriptions` | ✅ | BC4 | - | show, subscribe, unsubscribe, add/remove | | **Check-ins (Automatic)** | diff --git a/e2e/smoke/.qa-allowlist b/e2e/smoke/.qa-allowlist index 952ab0c12..6fba7c5a0 100644 --- a/e2e/smoke/.qa-allowlist +++ b/e2e/smoke/.qa-allowlist @@ -19,6 +19,10 @@ test_reports_schedule_returns_schedule_entries # 400 on some environments — sm test_timesheet_report_returns_timesheet_data # timesheets not enabled — smoke test_timesheet_project_returns_project_timesheet # timesheets not enabled — smoke test_timesheet_item_returns_timesheet_item_detail # timesheets not enabled — smoke +test_timesheet_entry_create_logs_time_against_a_todo # timesheets not enabled — smoke +test_timesheet_entry_show_returns_entry_detail # depends on timesheet entry create — smoke +test_timesheet_entry_update_changes_the_hours # depends on timesheet entry create — smoke +test_timesheet_entry_delete_removes_the_entry # depends on timesheet entry create — smoke # Lineup API may not exist on all environments test_lineup_create_creates_a_lineup_marker # API not available on some servers — smoke diff --git a/e2e/smoke/run_smoke.sh b/e2e/smoke/run_smoke.sh index 99aff3a23..b8bb6962e 100755 --- a/e2e/smoke/run_smoke.sh +++ b/e2e/smoke/run_smoke.sh @@ -92,6 +92,7 @@ echo "--- Level 1: Mutation tests (parallel, $jobs jobs) ---" level1=( "$SMOKE_DIR"/smoke_todos_write.bats "$SMOKE_DIR"/smoke_subtasks.bats + "$SMOKE_DIR"/smoke_timesheet_write.bats "$SMOKE_DIR"/smoke_messages_write.bats "$SMOKE_DIR"/smoke_files_write.bats "$SMOKE_DIR"/smoke_cards_write.bats diff --git a/e2e/smoke/smoke_timesheet_write.bats b/e2e/smoke/smoke_timesheet_write.bats new file mode 100644 index 000000000..44f15d503 --- /dev/null +++ b/e2e/smoke/smoke_timesheet_write.bats @@ -0,0 +1,57 @@ +#!/usr/bin/env bats +# smoke_timesheet_write.bats - Level 1: Timesheet entry lifecycle on a to-do + +load smoke_helper + +setup_file() { + # Each test depends on the one before it: create writes the entry id the + # rest read, and delete must come last. + export BATS_NO_PARALLELIZE_WITHIN_FILE=true + ensure_token || return 1 + ensure_todo || return 1 +} + +@test "timesheet entry create logs time against a todo" { + run_smoke basecamp timesheet entry create "$QA_TODO" --hours 0.25 --description "Smoke entry $(date +%s)" --json + # Timesheets may not be enabled on all accounts (403 forbidden) + [[ "$status" -eq 4 ]] && mark_unverifiable "Timesheets not enabled in this account" + assert_success + assert_json_value '.ok' 'true' + assert_json_not_null '.data.id' + + echo "$output" | jq -r '.data.id' > "$BATS_FILE_TMPDIR/timesheet_entry_id" +} + +@test "timesheet entry show returns entry detail" { + local id_file="$BATS_FILE_TMPDIR/timesheet_entry_id" + [[ -f "$id_file" ]] || mark_unverifiable "No timesheet entry created in prior test" + local entry_id + entry_id=$(<"$id_file") + + run_smoke basecamp timesheet entry show "$entry_id" --json + assert_success + assert_json_value '.ok' 'true' + assert_json_value '.data.parent.id' "$QA_TODO" +} + +@test "timesheet entry update changes the hours" { + local id_file="$BATS_FILE_TMPDIR/timesheet_entry_id" + [[ -f "$id_file" ]] || mark_unverifiable "No timesheet entry created in prior test" + local entry_id + entry_id=$(<"$id_file") + + run_smoke basecamp timesheet entry update "$entry_id" --hours 0:30 --json + assert_success + assert_json_value '.ok' 'true' +} + +@test "timesheet entry delete removes the entry" { + local id_file="$BATS_FILE_TMPDIR/timesheet_entry_id" + [[ -f "$id_file" ]] || mark_unverifiable "No timesheet entry created in prior test" + local entry_id + entry_id=$(<"$id_file") + + run_smoke basecamp timesheet entry delete "$entry_id" --force --json + assert_success + assert_json_value '.data.deleted' 'true' +} diff --git a/e2e/timesheet.bats b/e2e/timesheet.bats index 87cd49417..2c3933bcb 100644 --- a/e2e/timesheet.bats +++ b/e2e/timesheet.bats @@ -90,3 +90,42 @@ load test_helper run basecamp timesheet foobar # Command may show help or require project - just verify it runs } + + +# Entry verbs: refused locally, before any request + +@test "timesheet entry create without --hours shows error" { + create_credentials + create_global_config '{"account_id": 99999}' + + run basecamp timesheet entry create 789 + assert_failure + assert_output_contains "--hours is required" +} + +@test "timesheet entry update with nothing to change shows error" { + create_credentials + create_global_config '{"account_id": 99999}' + + run basecamp timesheet entry update 123 + assert_failure + assert_output_contains "Nothing to update" +} + +@test "timesheet entry delete refuses an item URL" { + create_credentials + create_global_config '{"account_id": 99999}' + + run basecamp timesheet entry delete "https://3.basecamp.com/99999/buckets/89/todos/789" --force + assert_failure + assert_output_contains "does not name a timesheet entry" +} + +@test "timesheet entry delete needs --force without a terminal" { + create_credentials + create_global_config '{"account_id": 99999}' + + run basecamp timesheet entry delete 123 --json + assert_failure + assert_output_contains "--force" +} diff --git a/internal/commands/commands.go b/internal/commands/commands.go index 3c4bc76ff..ed1385cd5 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -73,7 +73,7 @@ func CommandCategories() []CommandCategory { { Name: "Scheduling & Time", Commands: []CommandInfo{ - {Name: "timesheet", Category: "scheduling", Description: "Manage time tracking", Actions: []string{"report", "project", "item"}}, + {Name: "timesheet", Category: "scheduling", Description: "Manage time tracking", Actions: []string{"report", "project", "item", "entry"}}, {Name: "timeline", Category: "scheduling", Description: "View activity timelines", Actions: []string{}}, {Name: "reports", Category: "scheduling", Description: "View reports", Actions: []string{"assignable", "assigned", "overdue", "schedule"}}, {Name: "assignments", Category: "scheduling", Description: "View my assignments", Actions: []string{"list", "completed", "due", "prioritize", "deprioritize", "reorder"}}, diff --git a/internal/commands/timesheet.go b/internal/commands/timesheet.go index 5e8ed99d3..8e5824e76 100644 --- a/internal/commands/timesheet.go +++ b/internal/commands/timesheet.go @@ -25,8 +25,9 @@ func NewTimesheetCmd() *cobra.Command { Timesheet entries track time logged against any item (todo, message, document, etc.). The account-wide report defaults to the last month if no -date range is specified.`, - Annotations: map[string]string{"agent_notes": "Time is logged against items (todos, cards, messages, etc.)\nbasecamp clock is a shortcut for timesheet entry create\nUse basecamp reports assigned --json to see assigned items, then clock time against them"}, +date range is specified. "timesheet entry" logs, shows, changes and deletes +one entry.`, + Annotations: map[string]string{"agent_notes": "Time is logged against items (todos, cards, messages, etc.)\nbasecamp timesheet entry create --hours 1.5 logs time; show, update and delete take the entry id\nUse basecamp reports assigned --json to see assigned items, then log time against them"}, RunE: func(cmd *cobra.Command, args []string) error { return runTimesheetReport(cmd, startDate, endDate, personID) }, @@ -44,6 +45,7 @@ date range is specified.`, newTimesheetReportCmd(&startDate, &endDate, &personID), newTimesheetProjectCmd(&project), newTimesheetRecordingCmd(&project), + newTimesheetEntryCmd(), ) return cmd diff --git a/internal/commands/timesheet_entries.go b/internal/commands/timesheet_entries.go new file mode 100644 index 000000000..3b74899e5 --- /dev/null +++ b/internal/commands/timesheet_entries.go @@ -0,0 +1,562 @@ +package commands + +import ( + "errors" + "fmt" + "net/url" + "regexp" + "slices" + "strconv" + "strings" + "time" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + "github.com/spf13/cobra" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/dateparse" + "github.com/basecamp/basecamp-cli/internal/hostutil" + "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/internal/tui" + "github.com/basecamp/basecamp-cli/internal/urlarg" +) + +// newTimesheetEntryCmd creates the `timesheet entry` group: one timesheet +// entry at a time. +// +// bc3 routes timesheet entries by account alone — POST +// /recordings/:id/timesheet/entries.json to log time against a recording, and +// /timesheet_entries/:id for one entry — so no verb needs a project, and +// none reads the --project/--in the timesheet group carries for its reports. +func newTimesheetEntryCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "entry", + Short: "Log, view, change and delete timesheet entries", + Long: `Log, view, change and delete individual timesheet entries. + +An entry is time logged against one item: a to-do, message, card, document, +upload, schedule entry, or a project's own timesheet. Every verb but create +addresses the entry itself, by id or by its Basecamp URL. + + basecamp timesheet entry create 789 --hours 1.5 + basecamp timesheet entry show 1069480283 + basecamp timesheet entry update 1069480283 --hours 2:15 + basecamp timesheet entry delete 1069480283 --force`, + } + + cmd.AddCommand( + newTimesheetEntryShowCmd(), + newTimesheetEntryCreateCmd(), + newTimesheetEntryUpdateCmd(), + newTimesheetEntryDeleteCmd(), + ) + + return cmd +} + +func newTimesheetEntryShowCmd() *cobra.Command { + return &cobra.Command{ + Use: "show ", + Short: "Show a timesheet entry", + Long: `Show one timesheet entry: its date, hours, description, the person the +time is for, and the item it is logged against. + + basecamp timesheet entry show 1069480283 + basecamp timesheet entry show https://3.basecamp.com/123/projects/456/timesheet/entries/1069480283`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + entryID, urlAccountID, err := timesheetEntryIDArg(args[0]) + if err != nil { + return err + } + + app := appctx.FromContext(cmd.Context()) + scope := newFollowUpScope(app) + if err := timesheetURLAccount(app, args[0], urlAccountID); err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + entry, err := app.Account().Timesheet().Get(cmd.Context(), entryID) + if err != nil { + return convertSDKError(err) + } + + return app.OK(entry, + output.WithSummary(timesheetEntrySummary("Timesheet entry", entry)), + output.WithBreadcrumbs(timesheetEntryBreadcrumbs(cmd, entry, scope.args(app.Config.AccountID))...), + ) + }, + } +} + +func newTimesheetEntryCreateCmd() *cobra.Command { + var hours, date, description, person string + + cmd := &cobra.Command{ + Use: "create ", + Short: "Log time against an item", + Long: `Log time against an item: a to-do, message, card, document, upload or +schedule entry, by its id or pasted Basecamp URL. + +To log time to a project without an item, pass the id of the project's own +timesheet: the parent id of a project-level entry in +"basecamp timesheet project". + +--hours is required, as a decimal (1.5) or hours and minutes (1:30). +--date defaults to today and takes ISO 8601 or a relative date (yesterday, +monday). --person is who the time is for, by id, name, +email or "me"; it defaults to you, and must be a non-client member of the +project. + + basecamp timesheet entry create 789 --hours 1.5 + basecamp timesheet entry create https://3.basecamp.com/123/buckets/456/todos/789 --hours 0:45 --date yesterday --description "Review"`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + recordingID, urlAccountID, err := timesheetRecordingIDArg(args[0]) + if err != nil { + return err + } + if strings.TrimSpace(hours) == "" { + return output.ErrUsageHint("--hours is required", `Pass the time spent, as a decimal (1.5) or hours and minutes (1:30)`) + } + if err := validateTimesheetHours(hours); err != nil { + return err + } + entryDate := time.Now().Format("2006-01-02") + if cmd.Flags().Changed("date") { + if entryDate, err = timesheetDate(date); err != nil { + return err + } + } + + app := appctx.FromContext(cmd.Context()) + scope := newFollowUpScope(app) + if err := timesheetURLAccount(app, args[0], urlAccountID); err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + req := &basecamp.CreateTimesheetEntryRequest{ + Date: entryDate, + Hours: strings.TrimSpace(hours), + Description: description, + } + if cmd.Flags().Changed("person") { + if req.PersonID, err = timesheetPersonID(cmd, app, person); err != nil { + return err + } + } + + entry, err := app.Account().Timesheet().Create(cmd.Context(), recordingID, req) + if err != nil { + return convertSDKError(err) + } + + return app.OK(entry, + output.WithSummary(timesheetEntrySummary("Logged timesheet entry", entry)), + output.WithBreadcrumbs(timesheetEntryBreadcrumbs(cmd, entry, scope.args(app.Config.AccountID))...), + ) + }, + } + + cmd.Flags().StringVar(&hours, "hours", "", `Hours spent: decimal ("1.5") or hours and minutes ("1:30")`) + cmd.Flags().StringVarP(&date, "date", "d", "", "Date the time was spent (ISO 8601 or relative, e.g. yesterday; default today)") + cmd.Flags().StringVar(&description, "description", "", "Note about the work") + cmd.Flags().StringVar(&person, "person", "", `Person the time is for: ID, name, email or "me" (default: you)`) + + return cmd +} + +func newTimesheetEntryUpdateCmd() *cobra.Command { + var hours, date, description, person string + + cmd := &cobra.Command{ + Use: "update ", + Short: "Change a timesheet entry", + Long: `Change a timesheet entry's hours, date, description or person. Only the +flags you pass are sent; at least one is required. A description cannot be +cleared through the API, only replaced. + + basecamp timesheet entry update 1069480283 --hours 2.5 + basecamp timesheet entry update 1069480283 --date 2026-10-26 --description "Client call"`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + entryID, urlAccountID, err := timesheetEntryIDArg(args[0]) + if err != nil { + return err + } + + req := &basecamp.UpdateTimesheetEntryRequest{} + changed := false + if cmd.Flags().Changed("hours") { + if err := validateTimesheetHours(hours); err != nil { + return err + } + req.Hours = strings.TrimSpace(hours) + changed = true + } + if cmd.Flags().Changed("date") { + if req.Date, err = timesheetDate(date); err != nil { + return err + } + changed = true + } + if cmd.Flags().Changed("description") { + if description == "" { + return output.ErrUsageHint("--description cannot be empty", + "The API cannot clear a timesheet entry's description; pass the new text") + } + req.Description = description + changed = true + } + if !changed && !cmd.Flags().Changed("person") { + return output.ErrUsageHint("Nothing to update", + "Pass at least one of --hours, --date, --description or --person") + } + + app := appctx.FromContext(cmd.Context()) + scope := newFollowUpScope(app) + if err := timesheetURLAccount(app, args[0], urlAccountID); err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + if cmd.Flags().Changed("person") { + if req.PersonID, err = timesheetPersonID(cmd, app, person); err != nil { + return err + } + } + + entry, err := app.Account().Timesheet().Update(cmd.Context(), entryID, req) + if err != nil { + return convertSDKError(err) + } + + return app.OK(entry, + output.WithSummary(timesheetEntrySummary("Updated timesheet entry", entry)), + output.WithBreadcrumbs(timesheetEntryBreadcrumbs(cmd, entry, scope.args(app.Config.AccountID))...), + ) + }, + } + + cmd.Flags().StringVar(&hours, "hours", "", `Hours spent: decimal ("1.5") or hours and minutes ("1:30")`) + cmd.Flags().StringVarP(&date, "date", "d", "", "Date the time was spent (ISO 8601 or relative, e.g. yesterday)") + cmd.Flags().StringVar(&description, "description", "", "Note about the work") + cmd.Flags().StringVar(&person, "person", "", `Person the time is for: ID, name, email or "me"`) + + return cmd +} + +func newTimesheetEntryDeleteCmd() *cobra.Command { + var force bool + + cmd := &cobra.Command{ + Use: "delete ", + Short: "Delete a timesheet entry", + Long: `Permanently delete a timesheet entry. It never goes to the trash and +cannot be restored, so this asks for confirmation; machine-output modes and +non-interactive sessions cannot answer one and need --force instead. It is the +only way to remove an entry: Basecamp refuses to trash or archive one. People +who may not remove the entry are refused. + + basecamp timesheet entry delete 1069480283 + basecamp timesheet entry delete 1069480283 --json --force`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + entryID, urlAccountID, err := timesheetEntryIDArg(args[0]) + if err != nil { + return err + } + + // Permanent, so only with --force or a confirmation somebody can + // answer — refused before any account lookup or request. + if err := ensureDeleteConfirmable(cmd, force); err != nil { + return err + } + + app := appctx.FromContext(cmd.Context()) + scope := newFollowUpScope(app) + if err := timesheetURLAccount(app, args[0], urlAccountID); err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + if !force { + confirmed, err := tui.ConfirmDangerous("Permanently delete this timesheet entry?") + switch { + case errors.Is(err, tui.ErrCanceled): + return nil + case err != nil: + return fmt.Errorf("confirming the delete: %w", err) + case !confirmed: + return nil + } + } + + if err := app.Account().Timesheet().Destroy(cmd.Context(), entryID); err != nil { + return convertSDKError(err) + } + + return app.OK(map[string]any{"id": entryID, "deleted": true}, + output.WithSummary(fmt.Sprintf("Deleted timesheet entry #%d", entryID)), + output.WithBreadcrumbs(output.Breadcrumb{ + Action: "report", + Cmd: "basecamp timesheet report" + scope.args(app.Config.AccountID), + Description: "View timesheet report", + }), + ) + }, + } + + cmd.Flags().BoolVarP(&force, "force", "f", false, "Skip confirmation prompt") + + return cmd +} + +// timesheetEntryRoute matches the path of an entry's own URL: its app URL +// (projects/:p/timesheet/entries/:id) and its API URL in either form. The +// first group is the account, the second the entry. +var timesheetEntryRoute = regexp.MustCompile(`^/(\d+)/(?:(?:projects|buckets)/\d+/timesheet/entries|timesheet_entries)/(\d+)(?:\.json)?/?$`) + +// timesheetEntryIDArg resolves the positional the per-entry verbs +// take: a bare id or the entry's own URL, returning the URL's account too. Any +// other URL is refused rather than read as an entry id — a to-do's URL names +// the item the time is logged against, and `timesheet entry delete` must never +// aim at one of those. +func timesheetEntryIDArg(arg string) (id int64, urlAccountID string, err error) { + raw := strings.TrimSpace(arg) + if !isAllDigits(raw) { + var m []string + if u := pastedURL(raw); u != nil { + m = timesheetEntryRoute.FindStringSubmatch(u.Path) + } + if m == nil { + return 0, "", output.ErrUsageHint( + fmt.Sprintf("%q does not name a timesheet entry", arg), + "Pass the entry id, or its URL (…/timesheet/entries/); "+ + "basecamp timesheet item lists the entries on an item") + } + urlAccountID, raw = m[1], m[2] + } + id, perr := strconv.ParseInt(raw, 10, 64) + if perr != nil || id <= 0 { + return 0, "", output.ErrUsageHint( + fmt.Sprintf("%q is not a timesheet entry id or URL", arg), + "Pass the numeric entry id, or paste its Basecamp URL") + } + return id, urlAccountID, nil +} + +// timesheetKinds are the URL types that name an item bc3 logs time against, +// as its API docs list them, or a generic recordings/ URL, which is as good +// as its bare id. bc3 still decides each case; the list keeps a URL for +// something else (a person, a project, a Campfire, a path the router only +// matched by shape) from sending its id out as a recording's. +var timesheetKinds = []string{ + "cards", "documents", "messages", "recordings", "schedule_entries", "todos", "uploads", +} + +// timesheetRecordingIDArg resolves create's : a bare id, or +// the http(s) URL of one item of a timesheet kind, returning the URL's account +// too. Anything else is refused rather than read as a recording id. +func timesheetRecordingIDArg(arg string) (id int64, urlAccountID string, err error) { + raw := strings.TrimSpace(arg) + if !isAllDigits(raw) { + // Route only the URL's own scheme, host and path, so nothing in a query + // or fragment can be read as the item. No timesheet kind is named by a + // fragment: a comment's #__recording_ cannot hold time. + var parsed *urlarg.Parsed + if u := pastedURL(raw); u != nil { + parsed = urlarg.Parse(u.Scheme + "://" + u.Host + u.Path) + } + switch { + case parsed == nil || parsed.IsCollection || parsed.RecordingID == "" || + !slices.Contains(timesheetKinds, parsed.Type): + return 0, "", output.ErrUsageHint( + fmt.Sprintf("%q does not name an item to log time against", arg), + "Pass the id or URL of the to-do, message, card, document, upload or schedule entry. "+ + "For a project-level entry, pass the project timesheet's id: the parent id of a "+ + "project-level entry in basecamp timesheet project") + case parsed.OccurrenceDate != "": + return 0, "", output.ErrUsageHint("That URL names one occurrence of a recurring event", + "Pass the occurrence's own id, which "+showOccurrence(raw)+" prints, or the event's id") + } + raw, urlAccountID = parsed.RecordingID, parsed.AccountID + } + id, perr := strconv.ParseInt(raw, 10, 64) + if perr != nil || id <= 0 { + return 0, "", output.ErrUsageHint( + fmt.Sprintf("%q is not an item id or Basecamp URL", arg), + "Pass the numeric id of the item to log time against, or paste its Basecamp URL") + } + return id, urlAccountID, nil +} + +func isAllDigits(s string) bool { + if s == "" { + return false + } + for _, r := range s { + if r < '0' || r > '9' { + return false + } + } + return true +} + +// pastedURL parses an http(s) URL with a host, or returns nil. Callers match +// its Path alone: a regexp over the raw string would read text after a ? or # +// as path segments. +func pastedURL(s string) *url.URL { + u, err := url.Parse(s) + if err != nil || u.Host == "" || u.User != nil { + return nil + } + switch strings.ToLower(u.Scheme) { + case "http", "https": + return u + } + return nil +} + +// timesheetURLAccount holds a pasted URL to the account it names, before any +// request. Any argument that is not a bare id is a URL here, and the URL +// router is host-agnostic, so it must be on a trusted Basecamp host; its +// account is adopted when none is configured and must equal the configured one +// otherwise (numerically, as accountIDsEqual compares), rather than acting on +// a same-numbered entry or item in another account. +func timesheetURLAccount(app *appctx.App, arg, urlAccountID string) error { + raw := strings.TrimSpace(arg) + if isAllDigits(raw) { + return nil + } + if !hostutil.IsTrustedBasecampHost(raw, app.Config.BaseURL) { + return output.ErrUsage("refusing untrusted host in URL — expected a Basecamp URL") + } + switch { + case urlAccountID == "": + case app.Config.AccountID == "": + app.Config.AccountID = urlAccountID + case !accountIDsEqual(urlAccountID, app.Config.AccountID): + return output.ErrUsage(fmt.Sprintf("URL account %s does not match the configured account %s", urlAccountID, app.Config.AccountID)) + } + return nil +} + +// timesheetHours is what bc3 accepts for hours: a decimal, or hours and +// minutes. +var timesheetHours = regexp.MustCompile(`^(?:\d+(?:\.\d+)?|\.\d+|\d+:[0-5]\d)$`) + +func validateTimesheetHours(hours string) error { + h := strings.TrimSpace(hours) + if !timesheetHours.MatchString(h) { + return output.ErrUsageHint(fmt.Sprintf("Invalid hours %q", hours), + `Pass hours as a decimal ("1.5") or hours and minutes ("1:30")`) + } + return nil +} + +// timesheetISODate is the only form bc3 takes for an entry's date. +var timesheetISODate = regexp.MustCompile(`^\d{4}-\d{2}-\d{2}$`) + +// timesheetDate turns --date into the ISO 8601 date bc3 takes. dateparse +// returns what it doesn't recognize unchanged, so anything that still isn't a +// real date is refused here rather than sent. +func timesheetDate(input string) (string, error) { + parsed := dateparse.Parse(input) + if timesheetISODate.MatchString(parsed) { + if _, err := time.Parse("2006-01-02", parsed); err == nil { + return parsed, nil + } + } + return "", output.ErrUsageHint(fmt.Sprintf("Invalid date %q", input), + "Pass an ISO 8601 date (2026-10-27) or a relative one (today, yesterday, monday)") +} + +// timesheetPersonID resolves --person: a numeric id is sent as given, and a +// name, email or "me" is looked up. +func timesheetPersonID(cmd *cobra.Command, app *appctx.App, person string) (int64, error) { + p := strings.TrimSpace(person) + if p == "" { + return 0, output.ErrUsageHint("--person cannot be empty", `Pass a person's ID, name, email or "me"`) + } + if id, err := strconv.ParseInt(p, 10, 64); err == nil { + if id <= 0 { + return 0, output.ErrUsage("Invalid person ID") + } + return id, nil + } + resolved, _, err := app.Names.ResolvePerson(cmd.Context(), p) + if err != nil { + return 0, err + } + id, err := strconv.ParseInt(resolved, 10, 64) + if err != nil { + return 0, output.ErrUsage("Invalid person ID") + } + return id, nil +} + +func timesheetEntrySummary(prefix string, entry *basecamp.TimesheetEntry) string { + summary := fmt.Sprintf("%s #%d: %sh on %s", prefix, entry.ID, entry.Hours, entry.Date) + // The generic renderers skip nested objects, so whose time it is would be + // missing from styled and Markdown output without this. + if entry.Person != nil && entry.Person.ID != 0 { + summary += fmt.Sprintf(" by %s (#%d)", entry.Person.Name, entry.Person.ID) + } + if entry.Parent != nil && entry.Parent.Title != "" { + summary += " for " + entry.Parent.Title + } + return summary +} + +// timesheetEntryBreadcrumbs are the follow-ups for one entry: change or delete +// it, and see the rest of the time on its item. +// +// scopeArgs carries the profile and an account that would not come back on its +// own (adopted from a URL, given with --account), so a pasted follow-up acts +// where this one did. +func timesheetEntryBreadcrumbs(cmd *cobra.Command, entry *basecamp.TimesheetEntry, scopeArgs string) []output.Breadcrumb { + crumbs := []output.Breadcrumb{ + { + Action: "update", + Cmd: fmt.Sprintf("basecamp timesheet entry update %d --hours %s", entry.ID, scopeArgs), + Description: "Change entry", + }, + { + Action: "delete", + Cmd: timesheetEntryDeleteCmd(cmd, entry.ID, scopeArgs), + Description: "Delete entry", + }, + } + if entry.Parent != nil && entry.Parent.ID != 0 { + crumbs = append(crumbs, output.Breadcrumb{ + Action: "item", + Cmd: fmt.Sprintf("basecamp timesheet item %d%s", entry.Parent.ID, scopeArgs), + Description: "View the item's timesheet", + }) + } + return crumbs +} + +// timesheetEntryDeleteCmd builds the delete breadcrumb for whoever will read +// it, as deleteLineCmd does for chat lines: machine output cannot answer the +// confirmation, so it gets --force; a person gets the bare command and is +// asked, with any flag holding this invocation in a human format carried +// along so the pasted command renders the same way. +func timesheetEntryDeleteCmd(cmd *cobra.Command, entryID int64, scopeArgs string) string { + c := fmt.Sprintf("basecamp timesheet entry delete %d%s", entryID, scopeArgs) + if machineReadsThisOutput(cmd) { + return c + " --force" + } + return c + humanFormatOverride(appctx.FromContext(cmd.Context())) +} diff --git a/internal/commands/timesheet_entries_test.go b/internal/commands/timesheet_entries_test.go new file mode 100644 index 000000000..ef101a4dc --- /dev/null +++ b/internal/commands/timesheet_entries_test.go @@ -0,0 +1,339 @@ +package commands + +import ( + "encoding/json" + "fmt" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const timesheetEntryFixture = `{ + "id": 1069480283, + "status": "active", + "type": "Timesheet::Entry", + "title": "Timesheet entry", + "date": "2026-10-26", + "hours": "1.5", + "description": "Review", + "created_at": "2026-10-26T10:00:00Z", + "updated_at": "2026-10-26T10:00:00Z", + "parent": {"id": 789, "title": "Plan the offsite", "type": "Todo"}, + "bucket": {"id": 89, "name": "Offsite", "type": "Project"}, + "creator": {"id": 7, "name": "Victor Cooper"}, + "person": {"id": 7, "name": "Victor Cooper"} +}` + +func timesheetEntryPath(id int64) string { + return fmt.Sprintf("/99999/timesheet_entries/%d", id) +} + +func timesheetEntriesPath(recordingID int64) string { + return fmt.Sprintf("/99999/recordings/%d/timesheet/entries.json", recordingID) +} + +func runTimesheet(t *testing.T, routes []stubRoute, args ...string) (*recordingTransport, string, json.RawMessage, error) { + t.Helper() + app, transport, out := setupPersonalFeedApp(t, routes...) + err := executeRecordingCommand(NewTimesheetCmd(), app, args...) + if err != nil { + return transport, "", nil, err + } + data, summary := subtaskEnvelope(t, out) + return transport, summary, data, nil +} + +func TestTimesheetEntryShowGetsTheEntry(t *testing.T) { + transport, summary, data, err := runTimesheet(t, + []stubRoute{{method: http.MethodGet, path: timesheetEntryPath(1069480283), status: http.StatusOK, body: timesheetEntryFixture}}, + "entry", "show", "1069480283") + require.NoError(t, err) + + call := transport.last(t) + assert.Equal(t, http.MethodGet, call.Method) + assert.Equal(t, timesheetEntryPath(1069480283), call.Path) + assert.Equal(t, "Timesheet entry #1069480283: 1.5h on 2026-10-26 by Victor Cooper (#7) for Plan the offsite", summary) + + var entry map[string]any + require.NoError(t, json.Unmarshal(data, &entry)) + assert.Equal(t, "1.5", entry["hours"]) +} + +// An entry's own URL names it, in its app form and both API forms. +func TestTimesheetEntryShowAcceptsTheEntryURL(t *testing.T) { + for _, url := range []string{ + "https://3.basecamp.com/99999/projects/89/timesheet/entries/1069480283", + "https://3.basecampapi.com/99999/projects/89/timesheet/entries/1069480283.json", + "https://3.basecampapi.com/99999/timesheet_entries/1069480283.json", + } { + transport, _, _, err := runTimesheet(t, + []stubRoute{{method: http.MethodGet, path: timesheetEntryPath(1069480283), status: http.StatusOK, body: timesheetEntryFixture}}, + "entry", "show", url) + require.NoError(t, err, url) + assert.Equal(t, timesheetEntryPath(1069480283), transport.last(t).Path, url) + } +} + +func TestTimesheetEntryCreatePostsDateHoursDescriptionAndPerson(t *testing.T) { + transport, summary, _, err := runTimesheet(t, + []stubRoute{{method: http.MethodPost, path: timesheetEntriesPath(789), status: http.StatusCreated, body: timesheetEntryFixture}}, + "entry", "create", "789", "--hours", "1:30", "--date", "2026-10-26", "--description", "Review", "--person", "7") + require.NoError(t, err) + + call := transport.last(t) + assert.Equal(t, http.MethodPost, call.Method) + assert.Equal(t, timesheetEntriesPath(789), call.Path) + assert.Equal(t, map[string]any{ + "date": "2026-10-26", "hours": "1:30", "description": "Review", "person_id": float64(7), + }, decodeSubtaskBody(t, call)) + assert.Equal(t, "Logged timesheet entry #1069480283: 1.5h on 2026-10-26 by Victor Cooper (#7) for Plan the offsite", summary) +} + +// --date defaults to today, and unset optional fields are not sent. +func TestTimesheetEntryCreateDefaultsToTodayAndOmitsUnset(t *testing.T) { + transport, _, _, err := runTimesheet(t, + []stubRoute{{method: http.MethodPost, path: timesheetEntriesPath(789), status: http.StatusCreated, body: timesheetEntryFixture}}, + "entry", "create", "https://3.basecamp.com/99999/buckets/89/todos/789", "--hours", "2") + require.NoError(t, err) + + assert.Equal(t, map[string]any{ + "date": time.Now().Format("2006-01-02"), "hours": "2", + }, decodeSubtaskBody(t, transport.last(t))) +} + +// "me" is resolved to the caller's person id before the entry is created. +func TestTimesheetEntryCreateResolvesPersonMe(t *testing.T) { + transport, _, _, err := runTimesheet(t, + []stubRoute{ + {method: http.MethodGet, path: "/99999/my/profile.json", status: http.StatusOK, body: `{"id": 42, "name": "Me"}`}, + {method: http.MethodPost, path: timesheetEntriesPath(789), status: http.StatusCreated, body: timesheetEntryFixture}, + }, + "entry", "create", "789", "--hours", "1", "--person", "me") + require.NoError(t, err) + + assert.Equal(t, float64(42), decodeSubtaskBody(t, transport.last(t))["person_id"]) +} + +func TestTimesheetEntryCreateRefusesBadInput(t *testing.T) { + for _, args := range [][]string{ + {"entry", "create", "789"}, + {"entry", "create", "789", "--hours", ""}, + {"entry", "create", "789", "--hours", "an hour"}, + {"entry", "create", "789", "--hours", "1:75"}, + {"entry", "create", "789", "--hours", "1", "--date", "someday"}, + {"entry", "create", "789", "--hours", "1", "--date", "2026-13-40"}, + {"entry", "create", "789", "--hours", "1", "--person", ""}, + {"entry", "create", "not-an-id", "--hours", "1"}, + // A project's timesheet URL carries the project id, not the + // timesheet recording's. + {"entry", "create", "https://3.basecamp.com/99999/projects/89/timesheet", "--hours", "1"}, + // Neither does a project's URL, or a listing's. + {"entry", "create", "https://3.basecamp.com/99999/projects/89", "--hours", "1"}, + {"entry", "create", "https://3.basecamp.com/99999/buckets/89/todolists", "--hours", "1"}, + // A person's URL parses to an id, but not a recording's. + {"entry", "create", "https://3.basecamp.com/99999/people/789", "--hours", "1"}, + // Without a scheme there is no host to trust. + {"entry", "create", "//evil.example/11111/buckets/89/todos/789", "--hours", "1"}, + {"entry", "create", "3.basecamp.com/99999/buckets/89/todos/789", "--hours", "1"}, + } { + transport, _, _, err := runTimesheet(t, nil, args...) + requireBookmarksUsageError(t, err) + assert.Empty(t, transport.recorded(), "no request for %v", args) + } +} + +func TestTimesheetEntryUpdateSendsOnlyWhatChanged(t *testing.T) { + transport, summary, _, err := runTimesheet(t, + []stubRoute{{method: http.MethodPut, path: timesheetEntryPath(1069480283), status: http.StatusOK, body: timesheetEntryFixture}}, + "entry", "update", "1069480283", "--hours", "2.5") + require.NoError(t, err) + + call := transport.last(t) + assert.Equal(t, http.MethodPut, call.Method) + assert.Equal(t, timesheetEntryPath(1069480283), call.Path) + assert.Equal(t, map[string]any{"hours": "2.5"}, decodeSubtaskBody(t, call)) + assert.Equal(t, "Updated timesheet entry #1069480283: 1.5h on 2026-10-26 by Victor Cooper (#7) for Plan the offsite", summary) +} + +func TestTimesheetEntryUpdateSendsEveryField(t *testing.T) { + transport, _, _, err := runTimesheet(t, + []stubRoute{{method: http.MethodPut, path: timesheetEntryPath(1069480283), status: http.StatusOK, body: timesheetEntryFixture}}, + "entry", "update", "1069480283", "--hours", "3", "--date", "2026-10-25", "--description", "Call", "--person", "8") + require.NoError(t, err) + + assert.Equal(t, map[string]any{ + "date": "2026-10-25", "hours": "3", "description": "Call", "person_id": float64(8), + }, decodeSubtaskBody(t, transport.last(t))) +} + +func TestTimesheetEntryUpdateRefusesBadInput(t *testing.T) { + for _, args := range [][]string{ + {"entry", "update", "1069480283"}, + {"entry", "update", "1069480283", "--hours", "x"}, + {"entry", "update", "1069480283", "--date", "someday"}, + {"entry", "update", "1069480283", "--description", ""}, + {"entry", "update", "1069480283", "--person", ""}, + {"entry", "update", "not-an-id", "--hours", "1"}, + } { + transport, _, _, err := runTimesheet(t, nil, args...) + requireBookmarksUsageError(t, err) + assert.Empty(t, transport.recorded(), "no request for %v", args) + } +} + +func TestTimesheetEntryDeleteDestroysTheEntry(t *testing.T) { + transport, summary, data, err := runTimesheet(t, + []stubRoute{{method: http.MethodDelete, path: timesheetEntryPath(1069480283), status: http.StatusNoContent}}, + "entry", "delete", "1069480283", "--force") + require.NoError(t, err) + + call := transport.last(t) + assert.Equal(t, http.MethodDelete, call.Method) + assert.Equal(t, timesheetEntryPath(1069480283), call.Path) + assert.Equal(t, "Deleted timesheet entry #1069480283", summary) + assert.JSONEq(t, `{"id": 1069480283, "deleted": true}`, string(data)) +} + +// A permanent delete in machine-output mode has nobody to confirm it, so it +// needs --force and is refused before any request without it. +func TestTimesheetEntryDeleteNeedsForceWhenNothingCanConfirm(t *testing.T) { + transport, _, _, err := runTimesheet(t, nil, "entry", "delete", "1069480283") + outErr := requireBookmarksUsageError(t, err) + assert.Contains(t, outErr.Message, "--force") + assert.Empty(t, transport.recorded()) +} + +// A URL that is not an entry's own names something else — most often the item +// the time is logged against — and must never be read as an entry id, least +// of all by delete. +func TestTimesheetEntryVerbsRefuseOtherURLs(t *testing.T) { + for _, url := range []string{ + "https://3.basecamp.com/99999/buckets/89/todos/789", + "https://3.basecamp.com/99999/projects/89/timesheet", + "https://example.com/not/basecamp", + } { + for _, verb := range [][]string{{"show"}, {"update", "--hours", "1"}, {"delete", "--force"}} { + args := append([]string{"entry", verb[0], url}, verb[1:]...) + transport, _, _, err := runTimesheet(t, nil, args...) + requireBookmarksUsageError(t, err) + assert.Empty(t, transport.recorded(), "no request for %v", args) + } + } +} + +func TestTimesheetEntryVerbsRejectANonID(t *testing.T) { + for _, args := range [][]string{ + {"entry", "show", "not-an-id"}, + {"entry", "show", "0"}, + {"entry", "delete", "12abc", "--force"}, + } { + transport, _, _, err := runTimesheet(t, nil, args...) + requireBookmarksUsageError(t, err) + assert.Empty(t, transport.recorded(), "no request for %v", args) + } +} + +// A refusal from Basecamp (403 when the caller may not remove the entry) is +// returned as an error, not reported as a delete. +func TestTimesheetEntryDeleteReportsARefusal(t *testing.T) { + _, _, _, err := runTimesheet(t, + []stubRoute{{method: http.MethodDelete, path: timesheetEntryPath(1069480283), status: http.StatusForbidden, body: `{"error":"forbidden"}`}}, + "entry", "delete", "1069480283", "--force") + require.Error(t, err) +} + +// A pasted URL acts only on the account it names, on a Basecamp host: a +// look-alike host, or a URL from another account than the configured one, is +// refused before any request, on every verb. +func TestTimesheetEntryVerbsHoldAURLToItsHostAndAccount(t *testing.T) { + for _, tc := range []struct { + name string + args []string + }{ + {"show untrusted host", []string{"entry", "show", "https://evil.example/99999/timesheet_entries/1069480283.json"}}, + {"update untrusted host", []string{"entry", "update", "https://evil.example/99999/projects/89/timesheet/entries/1069480283", "--hours", "1"}}, + {"delete untrusted host", []string{"entry", "delete", "https://evil.example/99999/timesheet_entries/1069480283", "--force"}}, + {"create untrusted host", []string{"entry", "create", "https://evil.example/99999/buckets/89/todos/789", "--hours", "1"}}, + {"show other account", []string{"entry", "show", "https://3.basecamp.com/11111/projects/89/timesheet/entries/1069480283"}}, + {"update other account", []string{"entry", "update", "https://3.basecampapi.com/11111/timesheet_entries/1069480283.json", "--hours", "1"}}, + {"delete other account", []string{"entry", "delete", "https://3.basecamp.com/11111/projects/89/timesheet/entries/1069480283", "--force"}}, + {"create other account", []string{"entry", "create", "https://3.basecamp.com/11111/buckets/89/todos/789", "--hours", "1"}}, + {"show scheme-less", []string{"entry", "show", "//evil.example/11111/timesheet_entries/1069480283"}}, + {"delete scheme-less", []string{"entry", "delete", "3.basecamp.com/99999/timesheet_entries/1069480283", "--force"}}, + // A path written after a fragment or query is not the URL's path. + {"update path in fragment", []string{"entry", "update", "https://3.basecamp.com#x/99999/timesheet_entries/1069480283", "--hours", "1"}}, + {"delete path in query", []string{"entry", "delete", "https://3.basecamp.com/?x=/99999/timesheet_entries/1069480283", "--force"}}, + {"create path in fragment", []string{"entry", "create", "https://3.basecamp.com#x/99999/buckets/89/todos/789", "--hours", "1"}}, + {"create userinfo", []string{"entry", "create", "https://evil.example@3.basecamp.com/99999/buckets/89/todos/789", "--hours", "1"}}, + } { + t.Run(tc.name, func(t *testing.T) { + transport, _, _, err := runTimesheet(t, nil, tc.args...) + requireBookmarksUsageError(t, err) + assert.Empty(t, transport.recorded()) + }) + } +} + +// The delete breadcrumb is something its reader can run: machine output cannot +// answer a confirmation, so the JSON a test renders carries --force. +func TestTimesheetEntryDeleteBreadcrumbCarriesForceForMachineOutput(t *testing.T) { + app, _, out := setupPersonalFeedApp(t, + stubRoute{method: http.MethodGet, path: timesheetEntryPath(1069480283), status: http.StatusOK, body: timesheetEntryFixture}) + app.Flags.Hints = true // breadcrumbs are stripped from the envelope without this + require.NoError(t, executeRecordingCommand(NewTimesheetCmd(), app, "entry", "show", "1069480283")) + + var envelope struct { + Breadcrumbs []struct { + Action string `json:"action"` + Cmd string `json:"cmd"` + } `json:"breadcrumbs"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope), out.String()) + var deleteCmd string + for _, b := range envelope.Breadcrumbs { + if b.Action == "delete" { + deleteCmd = b.Cmd + } + } + assert.Equal(t, "basecamp timesheet entry delete 1069480283 --force", deleteCmd) +} + +// Account ids compare as numbers, so a zero-padded URL account is the +// configured one. +func TestTimesheetEntryAcceptsAZeroPaddedAccountInAURL(t *testing.T) { + transport, _, _, err := runTimesheet(t, + []stubRoute{{method: http.MethodGet, path: timesheetEntryPath(1069480283), status: http.StatusOK, body: timesheetEntryFixture}}, + "entry", "show", "https://3.basecamp.com/099999/projects/89/timesheet/entries/1069480283") + require.NoError(t, err) + assert.Equal(t, timesheetEntryPath(1069480283), transport.last(t).Path) +} + +// An account adopted from a pasted URL lasts only for this run, so the +// follow-ups name it; otherwise a pasted update or delete would act in +// whatever account the next run resolves. +func TestTimesheetEntryBreadcrumbsCarryAnAccountAdoptedFromTheURL(t *testing.T) { + app, _, out := setupPersonalFeedApp(t, + stubRoute{method: http.MethodGet, path: timesheetEntryPath(1069480283), status: http.StatusOK, body: timesheetEntryFixture}) + app.Config.AccountID = "" + app.Flags.Hints = true + require.NoError(t, executeRecordingCommand(NewTimesheetCmd(), app, "entry", "show", + "https://3.basecamp.com/99999/projects/89/timesheet/entries/1069480283")) + + var envelope struct { + Breadcrumbs []struct { + Action string `json:"action"` + Cmd string `json:"cmd"` + } `json:"breadcrumbs"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope), out.String()) + got := map[string]string{} + for _, b := range envelope.Breadcrumbs { + got[b.Action] = b.Cmd + } + assert.Equal(t, "basecamp timesheet entry update 1069480283 --hours --account 99999", got["update"]) + assert.Equal(t, "basecamp timesheet entry delete 1069480283 --account 99999 --force", got["delete"]) + assert.Equal(t, "basecamp timesheet item 789 --account 99999", got["item"]) +} diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index b5503556a..a8eb61b90 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -30,6 +30,8 @@ triggers: - basecamp checkin - basecamp check-in - basecamp timeline + - basecamp timesheet + - log time - basecamp template - basecamp webhook - basecamp gauge @@ -284,6 +286,26 @@ basecamp projects logo remove --in "" --json requires authentication and redirects to the image; it is not a public image link. +## Timesheet entries + +Time is logged against one item (to-do, message, card, document, upload, +schedule entry). `timesheet report`, `timesheet project` and `timesheet item` +read entries; `timesheet entry` writes them, addressed by account alone with +no `--in`: + +```bash +basecamp timesheet entry create --hours 1.5 --date yesterday --json +basecamp timesheet entry show --json +basecamp timesheet entry update --hours 2:15 --json +basecamp timesheet entry delete --force --json +``` + +- `--hours` is a decimal or `h:mm`; `--date` defaults to today on create; + `--person` (ID, name, email or `me`) logs time for someone else. +- `delete` is permanent: entries are never trashed. Machine output needs + `--force`. +- Accounts without timesheets enabled answer 403. + ## Content and stdin Command help declares content arguments and stdin support. Titles are generally