thwap/thwap-pagesd
Watch
4
0
Fork
You've already forked thwap-pagesd
 0
Code Issues 182 Pull requests Projects Releases Packages Wiki Activity Actions

feat(tpagectl): implement site update subcommand #558

Merged
fuzzy merged 2 commits from feat/cli-site-update into main 2026-08-06 01:07:32 +00:00
Conversation 1 Commits 2 Files changed 11 +586 -54
automation commented 2026-08-06 00:59:57 +00:00
Owner
Copy link

What

Implements site update end to end: the daemon's missing PATCH /api/v1/sites/{name} endpoint plus the tpagectl site update subcommand. This completes the Phase 2 site command group.

Daemon

  • UpdateSiteRequest (pointer fields for partial updates: visibility, custom domain, preview-enabled) + SiteService.Update — 404 for unknown sites, validates changed fields, re-creates the site Ingress via CreateSiteIngress when the custom domain changes, persists via sites.Update.
  • Validation helpers extracted from validateRequest (validateVisibility, validateCustomDomain) and reused.
  • PATCH /api/v1/sites/{name} handler: owner-scoped (403), invalid body/validation (400), 404, 200 with the updated site.
  • Tests: service partial updates, ingress re-created on domain change, validation, not-found; handler 200/400/403/404.

CLI

  • site update SITE with optional --visibility, --domain (empty clears), --preview-enabled (IsSet tri-state) — #188/#189.
  • Fetches the current site, issues the PATCH, and prints a before/after diff of changed fields — #191.
  • Client-side validation mirroring the daemon; 404 surfaces as a friendly site "X" not found.
  • Tests: PATCH body, diff output, clear-domain, --preview-enabled=false, validation, nothing-to-update, 404, missing arg.
  • All site subcommands are now implemented — the stub errNotImplemented sentinel and its test are removed.

Why

Phase 2 roadmap task #192; the final site command.

Testing

  • Daemon: service update tests (custom domain re-ingress, visibility/preview, not-found, invalid visibility/domain), handler 200/400/403/404 tests
  • CLI: PATCH body + diff, clear domain, preview disabled, validation, nothing-to-update, 404, missing arg
  • go test -race ./... passes (17 packages)
  • golangci-lint run clean
  • pre-commit hooks pass (gofmt, go mod tidy, go test)

Breaking Changes

None. SiteService gains Update; Site semantics unchanged.

Notes

Visibility updates are record-only: the gitea SDK's EditRepoOption can only express a binary Private flag and cannot represent limited repo visibility, so the backing repository's visibility is not changed. Custom domain changes do re-provision the Ingress so the new host serves the site.

Closes #188
Closes #189
Closes #190
Closes #191
Closes #192

## What Implements `site update` end to end: the daemon's missing `PATCH /api/v1/sites/{name}` endpoint plus the tpagectl `site update` subcommand. This completes the Phase 2 site command group. **Daemon** - `UpdateSiteRequest` (pointer fields for partial updates: visibility, custom domain, preview-enabled) + `SiteService.Update` — 404 for unknown sites, validates changed fields, re-creates the site Ingress via `CreateSiteIngress` when the custom domain changes, persists via `sites.Update`. - Validation helpers extracted from `validateRequest` (`validateVisibility`, `validateCustomDomain`) and reused. - `PATCH /api/v1/sites/{name}` handler: owner-scoped (403), invalid body/validation (400), 404, 200 with the updated site. - Tests: service partial updates, ingress re-created on domain change, validation, not-found; handler 200/400/403/404. **CLI** - `site update SITE` with optional `--visibility`, `--domain` (empty clears), `--preview-enabled` (`IsSet` tri-state) — #188/#189. - Fetches the current site, issues the PATCH, and prints a **before/after diff** of changed fields — #191. - Client-side validation mirroring the daemon; 404 surfaces as a friendly `site "X" not found`. - Tests: PATCH body, diff output, clear-domain, `--preview-enabled=false`, validation, nothing-to-update, 404, missing arg. - All site subcommands are now implemented — the stub `errNotImplemented` sentinel and its test are removed. ## Why Phase 2 roadmap task #192; the final site command. ## Testing - [x] Daemon: service update tests (custom domain re-ingress, visibility/preview, not-found, invalid visibility/domain), handler 200/400/403/404 tests - [x] CLI: PATCH body + diff, clear domain, preview disabled, validation, nothing-to-update, 404, missing arg - [x] `go test -race ./...` passes (17 packages) - [x] `golangci-lint run` clean - [x] pre-commit hooks pass (gofmt, go mod tidy, go test) ## Breaking Changes None. `SiteService` gains `Update`; `Site` semantics unchanged. ## Notes Visibility updates are **record-only**: the gitea SDK's `EditRepoOption` can only express a binary `Private` flag and cannot represent `limited` repo visibility, so the backing repository's visibility is not changed. Custom domain changes do re-provision the Ingress so the new host serves the site. Closes #188 Closes #189 Closes #190 Closes #191 Closes #192
Add the daemon PATCH /api/v1/sites/{name} endpoint and the tpagectl site
update subcommand.

Daemon: SiteService.Update applies partial updates to visibility, custom
domain, and preview-enabled, re-creating the site Ingress when the custom
domain changes. The handler is owner-scoped and returns 200 with the
updated site, 400 on validation, 403 when unauthorized, 404 when missing.
Visibility updates are record-only (the gitea SDK cannot express 'limited'
repo visibility).

