Skip to content

feat: remove visibility flag and warn on deprecated flow endpoint#918

Open
mbevc1 wants to merge 1 commit into
mainfrom
20260529_remove_visibility
Open

feat: remove visibility flag and warn on deprecated flow endpoint#918
mbevc1 wants to merge 1 commit into
mainfrom
20260529_remove_visibility

Conversation

@mbevc1
Copy link
Copy Markdown
Contributor

@mbevc1 mbevc1 commented May 29, 2026

The legacy PUT /api/v2/flows/{org} endpoint is deprecated. The --visibility flag was only meaningful for that endpoint, so remove it from create flow. Emit a deprecation warning when neither --template-file nor --use-empty-template is provided, since that path still uses the deprecated endpoint.

Fixes: https://github.com/kosli-dev/server/issues/5790

The legacy PUT /api/v2/flows/{org} endpoint is deprecated. The
`--visibility` flag was only meaningful for that endpoint, so remove it
from `create flow`. Emit a deprecation warning when neither
`--template-file` nor `--use-empty-template` is provided, since that path
still uses the deprecated endpoint.
@mbevc1 mbevc1 added customer-request For issues reported/inspired by customers - solution reported via community slack and to customer tech-debt go Pull requests that update go code labels May 29, 2026
@mbevc1 mbevc1 enabled auto-merge (squash) May 29, 2026 15:48
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 29, 2026

Claude finished @mbevc1's task in 1m 37s —— View job


PR Review: Remove visibility flag and warn on deprecated flow endpoint

  • Run static analysis (build, vet, mod tidy)
  • Read and review changed files
  • Post review feedback

Static Analysis

All checks pass: go build, go vet, go mod tidy — no issues.

Review Summary

Clean, well-scoped PR. The --visibility flag is removed from create flow, the Visibility field is dropped from FlowPayload, and a deprecation warning is emitted when the legacy PUT /api/v2/flows/{org} endpoint is still used (i.e. neither --template-file nor --use-empty-template is provided). Tests are updated to match.

Findings:

  1. Good: CreateFlow helper now uses --use-empty-template (testHelpers.go:345) — this means other test suites that call CreateFlow() no longer hit the deprecated endpoint, avoiding noisy warnings in test output. Thoughtful change.

  2. Minor: goldenRegex anchor removed — In the "fails when name is invalid" test case (createFlow_test.go:40), the ^ anchor was dropped from "^Error: .*foo bar""Error: .*foo bar". This is correct because the deprecation warning now precedes the error text, so the match can no longer start at the beginning of the output. Good catch.

  3. Note: visibility still appears in read-only commandsgetFlow.go:110 and listFlows.go:81,90 still display a visibility column from the API response. This is fine — the API still returns it; this PR only removes the ability to set it via create flow. Just noting for awareness in case follow-up cleanup is planned.

  4. No breaking change for existing users — Users who pass --visibility to create flow will now get an "unknown flag" error. Since the flag was only meaningful for the deprecated endpoint, this is the correct behavior, but worth mentioning in release notes.

Overall: Looks good to merge. The change is minimal, correct, and well-tested. No bugs, security issues, or performance concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

customer-request For issues reported/inspired by customers - solution reported via community slack and to customer go Pull requests that update go code tech-debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant