diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index cde6397..f21ab78 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -270,16 +270,28 @@ the moment the window opens. ### `jobs_view.go` file structure The size guideline for a file in this project is ~250 lines. -`src/ui/jobs_view.go` is split across three files along these seams; the view -file itself has grown back over the guideline since — see the split item in -[ROADMAP.md](ROADMAP.md), which tracks every file currently over it: +`src/ui/jobs_view.go` is split across five files along these seams: | File | Contents | |------|----------| -| `jobs_view.go` | `newJobsView` — list, toolbar, button wiring, and layout | +| `jobs_view.go` | `jobsView` struct — construction, `refresh`, `updateDetails`, the pause control, and layout assembly | +| `jobs_view_state.go` | `jobsViewState` — the jobs/runtime snapshot, the folder filter, and the selection | +| `jobs_view_list.go` | The sidebar list: row template, row rendering, row mode, and the compact/detailed toggle | +| `jobs_view_toolbar.go` | The per-job button row — new, edit, run, pause, delete | | `jobs_view_details.go` | `detailsPanel` struct — widget creation, `update`, `clear`, `container` | | `jobs_view_helpers.go` | Pure helpers — `filteredJobIndexes`, `folderOptions`, `filterValue`, `indexOfID`, `lastJobLogs`, `nextJobListView`, `viewToggleText` | +The widgets hold no job state of their own: they read `jobsViewState`, which is +the only thing that reads the Service. The **selection is a job ID, not a row +index.** Every path that changes the job list replaces the state's snapshot — +create, delete, and edit from this view's own handlers, adopting a different +jobs file from the Service, which the view only learns about through the refresh +`JobsLoaded` triggers. An index that outlives its snapshot points at whichever +job now sits there, so the details pane would describe one job while the list +highlighted another. Rows are derived from the ID at render time +(`selectedIndex`, `displayRow`), and `jobsView.refresh` ends by pointing the +list's highlight at the selected job. + ### `settings_view.go` file structure `src/ui/settings_view.go` is split across three files the same way, once its diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 1be7786..19a8594 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -51,6 +51,12 @@ the app icon (experimental).** gets slower the longer the app has been running. Measured on 5000 accumulated records, one History redraw went from **15.8 ms to 0.9 ms**; at the new cap the width rescan alone accounted for 1.5 ms of every redraw. +- **The Jobs tab keeps its selection on the job, not on the row.** Selecting a + different jobs file in Settings replaces the whole job list; the details pane + then described whichever job happened to land on the previously selected row — + or went blank if the new list was shorter — while the highlight in the list + stayed where it was. The selection now follows the job itself, and the + highlight and the details pane always describe the same one. - **Max log files and max log age days now accept 0, meaning "keep everything."** Log cleanup already supported disabling either policy; the Settings form and the Service validator rejected the value that would have @@ -81,6 +87,12 @@ the app icon (experimental).** released, in preparation order, so `jobs.json` still ends up matching the in-memory list. Seeding statistics from logs also opens each log file once instead of twice. +- The Jobs tab was split into `jobs_view.go` (construction, refresh, layout), + `jobs_view_state.go` (the job/runtime snapshot, folder filter, and selection), + `jobs_view_list.go`, and `jobs_view_toolbar.go`. What used to be one 330-line + constructor whose dozen closures shared seven mutable locals is now widgets + reading one named state object — which is what made the selection fix above a + change in one place instead of five. ## 1.0.1 - 2026-08-04 diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 2b86d45..d382c0f 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -123,24 +123,24 @@ Design notes / open questions: [ARCHITECTURE.md](ARCHITECTURE.md) sets a ~250-line guideline per source file and records the `jobs_view.go` and `settings_view.go` splits as the worked -examples. Six non-test files are over it at 1.0.0, including both files that -were already split once: +examples. `jobs_view.go` was split again in 1.0.2 — into view, state, list, and +toolbar — because the selection defect it carried was a symptom of the size +(one 330-line constructor over seven shared locals). Five non-test files are +over the guideline as of that pass: | File | Lines | |------|-------| -| `src/app/operations.go` | 490 | -| `src/ui/jobs_view.go` | 355 | -| `src/app/run.go` | 287 | -| `src/ui/history_view.go` | 282 | -| `src/ui/settings_view.go` | 277 | -| `src/storage/store.go` | 265 | +| `src/app/operations.go` | 529 | +| `src/ui/history_view.go` | 373 | +| `src/storage/store.go` | 365 | +| `src/ui/settings_view.go` | 318 | +| `src/app/run.go` | 274 | -This is deliberately deferred to the next whole-project review rather than done -piecemeal: a future review already asks item 2 to look for exactly this, -a split touches every reader of the file, and doing all six in one pass keeps -the seams consistent instead of settling them six different ways. Splitting is +The remaining five are deliberately deferred rather than done piecemeal: a +split touches every reader of the file, and doing them in one pass keeps the +seams consistent instead of settling them five different ways. Splitting is also the kind of change that reads as pure movement while quietly dropping a -function, so it wants one careful pass, not six hurried ones. +function, so it wants one careful pass, not five hurried ones. Seams visible today, as a starting point rather than a decision: @@ -152,13 +152,15 @@ Seams visible today, as a starting point rather than a decision: - **`history_view.go`** — the column-measuring helpers (`textWidth` through `historyColumnWidths`) are pure, already unit-tested, and independent of the table they size. -- **`jobs_view.go`** — nearly all of it is one `newJobsView` constructor, so the - split has to break that function up (list template, toolbar handlers, - assembly) rather than move whole functions. Larger judgement call than the - others. -- **`run.go`**, **`settings_view.go`**, **`store.go`** — barely over. Worth - re-measuring at the time; if a pass elsewhere has shrunk them, leave them - alone rather than splitting for the sake of the number. +- **`store.go`** — path resolution, the config load/normalize path, and the jobs + load/normalize path are three separate concerns in one file. +- **`run.go`**, **`settings_view.go`** — barely over. Worth re-measuring at the + time; if a pass elsewhere has shrunk them, leave them alone rather than + splitting for the sake of the number. + +The `jobs_view.go` pass is the worked example for the rest: the constructor was +broken up along the state it shared, not along line count, and the split landed +with the selection fix rather than promising it separately. Scope note: the guideline is about source files. Test files are much larger and that is fine — a table-driven test file grows with the cases it covers. diff --git a/docs/TESTS.md b/docs/TESTS.md index 7e22403..099a65a 100644 --- a/docs/TESTS.md +++ b/docs/TESTS.md @@ -471,11 +471,34 @@ widgets are assembled. | `TestJobListViewCompactConfigOpensCompact` | Verifies the persisted density is honoured at build time, not only after a tap. | | `TestJobsSidebarWidthIsItsContent` | Regression guard: nothing but the sidebar's own toolbar row imposes a width floor on it. | | `TestJobsSplitOpensAtTheSidebarWidth` | Verifies the derived split offset opens the divider at the sidebar's own width at the default window size — enough that the toolbar is never born clipped, and no more. | -| `TestToolbarButtonRedrawsRowAndDetails` | Regression guard: with the duplicate refreshes removed from the handlers, `refreshView` alone must re-snapshot the jobs and repopulate the details pane. | +| `TestToolbarButtonRedrawsRowAndDetails` | Regression guard: with the duplicate refreshes removed from the handlers, `jobsView.refresh` alone must re-snapshot the jobs and repopulate the details pane. | +| `TestJobsViewSelectionSurvivesAJobsFileSwitch` | Regression guard: adopting a different jobs file replaces the whole list from the Service, and the refresh that follows must leave the details pane and the list highlight describing the same job — not redraw the pane from a row index that belonged to the previous list. | | `TestDetailCaptionWidthCoversEveryCaption` | Verifies every caption `metadataRows` returns fits the measured caption column, which is what makes the single row list self-enforcing. | --- +### src/ui/jobs_view_state_test.go + +**Package:** `ui` + +Tests `jobsViewState`, the Jobs tab's model: the job/runtime snapshot, the +folder filter, and the ID-based selection. No Fyne app is built — the state +touches no widgets, so these run in milliseconds. + +| Test | Purpose | +|------|---------| +| `TestJobsViewStateSelectsTheFirstJob` | Verifies the opening state selects the first row, so the details pane is never blank when there is something to show. | +| `TestJobsViewStateEmptyListSelectsNothing` | Verifies an empty job list leaves nothing selected and no row to highlight (`displayRow` = -1). | +| `TestJobsViewStateSelectionFollowsTheJobNotTheRow` | Regression guard: a job removed above the selected one (through the Service, the way an external change reaches the view) must not slide the selection onto its neighbour — the selection is a job ID, and only its row moves. | +| `TestJobsViewStateDropsSelectionWhenItsJobIsGone` | Verifies a selection whose job no longer exists falls back to the first visible row instead of describing whichever job inherited its position. | +| `TestJobsViewStateApplyFilter` | Verifies the folder filter keeps a selection it still shows, moves it to the folder's first row when it does not, and that "No folder" matches the job without one. | +| `TestJobsViewStateEmptyFilterSelectsNothing` | Verifies a filter matching no job is a filter choice, not an error state: nothing selected, nothing highlighted, and the selection returns when the filter is cleared. | +| `TestJobsViewStateHiddenSelectionIsNotHighlighted` | Verifies a selected job the filter hides reports no display row rather than falling back to row 0, which would highlight an unrelated job. | +| `TestJobsViewStateRuntimeIsNeverNil` | Verifies `runtime` returns an empty `JobRuntime` for a job the Service has none for, so callers need no nil check. | +| `TestJobsViewStateJobAtRejectsRowsOutsideTheFilter` | Verifies row lookups are bounded by the filtered rows, which is what the list widget draws from. | + +--- + ### src/ui/history_view_test.go **Package:** `ui` diff --git a/src/ui/jobs_view.go b/src/ui/jobs_view.go index f309694..ae47102 100644 --- a/src/ui/jobs_view.go +++ b/src/ui/jobs_view.go @@ -1,8 +1,6 @@ package ui import ( - "fmt" - "gitea.mixdep.ru/mix/gosentry/src/app" "gitea.mixdep.ru/mix/gosentry/src/domain" @@ -22,353 +20,185 @@ const noFolder = "No folder" // view; this panel is a quick at-a-glance summary anchored below the output. const maxJobActivityRows = 3 +// jobsView owns the Jobs tab: the widgets, the view-only preferences they draw +// (list mode and the scheduler pause label), and the jobsViewState the widgets +// read. It replaces a single constructor whose dozen closures shared seven +// mutable locals — the state each handler touches is now named on the struct +// rather than captured, and the invariants that used to be maintained by hand in +// five places live on jobsViewState. +type jobsView struct { + w fyne.Window + svc *app.Service + state *jobsViewState + dp *detailsPanel + + list *widget.List + folderSelect *widget.Select + viewButton *widget.Button + stopAllButton *widget.Button + schedulerState *widget.Label + + // listView and paused mirror Service-owned config so the widgets can be + // relabelled without a round trip. Both are re-read from the Service on every + // refresh; neither is a source of truth. + listView domain.JobListView + paused bool +} + // newJobsView builds the Jobs tab: list sidebar, details panel, and toolbar. // It returns the assembled panel and a refresh function the caller invokes // whenever the service state may have changed (e.g., from the event subscriber // in mainwindow.go). The refresh function re-reads the service snapshot and // redraws all widgets in the jobs view; it does NOT touch history or settings. func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { - jobs := svc.Jobs() - runtimes := make(map[int]*domain.JobRuntime, len(jobs)) - syncFromService := func() { - jobs = svc.Jobs() - for id := range runtimes { - delete(runtimes, id) - } - for _, current := range jobs { - if rt := svc.Runtime(current.ID); rt != nil { - runtimes[current.ID] = rt - } - } + config := svc.Config() + v := &jobsView{ + w: w, + svc: svc, + state: newJobsViewState(svc), + listView: config.JobListView, + paused: config.Paused, } - syncFromService() - runtimeFor := func(index int) *domain.JobRuntime { - if index < 0 || index >= len(jobs) { - return &domain.JobRuntime{} - } - if rt := runtimes[jobs[index].ID]; rt != nil { - return rt - } - return &domain.JobRuntime{} + v.dp = newDetailsPanel(job{}, &domain.JobRuntime{}, config.OverlapPolicy, config.DefaultTimeoutSeconds) + v.updateDetails() + + // Build order follows what refresh() touches: the folder select fires its + // OnChanged from SetSelected below, which refreshes, so every widget that + // refresh() reaches has to exist by then. + v.list = v.newList() + v.viewButton = v.newViewToggle() + globalControls := v.newGlobalControls() + v.folderSelect = v.newFolderSelect() + v.folderSelect.SetSelected(v.state.folder) + v.syncListSelection() + + return v.assemble(globalControls), v.refresh +} + +// refresh re-reads the Service and redraws the whole view. It is the single +// entry point for "something changed": the toolbar handlers call it after a +// successful operation, and mainwindow's event observer calls it for everything +// else. +func (v *jobsView) refresh() { + v.state.sync() + // The pause state is Service-owned and can change from outside this view, so + // it is re-read here rather than mirrored from the tap handler alone — that is + // what makes this view a consumer of SchedulerStateChanged. + v.applySchedulerState(v.svc.Config().Paused) + v.updateDetails() + v.dp.logs.Refresh() + v.list.Refresh() + v.syncListSelection() +} + +// updateDetails repopulates the details pane from the current selection. +func (v *jobsView) updateDetails() { + current, ok := v.state.selected() + if !ok { + // A folder filter can temporarily leave no selectable rows. Clearing the + // details panel avoids showing stale information for a hidden job. + v.dp.clear() + return } + // Overlap policy and the default timeout are global settings that can change + // from the Settings tab while this view is open, so they are re-read on every + // update rather than captured once at construction. + config := v.svc.Config() + v.dp.update(current, v.state.runtime(current.ID), config.OverlapPolicy, config.DefaultTimeoutSeconds) +} - selected := 0 - if len(jobs) == 0 { - selected = -1 +// syncListSelection points the list's highlight at the selected job. It is what +// keeps the highlight and the details pane describing the same job when the row +// a job sits in moves — a job created or deleted above it, a folder filter +// applied, or a different jobs file adopted. widget.List.Select returns early +// when the row is already highlighted, so calling this on every refresh does not +// fight the user's scrolling. +func (v *jobsView) syncListSelection() { + row := v.state.displayRow() + if row < 0 { + v.list.UnselectAll() + return } - selectedFolder := allFolders - initialConfig := svc.Config() - schedulerPaused := initialConfig.Paused - listView := initialConfig.JobListView - filteredJobs := filteredJobIndexes(jobs, selectedFolder) + v.list.Select(row) +} - dp := newDetailsPanel(job{}, &domain.JobRuntime{}, initialConfig.OverlapPolicy, initialConfig.DefaultTimeoutSeconds) - if selected >= 0 { - dp.update(jobs[selected], runtimeFor(selected), initialConfig.OverlapPolicy, initialConfig.DefaultTimeoutSeconds) - } else { - dp.clear() - } +// rebuildFolders re-derives the folder filter's options from the current jobs. +// Creating, editing, and deleting a job can all add or remove a folder. +func (v *jobsView) rebuildFolders() { + v.folderSelect.Options = folderOptions(v.state.jobs) + v.folderSelect.Refresh() +} - updateDetails := func(index int) { - if index < 0 || index >= len(jobs) { - // A folder filter can temporarily leave no selectable rows. Clearing - // the details panel avoids showing stale information for a hidden job. - dp.clear() - return - } - selected = index - // Overlap policy and the default timeout are global settings that can - // change from the Settings tab while this view is open, so they are - // re-read on every update rather than captured once at construction. - config := svc.Config() - dp.update(jobs[selected], runtimeFor(selected), config.OverlapPolicy, config.DefaultTimeoutSeconds) - } - - // list, folderSelect, and applySchedulerState are declared early so closures - // below can reference them before the widgets that assign the values exist. - var list *widget.List - var folderSelect *widget.Select - var applySchedulerState func(bool) - - refreshView := func() { - syncFromService() - if applySchedulerState != nil { - // The pause state is Service-owned and can change from outside this - // view (Settings has no such control today, but the event that - // reports it — SchedulerStateChanged — is consumed here rather than - // relying solely on the tap handler's own mirror). - applySchedulerState(svc.Config().Paused) - } - filteredJobs = filteredJobIndexes(jobs, selectedFolder) - updateDetails(selected) - dp.logs.Refresh() - if list != nil { - list.Refresh() - } - } - - // applyRowMode expresses the current view mode as visibility on the row's - // four labels. widget.List caches the row template's MinSize, and - // list.Refresh() re-creates the template and recomputes it, so hiding lines - // is what actually shrinks the rows: layout.NewCustomPaddedVBoxLayout and the - // border layout both skip hidden children when measuring. - applyRowMode := func(inlineStatus, meta, status fyne.CanvasObject) { - if listView.IsCompact() { - inlineStatus.Show() - meta.Hide() - status.Hide() - return - } - inlineStatus.Hide() - meta.Show() - status.Show() - } - - list = widget.NewList( - func() int { return len(filteredJobs) }, - func() fyne.CanvasObject { - name := widget.NewLabelWithStyle("Job name", fyne.TextAlignLeading, fyne.TextStyle{Bold: true}) - // Truncating stops a long name from pushing the compact row's status - // off the right-hand edge. Labels default to TextWrapOff, which grows - // the widget to fit instead. - name.Truncation = fyne.TextTruncateClip - inlineStatus := widget.NewLabel("status") - meta := widget.NewLabel("schedule") - status := widget.NewLabel("status") - applyRowMode(inlineStatus, meta, status) - nameLine := container.NewBorder(nil, nil, nil, inlineStatus, name) - return container.New(layout.NewCustomPaddedVBoxLayout(rowOverlap()), nameLine, meta, status) - }, - func(id widget.ListItemID, item fyne.CanvasObject) { - row := item.(*fyne.Container) - // NewBorder keeps the center object first and appends the border slots - // after it, so nameLine is [name, inlineStatus]. - nameLine := row.Objects[0].(*fyne.Container) - name := nameLine.Objects[0].(*widget.Label) - inlineStatus := nameLine.Objects[1].(*widget.Label) - meta := row.Objects[1].(*widget.Label) - status := row.Objects[2].(*widget.Label) - - current := jobs[filteredJobs[id]] - name.SetText(current.Name) - // Keep each row compact: folder, schedule, and command are shown in one - // metadata line so the left pane stays useful even with many jobs. - meta.SetText(app.DisplayFolder(current.Folder) + " " + current.Schedule + " " + app.DisplayInvocation(current)) - statusText := app.StatusText(current, runtimes[current.ID]) - status.SetText(statusText) - inlineStatus.SetText(statusText) - // A full Refresh reuses rows built under the previous mode, so - // visibility cannot be left to the create callback alone. - applyRowMode(inlineStatus, meta, status) - }, - ) - list.OnSelected = func(id widget.ListItemID) { - if id < 0 || id >= len(filteredJobs) { - updateDetails(-1) - return - } - updateDetails(filteredJobs[id]) - } - if len(filteredJobs) > 0 && selected >= 0 { - list.Select(app.DisplayIndex(filteredJobs, selected)) - } - - folderSelect = widget.NewSelect(folderOptions(jobs), func(value string) { - if value == "" { - return - } - selectedFolder = value - filteredJobs = filteredJobIndexes(jobs, selectedFolder) - if len(filteredJobs) == 0 { - // The "No folder" filter is intentionally allowed to be empty. It is a - // real filter choice, not an error state, so the selection is cleared. - // This path returns without reaching refreshView(), so it is the one - // place the list has to be redrawn by hand. - selected = -1 - updateDetails(-1) - list.Refresh() - return - } - selected = filteredJobs[0] - list.Select(0) - refreshView() - }) - folderSelect.SetSelected(selectedFolder) - - // viewToggleIcon pairs with viewToggleText: both name the action the button - // performs, not the state it is in, matching stopAllButton's convention. - viewToggleIcon := func(current domain.JobListView) fyne.Resource { - if current.IsCompact() { - return theme.ViewFullScreenIcon() - } - return theme.ListIcon() - } - viewButton := widget.NewButtonWithIcon(viewToggleText(listView), viewToggleIcon(listView), nil) - viewButton.OnTapped = func() { - next := nextJobListView(listView) - listView = next - if err := svc.SetJobListView(next); err != nil { - // Roll the mode back and leave the button as it was, so the button - // never claims a preference that did not reach disk. - listView = nextJobListView(next) - dialog.ShowError(err, w) - return - } - viewButton.SetText(viewToggleText(listView)) - viewButton.SetIcon(viewToggleIcon(listView)) - // Refresh re-creates the row template, which is what recomputes the - // cached row height for the new mode. Selection is untouched. - list.Refresh() - } - - addButton := widget.NewButtonWithIcon("New job", theme.ContentAddIcon(), func() { - showJobDialog(w, "New job", job{Schedule: "@every 1m", Command: "echo GoSentry job ran", Enabled: true}, func(saved job) { - created, err := svc.CreateJob(saved) - if err != nil { - dialog.ShowError(err, w) - return - } - syncFromService() - folderSelect.Options = folderOptions(jobs) - folderSelect.Refresh() - targetFolder := filterValue(created.Folder) - if selectedFolder != allFolders && selectedFolder != targetFolder { - selectedFolder = targetFolder - folderSelect.SetSelected(targetFolder) - } - selected = indexOfID(jobs, created.ID) - filteredJobs = filteredJobIndexes(jobs, selectedFolder) - list.Select(app.DisplayIndex(filteredJobs, selected)) - refreshView() - }) - }) - editButton := widget.NewButtonWithIcon("Edit", theme.DocumentCreateIcon(), func() { - if selected < 0 || selected >= len(jobs) { - return - } - showJobDialog(w, "Edit job", jobs[selected], func(saved job) { - saved.ID = jobs[selected].ID - if err := svc.UpdateJob(saved); err != nil { - dialog.ShowError(err, w) - return - } - syncFromService() - folderSelect.Options = folderOptions(jobs) - folderSelect.Refresh() - refreshView() - }) - }) - runButton := widget.NewButtonWithIcon("Run now", theme.MediaPlayIcon(), func() { - if selected < 0 || selected >= len(jobs) { - return - } - // A manual run is allowed even while the scheduler is paused: pause only - // stops automatic scheduled runs, not the user's explicit "Run now". - if err := svc.RunNow(jobs[selected].ID); err != nil { - dialog.ShowError(err, w) - return - } - refreshView() - }) - - schedulerState := widget.NewLabel("") - stopAllButton := widget.NewButtonWithIcon("", nil, nil) - // applySchedulerState is the one place that draws the pause control and its - // status text from a pause value, so refreshView can drive it from whatever - // the Service reports (including a SchedulerStateChanged the general refresh - // picks up) instead of only the tap handler mirroring its own toggle. - applySchedulerState = func(paused bool) { - schedulerPaused = paused - if paused { - schedulerState.SetText("Scheduler paused") - stopAllButton.SetText("Enable auto") - stopAllButton.SetIcon(theme.MediaPlayIcon()) - } else { - schedulerState.SetText("Scheduler running") - stopAllButton.SetText("Disable auto") - stopAllButton.SetIcon(theme.MediaPauseIcon()) - } - } - applySchedulerState(schedulerPaused) - stopAllButton.OnTapped = func() { - // SetGlobalPause flips the pause flag, updates every job's next-run text, - // and emits the activity record the observer logs. Revert if the save fails. - if err := svc.SetGlobalPause(!schedulerPaused); err != nil { - dialog.ShowError(err, w) - return - } - // refreshView re-derives the pause state from the Service (see - // applySchedulerState above), so it is the single place that draws it. - refreshView() - } - pauseButton := widget.NewButtonWithIcon("Pause", theme.MediaPauseIcon(), func() { - if selected < 0 || selected >= len(jobs) { - return - } - current := jobs[selected] - if err := svc.SetEnabled(current.ID, !current.Enabled); err != nil { - dialog.ShowError(err, w) - return - } - refreshView() - }) - deleteButton := widget.NewButtonWithIcon("Delete", theme.DeleteIcon(), func() { - if selected < 0 || selected >= len(jobs) { - return - } - deleted := jobs[selected] - // Deletion is confirmed because jobs can represent real system actions. - // There is no undo yet, so accidental removal should require one more click. - dialog.ShowConfirm("Delete job", fmt.Sprintf("Delete %q?", deleted.Name), func(confirm bool) { - if !confirm { - return - } - if err := svc.DeleteJob(deleted.ID); err != nil { - dialog.ShowError(err, w) - return - } - syncFromService() - folderSelect.Options = folderOptions(jobs) - folderSelect.Refresh() - filteredJobs = filteredJobIndexes(jobs, selectedFolder) - if len(filteredJobs) == 0 && selectedFolder != allFolders { - selectedFolder = allFolders - folderSelect.SetSelected(allFolders) - filteredJobs = filteredJobIndexes(jobs, selectedFolder) - } - if len(filteredJobs) == 0 { - selected = -1 - } else { - selected = filteredJobs[0] - } - if selected >= 0 { - list.Select(app.DisplayIndex(filteredJobs, selected)) - } - refreshView() - }, w) - }) - - toolbar := container.NewHBox(addButton, editButton, runButton, pauseButton, deleteButton, layout.NewSpacer()) - // The row sits directly under the tab bar with no AppTabs inset, while the - // default VBox gap below it is one theme padding — add the same on top so - // the button is not flush against the tabs. - globalControls := container.New( - layout.NewCustomPaddedLayout(theme.Padding(), 0, 0, 0), - container.NewHBox(stopAllButton, schedulerState, layout.NewSpacer()), - ) +// assemble puts the sidebar (global controls, folder filter, toolbar, list) and +// the details pane into the master/detail split the tab shows. +func (v *jobsView) assemble(globalControls fyne.CanvasObject) fyne.CanvasObject { // The whole filter is one row: caption on the left, view toggle on the right, // select filling what is left. The border layout gives both edges their // MinSize, so the header is a line shorter than a stacked caption would make it. folderCaption := widget.NewLabelWithStyle("Folder", fyne.TextAlignLeading, fyne.TextStyle{Bold: true}) - filterRow := container.NewBorder(nil, nil, folderCaption, viewButton, folderSelect) - sidebarHeader := container.NewVBox(globalControls, widget.NewSeparator(), filterRow, toolbar) - sidebar := container.NewBorder(sidebarHeader, nil, nil, nil, list) + filterRow := container.NewBorder(nil, nil, folderCaption, v.viewButton, v.folderSelect) + sidebarHeader := container.NewVBox(globalControls, widget.NewSeparator(), filterRow, v.newToolbar()) + sidebar := container.NewBorder(sidebarHeader, nil, nil, nil, v.list) // A split rather than a Border left slot: the border pinned the sidebar at its // MinSize forever, so the user could never trade list width for detail width. // The divider lets either pane grow, and neither can be dragged below its own // content minimum. - panel := container.NewHSplit(sidebar, container.NewPadded(dp.container())) + panel := container.NewHSplit(sidebar, container.NewPadded(v.dp.container())) panel.SetOffset(initialSplitOffset(sidebar.MinSize().Width)) - return panel, refreshView + return panel +} + +// newFolderSelect builds the folder filter. Selecting a folder narrows the list +// and, when the selected job is no longer visible, moves the selection to the +// first row that is (see jobsViewState.applyFilter). +func (v *jobsView) newFolderSelect() *widget.Select { + return widget.NewSelect(folderOptions(v.state.jobs), func(value string) { + if value == "" { + return + } + v.state.applyFilter(value) + v.refresh() + }) +} + +// newGlobalControls builds the pause control row that sits above the filter. +func (v *jobsView) newGlobalControls() fyne.CanvasObject { + v.schedulerState = widget.NewLabel("") + v.stopAllButton = widget.NewButtonWithIcon("", nil, nil) + v.applySchedulerState(v.paused) + v.stopAllButton.OnTapped = func() { + // SetGlobalPause flips the pause flag, updates every job's next-run text, + // and emits the activity record the observer logs. refresh re-derives the + // pause state from the Service, so a failed save leaves the control showing + // what actually happened. + if err := v.svc.SetGlobalPause(!v.paused); err != nil { + dialog.ShowError(err, v.w) + return + } + v.refresh() + } + // The row sits directly under the tab bar with no AppTabs inset, while the + // default VBox gap below it is one theme padding — add the same on top so + // the button is not flush against the tabs. + return container.New( + layout.NewCustomPaddedLayout(theme.Padding(), 0, 0, 0), + container.NewHBox(v.stopAllButton, v.schedulerState, layout.NewSpacer()), + ) +} + +// applySchedulerState is the one place that draws the pause control and its +// status text from a pause value, so refresh can drive it from whatever the +// Service reports instead of only the tap handler mirroring its own toggle. +func (v *jobsView) applySchedulerState(paused bool) { + v.paused = paused + if paused { + v.schedulerState.SetText("Scheduler paused") + v.stopAllButton.SetText("Enable auto") + v.stopAllButton.SetIcon(theme.MediaPlayIcon()) + return + } + v.schedulerState.SetText("Scheduler running") + v.stopAllButton.SetText("Disable auto") + v.stopAllButton.SetIcon(theme.MediaPauseIcon()) } diff --git a/src/ui/jobs_view_list.go b/src/ui/jobs_view_list.go new file mode 100644 index 0000000..00a7704 --- /dev/null +++ b/src/ui/jobs_view_list.go @@ -0,0 +1,114 @@ +package ui + +import ( + "gitea.mixdep.ru/mix/gosentry/src/app" + "gitea.mixdep.ru/mix/gosentry/src/domain" + + "fyne.io/fyne/v2" + "fyne.io/fyne/v2/container" + "fyne.io/fyne/v2/dialog" + "fyne.io/fyne/v2/layout" + "fyne.io/fyne/v2/theme" + "fyne.io/fyne/v2/widget" +) + +// newList builds the sidebar's job list. Rows are drawn from jobsViewState's +// filtered view, so the row index the widget reports is a position in the +// filter, never an index into the job snapshot. +func (v *jobsView) newList() *widget.List { + list := widget.NewList( + func() int { return len(v.state.filtered) }, + func() fyne.CanvasObject { + name := widget.NewLabelWithStyle("Job name", fyne.TextAlignLeading, fyne.TextStyle{Bold: true}) + // Truncating stops a long name from pushing the compact row's status + // off the right-hand edge. Labels default to TextWrapOff, which grows + // the widget to fit instead. + name.Truncation = fyne.TextTruncateClip + inlineStatus := widget.NewLabel("status") + meta := widget.NewLabel("schedule") + status := widget.NewLabel("status") + v.applyRowMode(inlineStatus, meta, status) + nameLine := container.NewBorder(nil, nil, nil, inlineStatus, name) + return container.New(layout.NewCustomPaddedVBoxLayout(rowOverlap()), nameLine, meta, status) + }, + func(id widget.ListItemID, item fyne.CanvasObject) { + current, ok := v.state.jobAt(int(id)) + if !ok { + return + } + row := item.(*fyne.Container) + // NewBorder keeps the center object first and appends the border slots + // after it, so nameLine is [name, inlineStatus]. + nameLine := row.Objects[0].(*fyne.Container) + name := nameLine.Objects[0].(*widget.Label) + inlineStatus := nameLine.Objects[1].(*widget.Label) + meta := row.Objects[1].(*widget.Label) + status := row.Objects[2].(*widget.Label) + + name.SetText(current.Name) + // Keep each row compact: folder, schedule, and command are shown in one + // metadata line so the left pane stays useful even with many jobs. + meta.SetText(app.DisplayFolder(current.Folder) + " " + current.Schedule + " " + app.DisplayInvocation(current)) + statusText := app.StatusText(current, v.state.runtime(current.ID)) + status.SetText(statusText) + inlineStatus.SetText(statusText) + // A full Refresh reuses rows built under the previous mode, so + // visibility cannot be left to the create callback alone. + v.applyRowMode(inlineStatus, meta, status) + }, + ) + list.OnSelected = func(id widget.ListItemID) { + v.state.selectRow(int(id)) + v.updateDetails() + } + return list +} + +// applyRowMode expresses the current view mode as visibility on the row's +// four labels. widget.List caches the row template's MinSize, and +// list.Refresh() re-creates the template and recomputes it, so hiding lines +// is what actually shrinks the rows: layout.NewCustomPaddedVBoxLayout and the +// border layout both skip hidden children when measuring. +func (v *jobsView) applyRowMode(inlineStatus, meta, status fyne.CanvasObject) { + if v.listView.IsCompact() { + inlineStatus.Show() + meta.Hide() + status.Hide() + return + } + inlineStatus.Hide() + meta.Show() + status.Show() +} + +// newViewToggle builds the compact/detailed switch that sits at the right edge +// of the filter row. +func (v *jobsView) newViewToggle() *widget.Button { + button := widget.NewButtonWithIcon(viewToggleText(v.listView), viewToggleIcon(v.listView), nil) + button.OnTapped = func() { + next := nextJobListView(v.listView) + v.listView = next + if err := v.svc.SetJobListView(next); err != nil { + // Roll the mode back and leave the button as it was, so the button + // never claims a preference that did not reach disk. + v.listView = nextJobListView(next) + dialog.ShowError(err, v.w) + return + } + button.SetText(viewToggleText(v.listView)) + button.SetIcon(viewToggleIcon(v.listView)) + // Refresh re-creates the row template, which is what recomputes the + // cached row height for the new mode. Selection is untouched. + v.list.Refresh() + } + return button +} + +// viewToggleIcon pairs with viewToggleText: both name the action the button +// performs, not the state it is in, matching stopAllButton's convention. +func viewToggleIcon(current domain.JobListView) fyne.Resource { + if current.IsCompact() { + return theme.ViewFullScreenIcon() + } + return theme.ListIcon() +} diff --git a/src/ui/jobs_view_state.go b/src/ui/jobs_view_state.go new file mode 100644 index 0000000..76eca05 --- /dev/null +++ b/src/ui/jobs_view_state.go @@ -0,0 +1,156 @@ +package ui + +import ( + "gitea.mixdep.ru/mix/gosentry/src/app" + "gitea.mixdep.ru/mix/gosentry/src/domain" +) + +// jobsViewState is the model behind the Jobs tab: the snapshot of the Service's +// jobs and runtimes, the folder filter, and the selection. The widgets in +// jobsView read it and never keep a second copy of any of it. +// +// The selection is a job ID, not an index into the snapshot. Every path that +// changes the job list replaces that snapshot underneath the view — create, +// delete, and edit do it from this view's own handlers, but adopting a different +// jobs file does it from the Service, and the view only learns about it through +// the refresh that JobsLoaded triggers. An index that outlives its snapshot then +// points at whichever job happens to sit there now, so the details pane +// describes one job while the list highlights another. Indexes are derived from +// the ID at render time instead (selectedIndex, displayRow). +type jobsViewState struct { + svc *app.Service + jobs []job + runtimes map[int]*domain.JobRuntime + folder string + // selectedID is 0 when nothing is selected; job IDs start at 1. + selectedID int + // filtered holds the indexes into jobs that the folder filter shows, in list + // row order: filtered[row] is the index of the job drawn in that row. + filtered []int +} + +func newJobsViewState(svc *app.Service) *jobsViewState { + s := &jobsViewState{ + svc: svc, + runtimes: map[int]*domain.JobRuntime{}, + folder: allFolders, + } + s.sync() + return s +} + +// sync re-reads the Service snapshot, re-applies the folder filter, and +// re-resolves the selection against the new list. It is the only place the view +// reads job state from the Service. +func (s *jobsViewState) sync() { + s.jobs = s.svc.Jobs() + clear(s.runtimes) + for _, current := range s.jobs { + if rt := s.svc.Runtime(current.ID); rt != nil { + s.runtimes[current.ID] = rt + } + } + s.filtered = filteredJobIndexes(s.jobs, s.folder) + s.resolveSelection() +} + +// applyFilter switches the folder filter, keeping the current selection when the +// new filter still shows it. A filter that matches nothing — "No folder" with no +// such job — is a real filter choice, not an error state, so it simply leaves +// nothing selected. +func (s *jobsViewState) applyFilter(folder string) { + s.folder = folder + s.filtered = filteredJobIndexes(s.jobs, s.folder) + if !s.visible(s.selectedID) { + s.selectedID = 0 + } + s.resolveSelection() +} + +// resolveSelection drops a selection whose job is gone and falls back to the +// first visible row, so the details pane never describes a job the current +// snapshot no longer holds. +func (s *jobsViewState) resolveSelection() { + if s.selectedID != 0 && indexOfID(s.jobs, s.selectedID) < 0 { + s.selectedID = 0 + } + if s.selectedID == 0 && len(s.filtered) > 0 { + s.selectedID = s.jobs[s.filtered[0]].ID + } +} + +// selectByID records the selection directly, for handlers that know the job they +// want selected (a newly created job, for instance) rather than its row. +func (s *jobsViewState) selectByID(id int) { + s.selectedID = id +} + +// selectRow records the selection from a list row, which is what widget.List +// reports through OnSelected. +func (s *jobsViewState) selectRow(row int) { + current, ok := s.jobAt(row) + if !ok { + s.selectedID = 0 + return + } + s.selectedID = current.ID +} + +// selected returns the selected job, or false when nothing is selected. +func (s *jobsViewState) selected() (job, bool) { + index := s.selectedIndex() + if index < 0 { + return job{}, false + } + return s.jobs[index], true +} + +// selectedIndex resolves the selected ID to an index into the current snapshot, +// or -1 when nothing is selected. +func (s *jobsViewState) selectedIndex() int { + if s.selectedID == 0 { + return -1 + } + return indexOfID(s.jobs, s.selectedID) +} + +// displayRow maps the selection onto a list row, or -1 when nothing is selected +// or the filter hides the selected job — so a caller unselects rather than +// highlighting an unrelated row. +func (s *jobsViewState) displayRow() int { + index := s.selectedIndex() + if index < 0 || !s.visible(s.selectedID) { + return -1 + } + return app.DisplayIndex(s.filtered, index) +} + +// jobAt returns the job drawn in the given list row. +func (s *jobsViewState) jobAt(row int) (job, bool) { + if row < 0 || row >= len(s.filtered) { + return job{}, false + } + return s.jobs[s.filtered[row]], true +} + +// runtime returns a job's runtime, or an empty one when the Service has none +// yet, so callers can read it without a nil check. +func (s *jobsViewState) runtime(id int) *domain.JobRuntime { + if rt := s.runtimes[id]; rt != nil { + return rt + } + return &domain.JobRuntime{} +} + +// visible reports whether the folder filter shows the given job. +func (s *jobsViewState) visible(id int) bool { + if id == 0 { + return false + } + for _, index := range s.filtered { + if s.jobs[index].ID == id { + return true + } + } + return false +} diff --git a/src/ui/jobs_view_state_test.go b/src/ui/jobs_view_state_test.go new file mode 100644 index 0000000..729378f --- /dev/null +++ b/src/ui/jobs_view_state_test.go @@ -0,0 +1,198 @@ +package ui + +import ( + "testing" + + "gitea.mixdep.ru/mix/gosentry/src/app" + "gitea.mixdep.ru/mix/gosentry/src/domain" +) + +// newStateForTest builds a jobsViewState over a Service holding the given jobs. +// No Fyne app is needed: the state is the view's model and touches no widgets. +func newStateForTest(t *testing.T, jobs []domain.Job) (*jobsViewState, *app.Service) { + t.Helper() + svc := app.NewService(newTestStore(t), jobs) + t.Cleanup(svc.Stop) + return newJobsViewState(svc), svc +} + +func threeJobs() []domain.Job { + return []domain.Job{ + {ID: 1, Name: "First", Folder: "Maintenance", Schedule: "@every 1m", Command: "echo one", Enabled: true}, + {ID: 2, Name: "Second", Schedule: "@every 2m", Command: "echo two", Enabled: true}, + {ID: 3, Name: "Third", Folder: "Reports", Schedule: "@every 3m", Command: "echo three", Enabled: true}, + } +} + +func selectedName(t *testing.T, s *jobsViewState) string { + t.Helper() + current, ok := s.selected() + if !ok { + return "" + } + return current.Name +} + +// TestJobsViewStateSelectsTheFirstJob pins the opening state: the first row is +// selected so the details pane is never blank when there is something to show. +func TestJobsViewStateSelectsTheFirstJob(t *testing.T) { + s, _ := newStateForTest(t, threeJobs()) + if got := selectedName(t, s); got != "First" { + t.Errorf("selected job = %q, want %q", got, "First") + } + if got := s.displayRow(); got != 0 { + t.Errorf("displayRow = %d, want 0", got) + } +} + +func TestJobsViewStateEmptyListSelectsNothing(t *testing.T) { + s, _ := newStateForTest(t, nil) + if _, ok := s.selected(); ok { + t.Error("an empty job list should leave nothing selected") + } + if got := s.displayRow(); got != -1 { + t.Errorf("displayRow with nothing selected = %d, want -1", got) + } +} + +// TestJobsViewStateSelectionFollowsTheJobNotTheRow is the regression guard for +// the selection defect: the selection is a job ID, so a job removed above the +// selected one must not slide the selection onto its neighbour. The deletion +// goes through the Service rather than the Delete button, which is how the view +// learns about a job list that changed underneath it (a different jobs file +// adopted, or any other broad JobChanged). +func TestJobsViewStateSelectionFollowsTheJobNotTheRow(t *testing.T) { + s, svc := newStateForTest(t, threeJobs()) + + s.selectRow(2) + if got := selectedName(t, s); got != "Third" { + t.Fatalf("selected job after selecting row 2 = %q, want %q", got, "Third") + } + + if err := svc.DeleteJob(1); err != nil { + t.Fatalf("DeleteJob: %v", err) + } + s.sync() + + if got := selectedName(t, s); got != "Third" { + t.Errorf("selected job after the first job was removed = %q, want it still on %q", got, "Third") + } + if got := s.displayRow(); got != 1 { + t.Errorf("displayRow = %d, want the row %q moved to (1)", got, "Third") + } +} + +// TestJobsViewStateDropsSelectionWhenItsJobIsGone covers the other half: a +// selected job that no longer exists falls back to the first visible row instead +// of describing whichever job inherited its position. +func TestJobsViewStateDropsSelectionWhenItsJobIsGone(t *testing.T) { + s, svc := newStateForTest(t, threeJobs()) + + s.selectRow(1) + if err := svc.DeleteJob(2); err != nil { + t.Fatalf("DeleteJob: %v", err) + } + s.sync() + + if got := selectedName(t, s); got != "First" { + t.Errorf("selected job after deleting the selected one = %q, want the fallback %q", got, "First") + } +} + +func TestJobsViewStateApplyFilter(t *testing.T) { + s, _ := newStateForTest(t, threeJobs()) + + // The selected job is in the folder being filtered to, so it stays selected. + s.selectRow(2) + s.applyFilter("Reports") + if got := selectedName(t, s); got != "Third" { + t.Errorf("selection after filtering to its own folder = %q, want %q", got, "Third") + } + if got := s.displayRow(); got != 0 { + t.Errorf("displayRow inside the filter = %d, want 0", got) + } + + // Filtering to a folder that hides it moves the selection to the first row + // that folder does show. + s.applyFilter("Maintenance") + if got := selectedName(t, s); got != "First" { + t.Errorf("selection after filtering it away = %q, want %q", got, "First") + } + + // "No folder" matches the one job without one. + s.applyFilter(noFolder) + if got := selectedName(t, s); got != "Second" { + t.Errorf("selection under the %q filter = %q, want %q", noFolder, got, "Second") + } + + s.applyFilter(allFolders) + if got := len(s.filtered); got != 3 { + t.Errorf("rows under %q = %d, want 3", allFolders, got) + } +} + +// TestJobsViewStateEmptyFilterSelectsNothing pins that a filter matching no job +// is a filter choice, not an error state: nothing is selected, and nothing is +// highlighted either. +func TestJobsViewStateEmptyFilterSelectsNothing(t *testing.T) { + s, _ := newStateForTest(t, []domain.Job{ + {ID: 1, Name: "First", Folder: "Maintenance", Schedule: "@every 1m", Command: "echo one", Enabled: true}, + }) + + s.applyFilter(noFolder) + if _, ok := s.selected(); ok { + t.Error("a filter that matches nothing should leave nothing selected") + } + if got := s.displayRow(); got != -1 { + t.Errorf("displayRow under an empty filter = %d, want -1", got) + } + + // The selection comes back when the filter does. + s.applyFilter(allFolders) + if got := selectedName(t, s); got != "First" { + t.Errorf("selection after clearing the filter = %q, want %q", got, "First") + } +} + +// TestJobsViewStateHiddenSelectionIsNotHighlighted covers the case the list +// widget cannot express: the selected job still exists but the filter hides it, +// so there is no row to highlight and displayRow must say so rather than fall +// back to row 0. +func TestJobsViewStateHiddenSelectionIsNotHighlighted(t *testing.T) { + s, _ := newStateForTest(t, threeJobs()) + + s.applyFilter("Maintenance") + // Selecting by ID is how the create handler points the view at a job it just + // made; here it reaches the state a hidden-but-selected job would be in. + s.selectByID(3) + if s.visible(3) { + t.Fatal("job 3 should be hidden by the Maintenance filter") + } + if got := s.displayRow(); got != -1 { + t.Errorf("displayRow for a hidden selection = %d, want -1", got) + } + if got := selectedName(t, s); got != "Third" { + t.Errorf("selected job = %q, want it still %q", got, "Third") + } +} + +func TestJobsViewStateRuntimeIsNeverNil(t *testing.T) { + s, _ := newStateForTest(t, threeJobs()) + if rt := s.runtime(99); rt == nil { + t.Error("runtime for an unknown job returned nil, want an empty runtime") + } +} + +func TestJobsViewStateJobAtRejectsRowsOutsideTheFilter(t *testing.T) { + s, _ := newStateForTest(t, threeJobs()) + s.applyFilter("Reports") + if current, ok := s.jobAt(0); !ok || current.Name != "Third" { + t.Errorf("jobAt(0) = (%q, %v), want (%q, true)", current.Name, ok, "Third") + } + if _, ok := s.jobAt(1); ok { + t.Error("jobAt past the last filtered row should report no job") + } + if _, ok := s.jobAt(-1); ok { + t.Error("jobAt(-1) should report no job") + } +} diff --git a/src/ui/jobs_view_test.go b/src/ui/jobs_view_test.go index 981ff56..9592db4 100644 --- a/src/ui/jobs_view_test.go +++ b/src/ui/jobs_view_test.go @@ -1,6 +1,9 @@ package ui import ( + "encoding/json" + "os" + "path/filepath" "testing" "gitea.mixdep.ru/mix/gosentry/src/app" @@ -378,7 +381,7 @@ func TestJobsSplitOpensAtTheSidebarWidth(t *testing.T) { // TestToolbarButtonRedrawsRowAndDetails is the regression guard for F12: the // toolbar handlers no longer re-read the service or refresh the list -// themselves, so refreshView alone has to re-snapshot the jobs and repopulate +// themselves, so jobsView.refresh alone has to re-snapshot the jobs and repopulate // the details pane. If it ever stops doing either, the row renders a stale // status and the details lose the selection — neither is a compile error. func TestToolbarButtonRedrawsRowAndDetails(t *testing.T) { @@ -441,6 +444,78 @@ func TestToolbarButtonRedrawsRowAndDetails(t *testing.T) { } } +// TestJobsViewSelectionSurvivesAJobsFileSwitch is the view-level regression +// guard for the selection defect. Adopting a different jobs file replaces the +// whole list from the Service; the view only hears about it through the refresh +// that JobsLoaded triggers, which is exactly what this test calls. With the +// selection held as a row index, that refresh redrew the details pane from the +// old index — describing whichever job now sat there, or clearing the pane when +// the new list was shorter — while the list's highlight stayed where it was. +func TestJobsViewSelectionSurvivesAJobsFileSwitch(t *testing.T) { + testApp := test.NewApp() + defer testApp.Quit() + w := testApp.NewWindow("test") + defer w.Close() + + store := newTestStore(t) + svc := app.NewService(store, []domain.Job{ + {ID: 1, Name: "First", Schedule: "@every 1m", Command: "echo one", Enabled: true}, + {ID: 2, Name: "Second", Schedule: "@every 2m", Command: "echo two", Enabled: true}, + {ID: 3, Name: "Third", Schedule: "@every 3m", Command: "echo three", Enabled: true}, + }) + defer svc.Stop() + + content, refresh := newJobsView(w, svc) + w.SetContent(content) + + list := jobsList(t, content) + list.Select(2) + if got := jobsDetailsTitle(t, content); got != "Third" { + t.Fatalf("details title after selecting row 2 = %q, want %q", got, "Third") + } + + // A second jobs file with different jobs and different IDs, so nothing about + // the old selection can resolve into the new list. + other := []domain.Job{ + {ID: 10, Name: "Alpha", Schedule: "@every 1m", Command: "echo alpha", Enabled: true}, + {ID: 11, Name: "Beta", Schedule: "@every 2m", Command: "echo beta", Enabled: true}, + } + payload, err := json.Marshal(domain.JobsFile{Jobs: other}) + if err != nil { + t.Fatalf("marshal jobs: %v", err) + } + if err := os.WriteFile(filepath.Join(store.Paths.AppDir, "other.json"), payload, 0o644); err != nil { + t.Fatalf("write jobs file: %v", err) + } + config := svc.Config() + config.JobsFile = "other.json" + if err := svc.UpdateSettings(config); err != nil { + t.Fatalf("UpdateSettings: %v", err) + } + refresh() + + if got := jobsDetailsTitle(t, content); got != "Alpha" { + t.Errorf("details title after the switch = %q, want the first job of the new file %q", got, "Alpha") + } + if got := list.Length(); got != len(other) { + t.Fatalf("list length after the switch = %d, want %d", got, len(other)) + } + // widget.List.Select returns without calling OnSelected when the row is + // already highlighted, so a silent Select(0) is what proves the highlight and + // the details pane are describing the same job. + reselected := false + inner := list.OnSelected + list.OnSelected = func(id widget.ListItemID) { + reselected = true + inner(id) + } + defer func() { list.OnSelected = inner }() + list.Select(0) + if reselected { + t.Error("row 0 was not the highlighted row after the switch, so the highlight and the details pane disagree") + } +} + // TestDetailCaptionWidthCoversEveryCaption is the guard that makes the single // metadataRows list self-enforcing (F10): every caption it returns must // measure no wider than captionColumnWidth's result for that same list, or a diff --git a/src/ui/jobs_view_toolbar.go b/src/ui/jobs_view_toolbar.go new file mode 100644 index 0000000..a05bbbc --- /dev/null +++ b/src/ui/jobs_view_toolbar.go @@ -0,0 +1,136 @@ +package ui + +import ( + "fmt" + + "fyne.io/fyne/v2" + "fyne.io/fyne/v2/container" + "fyne.io/fyne/v2/dialog" + "fyne.io/fyne/v2/layout" + "fyne.io/fyne/v2/theme" + "fyne.io/fyne/v2/widget" +) + +// newToolbar builds the per-job button row under the folder filter. Every +// handler works from the selected job — never from a row index — and ends in +// refresh, which is what re-reads the Service and redraws the row, the details +// pane, and the list highlight. +func (v *jobsView) newToolbar() fyne.CanvasObject { + return container.NewHBox( + v.newAddButton(), + v.newEditButton(), + v.newRunButton(), + v.newPauseButton(), + v.newDeleteButton(), + layout.NewSpacer(), + ) +} + +func (v *jobsView) newAddButton() *widget.Button { + return widget.NewButtonWithIcon("New job", theme.ContentAddIcon(), func() { + blank := job{Schedule: "@every 1m", Command: "echo GoSentry job ran", Enabled: true} + showJobDialog(v.w, "New job", blank, func(saved job) { + created, err := v.svc.CreateJob(saved) + if err != nil { + dialog.ShowError(err, v.w) + return + } + v.state.sync() + // The new job may have introduced a folder, so the options are rebuilt + // before the filter is pointed at it. + v.rebuildFolders() + v.state.selectByID(created.ID) + if target := filterValue(created.Folder); v.state.folder != allFolders && v.state.folder != target { + // The current filter would hide the job the user just created. Switch + // to its folder; SetSelected fires OnChanged, which applies the filter + // and refreshes. + v.folderSelect.SetSelected(target) + } + v.refresh() + }) + }) +} + +func (v *jobsView) newEditButton() *widget.Button { + return widget.NewButtonWithIcon("Edit", theme.DocumentCreateIcon(), func() { + current, ok := v.state.selected() + if !ok { + return + } + showJobDialog(v.w, "Edit job", current, func(saved job) { + // The ID comes from the job the dialog was opened on, so a list that + // changed underneath the open dialog cannot redirect the save. + saved.ID = current.ID + if err := v.svc.UpdateJob(saved); err != nil { + dialog.ShowError(err, v.w) + return + } + v.state.sync() + // An edit can rename the job's folder, add a new one, or empty the last + // job out of an existing one. + v.rebuildFolders() + v.refresh() + }) + }) +} + +func (v *jobsView) newRunButton() *widget.Button { + return widget.NewButtonWithIcon("Run now", theme.MediaPlayIcon(), func() { + current, ok := v.state.selected() + if !ok { + return + } + // A manual run is allowed even while the scheduler is paused: pause only + // stops automatic scheduled runs, not the user's explicit "Run now". + if err := v.svc.RunNow(current.ID); err != nil { + dialog.ShowError(err, v.w) + return + } + v.refresh() + }) +} + +func (v *jobsView) newPauseButton() *widget.Button { + return widget.NewButtonWithIcon("Pause", theme.MediaPauseIcon(), func() { + current, ok := v.state.selected() + if !ok { + return + } + if err := v.svc.SetEnabled(current.ID, !current.Enabled); err != nil { + dialog.ShowError(err, v.w) + return + } + v.refresh() + }) +} + +func (v *jobsView) newDeleteButton() *widget.Button { + return widget.NewButtonWithIcon("Delete", theme.DeleteIcon(), func() { + deleted, ok := v.state.selected() + if !ok { + return + } + // Deletion is confirmed because jobs can represent real system actions. + // There is no undo yet, so accidental removal should require one more click. + dialog.ShowConfirm("Delete job", fmt.Sprintf("Delete %q?", deleted.Name), func(confirm bool) { + if !confirm { + return + } + if err := v.svc.DeleteJob(deleted.ID); err != nil { + dialog.ShowError(err, v.w) + return + } + // sync drops the deleted job's selection and falls back to the first row + // the filter still shows. + v.state.sync() + v.rebuildFolders() + if len(v.state.filtered) == 0 && v.state.folder != allFolders { + // The deleted job was the last one in its folder, and that folder is + // no longer an option. Fall back to "All" rather than leaving the user + // on an empty filter they did not choose. + v.folderSelect.SetSelected(allFolders) + } + v.refresh() + }, v.w) + }) +}