CLI: site update takes a required site name and optional
--visibility/--domain/--preview-enabled flags, fetches the current site,
issues the PATCH, and prints a before/after diff of the changed fields.
A 404 surfaces as a friendly not-found error. All site subcommands are now
implemented, so the stub sentinel and its test are removed.
docs(roadmap): mark site update subcommand complete
All checks were successful
Test and Release / lint (pull_request) Successful in 7m18s
Details
Test and Release / test (pull_request) Successful in 7m31s
Details
69e6b5dbd4 
Record completion of the tpagectl site update subcommand and its daemon
endpoint: positional site name, visibility/domain/preview-enabled
updates, PATCH /api/v1/sites/{name}, and before/after diff display. This
completes the Phase 2 site command group.

closes #188
closes #189
closes #190
closes #191
closes #192
fuzzy scheduled this pull request to auto merge when all checks succeed 2026-08-06 01:01:29 +00:00
the.auditor approved these changes 2026-08-06 01:04:17 +00:00
the.auditor left a comment
Copy link

Summary

site update: daemon PATCH /api/v1/sites/{name} + tpagectl site update, completing the Phase 2 site command group. Verified: go test -race ./... passes (17 packages) and golangci-lint run ./... is clean. Closes #188-192.

Verified behavior: updateRequest mirrors UpdateSiteRequest (pointer fields, identical JSON keys); CreateSiteIngress is create-or-update (TestCreateSiteIngressUpdate), so domain changes upsert the site's single ingress in place — no orphaned old-domain ingress.

No blocking issues.

Suggestions (filed as issues)

  1. internal/site/service.go:456 – visibility updates are record-only: the Forgejo repo's actual visibility is not changed (SDK EditRepoOption can't express limited), so site update --visibility private reports success while repo access is unchanged. Filed as #559
  2. internal/site/service.go:477 – ingress is re-provisioned on any CustomDomain != nil, even a no-op domain change. Filed as #560
  3. internal/tpagectl/site/update.go – friendlyNotFound helper is extracted but get.go/delete.go still inline the same 404 mapping. Filed as #561

Praise

  • Pointer-field partial-update request (nil = unchanged) is the right PATCH design
  • Clean validation-helper extraction (validateVisibility/validateCustomDomain) reused by register
  • IsSet tri-state handling for --preview-enabled is correct, including --preview-enabled=false
  • Before/after diff with clear-domain handling and a (no changes) case
  • errNotImplemented removal tidily completes the group
  • Strong daemon + CLI coverage (PATCH body, diff, clear-domain, disabled previews, validation, 400/403/404)
## Summary `site update`: daemon `PATCH /api/v1/sites/{name}` + tpagectl `site update`, completing the Phase 2 site command group. Verified: `go test -race ./...` passes (17 packages) and `golangci-lint run ./...` is clean. Closes #188-192. Verified behavior: `updateRequest` mirrors `UpdateSiteRequest` (pointer fields, identical JSON keys); `CreateSiteIngress` is create-or-update (`TestCreateSiteIngressUpdate`), so domain changes upsert the site's single ingress in place — no orphaned old-domain ingress. No blocking issues. ## Suggestions (filed as issues) 1. `internal/site/service.go:456` – visibility updates are record-only: the Forgejo repo's actual visibility is not changed (SDK `EditRepoOption` can't express `limited`), so `site update --visibility private` reports success while repo access is unchanged. Filed as #559 2. `internal/site/service.go:477` – ingress is re-provisioned on any `CustomDomain != nil`, even a no-op domain change. Filed as #560 3. `internal/tpagectl/site/update.go` – `friendlyNotFound` helper is extracted but `get.go`/`delete.go` still inline the same 404 mapping. Filed as #561 ## Praise - Pointer-field partial-update request (nil = unchanged) is the right PATCH design - Clean validation-helper extraction (`validateVisibility`/`validateCustomDomain`) reused by register - `IsSet` tri-state handling for `--preview-enabled` is correct, including `--preview-enabled=false` - Before/after diff with clear-domain handling and a `(no changes)` case - `errNotImplemented` removal tidily completes the group - Strong daemon + CLI coverage (PATCH body, diff, clear-domain, disabled previews, validation, 400/403/404)
fuzzy merged commit 69e6b5dbd4 into main 2026-08-06 01:07:32 +00:00
fuzzy deleted branch feat/cli-site-update 2026-08-06 01:07:32 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
the.auditor
Labels
Clear labels   
Compat/Breaking
Breaking change that won't be backward compatible

  
Kind/Bug
Something is not working

  
Kind/Documentation
Documentation changes

  
Kind/Enhancement
Improve existing functionality

  
Kind/Feature
New functionality

  
Kind/Security
This is security issue

  
Kind/Testing
Issue or pull request related to testing

  
Priority
Critical
 The priority is critical

  
Priority
High
 The priority is high

  
Priority
Low
 The priority is low

  
Priority
Medium
 The priority is medium

  
Reviewed
Confirmed
 Issue has been confirmed

  
Reviewed
Duplicate
 This issue or pull request already exists

  
Reviewed
Invalid
 Invalid issue

  
Reviewed
Won't Fix
 This issue won't be fixed

  
Status
Abandoned
 Somebody has started to work on this but abandoned work

  
Status
Blocked
 Something is blocking this issue or pull request

  
Status
Need More Info
 Feedback is required to reproduce issue or to continue work

No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
thwap/thwap-pagesd!558
No description provided.