Skip to content

refactor: resolve the project through selectors.ResolveProject - #667

Open
NickJosevski wants to merge 1 commit into
nj/channel-list-view-deletefrom
nj/selectors-resolve-project
Open

refactor: resolve the project through selectors.ResolveProject#667
NickJosevski wants to merge 1 commit into
nj/channel-list-view-deletefrom
nj/selectors-resolve-project

Conversation

@NickJosevski

Copy link
Copy Markdown
Contributor

Nine commands hand-rolled the same block: prompt for the project when it wasn't named, look it up when it was, error out in automation mode. It was copy-paste, not parallel evolution — the comment // project name is already provided, fetch the object because it's needed for further questions appeared verbatim 8 times.

They now call selectors.ResolveProject, which #609 adds next to the Project/FindProject primitives it composes.

release/{create,deploy,delete,list}, runbook/{list,run}, runbook/snapshot/list, channel/{create,list} — 9 files, -109 net lines.

Behaviour is unchanged

  • Commands that echo Project <name> still do, on the same condition — the project came from the command line, so no prompt was shown.
  • Automation-mode validation still runs before any API call. release delete in particular must reject a missing --version before looking the project up; its existing test asserts no project request is made, and caught me when I first got this wrong.

Not adopted: release-progression allow / prevent

These are the only two remaining selectors.Project callers. They have no lookup branch at all — when --project is supplied, selectedProject stays nil. Adopting the helper would change behaviour rather than just deduplicate, so they're left alone.

They're also independently broken. allow.go dereferences that nil:

if opts.Project.Value == "" {
    selectedProject, err = selectors.Project(...)   // only assigned when empty
}
opts.Project.Value = selectedProject.GetName()      // panics when --project was given

PromptMissing runs whenever !opts.NoPrompt, so octopus release-progression allow --project X panics interactively. prevent.go has the same missing branch and passes a nil project to SelectRelease. Confirmed (*projects.Project)(nil).GetName() panics. Worth its own fix + regression tests rather than being smuggled into a refactor.

Note on base

Based on nj/channel-list-view-delete (#609), which adds the helper. Retarget to main once that merges.

Nine commands hand-rolled the same "prompt for the project when it wasn't
named, look it up when it was, error out in automation mode" block. It was
copy-paste rather than parallel evolution — the comment "project name is
already provided, fetch the object because it's needed for further questions"
appeared verbatim eight times.

They now call selectors.ResolveProject. Behaviour is unchanged: callers that
echo the resolved project still do so on the same condition (the project came
from the command line, so no prompt was shown), and automation-mode validation
still runs before any API call — release delete in particular must reject a
missing --version before looking the project up.

release-progression allow and prevent are deliberately left alone. They have no
lookup branch at all, so adopting the helper there would change behaviour
rather than just deduplicate; they are separately buggy and want their own fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant