From fc19678727a6c9e709526ad61ec0e69f0c28437b Mon Sep 17 00:00:00 2001 From: Vassiliy Yegorov Date: Sun, 5 Jul 2026 15:03:14 +0700 Subject: [PATCH 1/4] docs: plan for per-record apply selection + deletes-first order Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BwxdSt4reTm7Dj1oxRvpP3 --- .../plans/2026-07-05-selective-apply-order.md | 219 ++++++++++++++++++ 1 file changed, 219 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-05-selective-apply-order.md diff --git a/docs/superpowers/plans/2026-07-05-selective-apply-order.md b/docs/superpowers/plans/2026-07-05-selective-apply-order.md new file mode 100644 index 0000000..58de106 --- /dev/null +++ b/docs/superpowers/plans/2026-07-05-selective-apply-order.md @@ -0,0 +1,219 @@ +# Пер-записевый выбор в Apply + порядок «удаления раньше обновлений» Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: superpowers:subagent-driven-development. Steps use `- [ ]`. + +**Goal:** (1) Дать выбирать чекбоксами конкретные записи для применения (updates и prunes), а не «всё или ничего». (2) Применять удаления (prunes) ДО обновлений (updates), иначе провайдер отвергает конфликтующие изменения (нельзя создать CNAME на имени, где ещё живёт A-запись). + +**Architecture:** `ApplyRequest` из двух булевых превращается в два списка выбранных ключей записей. Каждая запись в diff-ответе получает стабильный `key` (нормализованный `ТИП имя.`), которым оперируют чекбоксы фронта и по которому бэк фильтрует. `service.Apply` собирает выбранные prunes, затем выбранные updates — провайдер применяет удаления первыми. + +**Tech Stack:** Go (diff/service/api), React + Vite. + +## Global Constraints + +- Ключ записи: `RecordDiff.Key()` — нормализованный `ТИП имя.` (через `model.Record.Key()` по Type+Name). Фронт НЕ конструирует ключ сам — берёт `key` из ответа diff и возвращает его в Apply. +- Порядок применения — ИНВАРИАНТ: выбранные prunes (Delete) добавляются в набор ДО выбранных updates (Add/Update). Не опция. +- Только actionable-записи выбираемы; read-only (NS/SOA) чекбоксов не имеют и никогда не применяются. +- Multi-tenancy/IDOR: Apply уже скоуплен по projectID (resolve по pid) — не регрессировать. +- Комментарии в Go — на английском; в web — как в окружающих файлах. TDD. НЕ коммитить реальную сборку `internal/web/dist/*`. + +--- + +### Task 1: Backend — ключи записей + выборочный Apply с порядком deletes-first + +**Files:** +- Modify: `internal/diff/diff.go` (RecordDiff.Key), `internal/api/dto.go` (recordView.Key + applyRequest), `internal/service/service.go` (ApplyRequest + Apply), `internal/api/handlers.go` (handleApply маппинг) +- Test: `internal/diff/diff_test.go`, `internal/service/service_test.go`, `internal/api/*_test.go` + +**Interfaces:** +- Produces: `diff.RecordDiff.Key() string`; `recordView.Key`; `service.ApplyRequest{Updates []string, Prunes []string}`. +- Consumes: `model.Record.Key()`; `Changeset.Updates()/Prunes()`. + +- [ ] **Step 1: Тест RecordDiff.Key** + +`internal/diff/diff_test.go`: +```go +func TestRecordDiffKeyNormalizes(t *testing.T) { + d := RecordDiff{Kind: Delete, Type: model.A, Name: "Mail.Example.COM"} + if got := d.Key(); got != "A mail.example.com." { + t.Fatalf("key: %q", got) + } +} +``` +Run: `go test ./internal/diff/... -run RecordDiffKey` — Ожидание FAIL. + +- [ ] **Step 2: RecordDiff.Key** + +`internal/diff/diff.go`: +```go +// Key is the stable identifier of the RRset this diff targets, normalised the +// same way as model.Record.Key ("TYPE name."). Used to select individual diffs +// for a partial apply. Works for every Kind (Delete has no Desired, Add has no +// Actual) because Type/Name are always populated. +func (d RecordDiff) Key() string { + return model.Record{Type: d.Type, Name: d.Name}.Key() +} +``` +Run: `go test ./internal/diff/... -run RecordDiffKey` — Ожидание PASS. + +- [ ] **Step 3: recordView.Key + applyRequest** + +`internal/api/dto.go`: +```go +type recordView struct { + Key string `json:"key"` + Kind string `json:"kind"` + Type string `json:"type"` + Name string `json:"name"` + Desired []string `json:"desired,omitempty"` + Actual []string `json:"actual,omitempty"` + ReadOnly bool `json:"readOnly"` +} +``` +`toRecordView` — установить `Key: d.Key()` (первым полем). +```go +type applyRequest struct { + Updates []string `json:"updates"` + Prunes []string `json:"prunes"` +} +``` +(убрать старые ApplyUpdates/ApplyPrunes bool.) + +- [ ] **Step 4: service.ApplyRequest + deletes-first selective** + +`internal/service/service.go`: +```go +type ApplyRequest struct { + Updates []string // record keys (RecordDiff.Key) to add/update + Prunes []string // record keys to delete +} +``` +`Apply`: +```go +func (s *DomainService) Apply(ctx context.Context, projectID, domainID uuid.UUID, req ApplyRequest) (diff.Changeset, error) { + p, creds, ref, cs, err := s.resolve(ctx, projectID, domainID) + if err != nil { + return diff.Changeset{}, err + } + selPrunes := toSet(req.Prunes) + selUpdates := toSet(req.Updates) + var toApply []diff.RecordDiff + // Deletes first: the provider rejects an Add/Update whose name still has a + // conflicting record (e.g. a CNAME cannot be created while an A on the same + // name exists). Pruning the old records before applying updates avoids that. + for _, d := range cs.Prunes() { + if selPrunes[d.Key()] { + toApply = append(toApply, d) + } + } + for _, d := range cs.Updates() { + if selUpdates[d.Key()] { + toApply = append(toApply, d) + } + } + applied := diff.Changeset{Diffs: toApply} + if len(toApply) > 0 { + if err := p.ApplyChanges(ctx, creds, ref.ZoneID, applied); err != nil { + return diff.Changeset{}, err + } + } + return applied, nil +} + +func toSet(keys []string) map[string]bool { + m := make(map[string]bool, len(keys)) + for _, k := range keys { + m[k] = true + } + return m +} +``` +(`ApplyChanges` итерирует `cs.Diffs` в порядке слайса — сверено; порядок toApply сохраняется, prunes применяются первыми.) + +- [ ] **Step 5: handleApply маппинг** + +`internal/api/handlers.go` `handleApply`: +```go +cs, err := a.Svc.Apply(r.Context(), pid, did, service.ApplyRequest{ + Updates: req.Updates, Prunes: req.Prunes, +}) +``` +(пустое тело → пустые списки → ничего не применяется; сохранить существующую обработку EOF/битого JSON.) + +- [ ] **Step 6: Тесты** + +- `service_test.go`: changeset с 1 update + 1 prune; `Apply{Prunes:[pruneKey]}` → применён только prune; `Apply{Updates:[updKey]}` → только update; `Apply{Updates:[updKey], Prunes:[pruneKey]}` → **порядок toApply: prune ПЕРВЫМ, update вторым** (проверь через фейковый провайдер, записывающий порядок полученных Diffs — критический тест конфликта CNAME/A). Невыбранные ключи не применяются. +- `api_test.go`: POST `/apply` с `{"prunes":["A gitlocator.com."]}` → Svc.Apply получил Prunes с этим ключом, Updates пусто. + +- [ ] **Step 7: Прогон и коммит** + +Run: `go build ./... && go test ./internal/...`. Ожидание PASS. +```bash +git add internal/ +git commit -m "feat(apply): per-record selection + deletes-before-updates ordering" +``` + +--- + +### Task 2: Frontend — чекбоксы на записях, выборочный Apply + +**Files:** +- Modify: `web/src/api/types.ts`, `web/src/api/client.ts`, `web/src/hooks/useApi.ts`, `web/src/components/DiffView.tsx`, `web/src/pages/DomainDiffPage.tsx` +- Test: `web/src/components/DiffView.test.tsx`, `web/src/pages/DomainDiffPage.test.tsx` + +**Interfaces:** +- Consumes: `recordView.key`; `POST /apply {updates:[], prunes:[]}`. +- Produces: чекбокс на каждой update/prune записи; выбор → Apply шлёт ключи; deletes-first обеспечен бэком. + +- [ ] **Step 1: types + client + hooks** + +`types.ts`: `RecordView += key: string`; `ApplyRequest` → `{ updates: string[]; prunes: string[] }`. +`client.ts` `applyDomain(projectId, id, body: ApplyRequest)` — тело `{updates, prunes}` (уже сериализует body). +`useApi.ts` `useApplyDomain` — тип body обновится; onSuccess как есть (инвалидация check). + +- [ ] **Step 2: DiffView — чекбоксы + select-all** + +`DiffView.tsx`: сделать секции Updates/Prunes выбираемыми. Расширить проп: +```tsx +export function DiffView({ + changeset, + selectedUpdates, selectedPrunes, // Set + onToggleUpdate, onTogglePrune, // (key: string) => void + onToggleAllUpdates, onToggleAllPrunes, // (checked: boolean) => void + footerExtra, +}: { ... }) +``` +- В `Section` для tone `update`/`delete` — чекбокс в заголовке секции (select-all: отмечен если все выбраны, indeterminate если часть) и чекбокс в каждой `RecordRow` (отмечен по `selected.has(record.key)`). Для tone `readonly` — без чекбоксов (не выбираемо). +- Используй существующий `Checkbox` из `@/components/ui/checkbox`. Ключ строки — `record.key`. +- Сохрани визуальный стиль; чекбокс слева от badge типа, выравнивание аккуратное. + +- [ ] **Step 3: DomainDiffPage — состояние выбора + Apply** + +`DomainDiffPage.tsx`: +- Состояние: `selectedUpdates: Set`, `selectedPrunes: Set`. +- При загрузке changeset — инициализировать: `selectedUpdates` = все ключи updates (default on), `selectedPrunes` = пусто (default off, удаление opt-in). Пересчитывать при смене changeset (useEffect на changeset). +- Тогглы: add/remove ключ; select-all: заполнить/очистить набор ключей секции. +- Убрать старый общий `applyPrunes` чекбокс и `pruneWarning`, завязанный на него. Вместо — предупреждение, если `selectedPrunes.size > 0`: «Будет удалено записей: N. Действие необратимо.» +- Apply: `apply.mutate({ updates: [...selectedUpdates], prunes: [...selectedPrunes] })`. +- Кнопка Apply disabled, если `selectedUpdates.size + selectedPrunes.size === 0`; текст статуса «Готово к применению» / «Изменений для применения нет». + +- [ ] **Step 4: Тесты** + +- `DiffView.test.tsx`: чекбоксы рендерятся для update/prune записей, не для read-only; клик по чекбоксу вызывает onToggle с `record.key`; select-all в заголовке. +- `DomainDiffPage.test.tsx`: по умолчанию updates отмечены, prunes сняты; отметка prune → предупреждение о количестве; Apply шлёт выбранные `{updates:[...], prunes:[...]}`; снятие всех → Apply disabled. + +- [ ] **Step 5: Прогон и коммит** + +Run: `cd web && npm run test -- --run && npx tsc --noEmit && npm run build`. +```bash +cd .. && git checkout internal/web/dist/index.html +git add web/src/ +git commit -m "feat(web): per-record apply checkboxes with select-all; prune opt-in" +``` + +--- + +## Итоговая проверка + +- `go build ./... && go test ./...` — PASS. +- `cd web && npm run test -- --run && npm run build` — PASS. +- Ручная: на домене с конфликтом (A `mail` + желаемый CNAME `mail`) отметить prune A `mail` и update CNAME `mail`, Apply → удаление проходит раньше, CNAME создаётся без ошибки провайдера; чекбоксы позволяют применить подмножество записей. From 0b26923586fc99da9195bb3e9396378528715ba1 Mon Sep 17 00:00:00 2001 From: Vassiliy Yegorov Date: Sun, 5 Jul 2026 15:10:01 +0700 Subject: [PATCH 2/4] feat(apply): per-record selection + deletes-before-updates ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RecordDiff.Key() gives a stable normalized identifier ("TYPE name.") for every diff kind, exposed as recordView.Key. ApplyRequest now takes Updates/Prunes key lists instead of two booleans, so callers can apply a subset of records. service.Apply builds the applied set with selected prunes (Delete) added before selected updates (Add/Update) — an invariant, not an option — since the provider rejects an Add/Update whose name still conflicts with an existing record (e.g. a CNAME cannot be created while an A on the same name still exists). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BwxdSt4reTm7Dj1oxRvpP3 --- internal/api/api_test.go | 15 ++++--- internal/api/dto.go | 7 ++-- internal/api/handlers.go | 7 ++-- internal/diff/diff.go | 8 ++++ internal/diff/diff_test.go | 10 +++++ internal/service/service.go | 33 ++++++++++++---- internal/service/service_test.go | 68 +++++++++++++++++++++++--------- 7 files changed, 110 insertions(+), 38 deletions(-) diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 75f5e9b..89b3e27 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -84,12 +84,15 @@ func TestCheckEndpoint(t *testing.T) { } } -func TestApplyDefaultsPruneFalse(t *testing.T) { +// TestApplySendsSelectedKeys covers the per-record selection request shape: +// POST /apply with only "prunes" set must reach the service with that key in +// Prunes and an empty Updates. +func TestApplySendsSelectedKeys(t *testing.T) { a, m := newTestAPI() router := NewRouter(a) did := uuid.New().String() - body := `{"applyUpdates":true}` // applyPrunes отсутствует → false + body := `{"prunes":["A gitlocator.com."]}` req := requestWithSessionCookie(http.MethodPost, "/api/v1/projects/00000000-0000-0000-0000-000000000002/domains/"+did+"/apply", strings.NewReader(body)) @@ -99,7 +102,7 @@ func TestApplyDefaultsPruneFalse(t *testing.T) { if w.Code != http.StatusOK { t.Fatalf("status %d body %s", w.Code, w.Body.String()) } - if m.lastReq.ApplyPrunes != false || m.lastReq.ApplyUpdates != true { + if len(m.lastReq.Prunes) != 1 || m.lastReq.Prunes[0] != "A gitlocator.com." || len(m.lastReq.Updates) != 0 { t.Fatalf("apply request mismatch: %+v", m.lastReq) } } @@ -117,8 +120,8 @@ func TestApplyEmptyBodyOK(t *testing.T) { if w.Code != http.StatusOK { t.Fatalf("status %d body %s", w.Code, w.Body.String()) } - if m.lastReq.ApplyPrunes != false { - t.Fatalf("expected ApplyPrunes=false for empty body, got %+v", m.lastReq) + if len(m.lastReq.Updates) != 0 || len(m.lastReq.Prunes) != 0 { + t.Fatalf("expected empty Updates/Prunes for empty body, got %+v", m.lastReq) } } @@ -127,7 +130,7 @@ func TestApplyMalformedBody(t *testing.T) { router := NewRouter(a) did := uuid.New().String() - body := `{"applyUpdates":` + body := `{"updates":` req := requestWithSessionCookie(http.MethodPost, "/api/v1/projects/00000000-0000-0000-0000-000000000002/domains/"+did+"/apply", strings.NewReader(body)) diff --git a/internal/api/dto.go b/internal/api/dto.go index ddef2dd..6d5fa9f 100644 --- a/internal/api/dto.go +++ b/internal/api/dto.go @@ -40,11 +40,12 @@ func toAuthResponse(u store.User, p store.Project) authResponse { } type applyRequest struct { - ApplyUpdates bool `json:"applyUpdates"` - ApplyPrunes bool `json:"applyPrunes"` + Updates []string `json:"updates"` + Prunes []string `json:"prunes"` } type recordView struct { + Key string `json:"key"` Kind string `json:"kind"` Type string `json:"type"` Name string `json:"name"` @@ -61,7 +62,7 @@ type changesetResponse struct { } func toRecordView(d diff.RecordDiff) recordView { - rv := recordView{Kind: string(d.Kind), Type: string(d.Type), Name: d.Name, ReadOnly: d.ReadOnly} + rv := recordView{Key: d.Key(), Kind: string(d.Kind), Type: string(d.Type), Name: d.Name, ReadOnly: d.ReadOnly} if d.Desired != nil { rv.Desired = d.Desired.Values } diff --git a/internal/api/handlers.go b/internal/api/handlers.go index adcc435..a8491de 100644 --- a/internal/api/handlers.go +++ b/internal/api/handlers.go @@ -66,15 +66,16 @@ func (a *API) handleApply(w http.ResponseWriter, r *http.Request) { var req applyRequest if r.Body != nil { r.Body = http.MaxBytesReader(w, r.Body, 1<<20) // 1 MiB - // пустое тело допустимо → значения по умолчанию (prune=false); - // любая другая ошибка decode (битый JSON, неверные типы) → 400 + // пустое тело допустимо → значения по умолчанию (пустые списки, ничего + // не применяется); любая другая ошибка decode (битый JSON, неверные + // типы) → 400 if err := json.NewDecoder(r.Body).Decode(&req); err != nil && !errors.Is(err, io.EOF) { writeErr(w, http.StatusBadRequest, "invalid request body") return } } cs, err := a.Svc.Apply(r.Context(), pid, did, service.ApplyRequest{ - ApplyUpdates: req.ApplyUpdates, ApplyPrunes: req.ApplyPrunes, + Updates: req.Updates, Prunes: req.Prunes, }) if err != nil { log.Printf("api: apply failed: %v", err) diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 78df864..e5dd15c 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -21,6 +21,14 @@ type RecordDiff struct { ReadOnly bool // NS/SOA — shown but never applied } +// Key is the stable identifier of the RRset this diff targets, normalised the +// same way as model.Record.Key ("TYPE name."). Used to select individual diffs +// for a partial apply. Works for every Kind (Delete has no Desired, Add has no +// Actual) because Type/Name are always populated. +func (d RecordDiff) Key() string { + return model.Record{Type: d.Type, Name: d.Name}.Key() +} + type Changeset struct { Diffs []RecordDiff } diff --git a/internal/diff/diff_test.go b/internal/diff/diff_test.go index 6a50200..baf90ac 100644 --- a/internal/diff/diff_test.go +++ b/internal/diff/diff_test.go @@ -22,6 +22,16 @@ func find(cs Changeset, key string) *RecordDiff { return nil } +// TestRecordDiffKeyNormalizes pins RecordDiff.Key() down to the same +// normalization as model.Record.Key() (lowercase + trailing dot), for a +// Delete diff (which has no Desired, only Type/Name populated directly). +func TestRecordDiffKeyNormalizes(t *testing.T) { + d := RecordDiff{Kind: Delete, Type: model.A, Name: "Mail.Example.COM"} + if got := d.Key(); got != "A mail.example.com." { + t.Fatalf("key: %q", got) + } +} + func TestDiffAddUpdateDeleteInSync(t *testing.T) { tmpl := []model.Record{ {Type: model.A, Name: "a.example.com.", TTL: 300, Values: []string{"1.1.1.1"}}, // in sync diff --git a/internal/service/service.go b/internal/service/service.go index a4662fa..80d8ca8 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -50,8 +50,8 @@ type Recorder interface { } type ApplyRequest struct { - ApplyUpdates bool - ApplyPrunes bool + Updates []string // record keys (RecordDiff.Key) to add/update + Prunes []string // record keys to delete } type DomainService struct { @@ -128,18 +128,29 @@ func (s *DomainService) ZoneRecords(ctx context.Context, projectID, domainID uui return recs, nil } -// Apply applies updates always (when ApplyUpdates) and prunes only when ApplyPrunes. +// Apply applies exactly the diffs whose keys are selected in req.Updates and +// req.Prunes. Selected prunes are added to the applied set BEFORE selected +// updates: deletes first is an invariant, not an option — the provider +// rejects an Add/Update whose name still has a conflicting record (e.g. a +// CNAME cannot be created while an A on the same name exists), so pruning the +// old records before applying updates avoids that. func (s *DomainService) Apply(ctx context.Context, projectID, domainID uuid.UUID, req ApplyRequest) (diff.Changeset, error) { p, creds, ref, cs, err := s.resolve(ctx, projectID, domainID) if err != nil { return diff.Changeset{}, err } + selPrunes := toSet(req.Prunes) + selUpdates := toSet(req.Updates) var toApply []diff.RecordDiff - if req.ApplyUpdates { - toApply = append(toApply, cs.Updates()...) + for _, d := range cs.Prunes() { + if selPrunes[d.Key()] { + toApply = append(toApply, d) + } } - if req.ApplyPrunes { - toApply = append(toApply, cs.Prunes()...) + for _, d := range cs.Updates() { + if selUpdates[d.Key()] { + toApply = append(toApply, d) + } } applied := diff.Changeset{Diffs: toApply} if len(toApply) > 0 { @@ -149,3 +160,11 @@ func (s *DomainService) Apply(ctx context.Context, projectID, domainID uuid.UUID } return applied, nil } + +func toSet(keys []string) map[string]bool { + m := make(map[string]bool, len(keys)) + for _, k := range keys { + m[k] = true + } + return m +} diff --git a/internal/service/service_test.go b/internal/service/service_test.go index 4bcea46..69f080a 100644 --- a/internal/service/service_test.go +++ b/internal/service/service_test.go @@ -125,39 +125,69 @@ func TestZoneRecordsReadsProviderDirectly(t *testing.T) { } } -func TestApplyRespectsPruneGuard(t *testing.T) { - // зона содержит лишнюю запись b (нет в шаблоне) → Prune-кандидат +// TestApplySelectsByKeyAndOrdersPrunesBeforeUpdates covers the two goals of +// selective apply: (1) only diffs whose key is present in the request are +// applied, and (2) when both an update and a prune are selected, the prune +// (Delete) must land BEFORE the update in the applied Changeset — this is the +// regression guard for the provider rejecting an Add/Update whose name still +// conflicts with an existing record (e.g. a CNAME cannot be created while an +// A on the same name still exists). +func TestApplySelectsByKeyAndOrdersPrunesBeforeUpdates(t *testing.T) { + // zone: a needs updating (9.9.9.9 -> 1.1.1.1), b is an extra record not in + // the template (prune candidate). actual := []model.Record{ - {Type: model.A, Name: "a.example.com.", TTL: 300, Values: []string{"1.1.1.1"}}, + {Type: model.A, Name: "a.example.com.", TTL: 300, Values: []string{"9.9.9.9"}}, {Type: model.A, Name: "b.example.com.", TTL: 300, Values: []string{"2.2.2.2"}}, } tmpl := dto.TemplateDoc{Records: []dto.RecordDTO{ - {Type: "A", Name: "a.example.com.", TTL: 300, Values: []string{"1.1.1.1"}}, // in sync + {Type: "A", Name: "a.example.com.", TTL: 300, Values: []string{"1.1.1.1"}}, // update }} - // applyPrunes=false → удаление b НЕ применяется + const updKey = "A a.example.com." + const pruneKey = "A b.example.com." + + // Only the prune selected -> only the delete is applied. svc, fp := setup(t, actual, tmpl) - if _, err := svc.Apply(context.Background(), uuid.New(), uuid.New(), ApplyRequest{ApplyUpdates: true, ApplyPrunes: false}); err != nil { + if _, err := svc.Apply(context.Background(), uuid.New(), uuid.New(), ApplyRequest{Prunes: []string{pruneKey}}); err != nil { t.Fatal(err) } - for _, d := range fp.applied.Diffs { - if d.Kind == diff.Delete { - t.Fatalf("prune must be skipped when ApplyPrunes=false, applied: %+v", fp.applied.Diffs) - } + if len(fp.applied.Diffs) != 1 || fp.applied.Diffs[0].Kind != diff.Delete { + t.Fatalf("expected only the selected prune applied, got %+v", fp.applied.Diffs) } - // applyPrunes=true → удаление b применяется + // Only the update selected -> only the update is applied. svc2, fp2 := setup(t, actual, tmpl) - if _, err := svc2.Apply(context.Background(), uuid.New(), uuid.New(), ApplyRequest{ApplyUpdates: true, ApplyPrunes: true}); err != nil { + if _, err := svc2.Apply(context.Background(), uuid.New(), uuid.New(), ApplyRequest{Updates: []string{updKey}}); err != nil { t.Fatal(err) } - var sawDelete bool - for _, d := range fp2.applied.Diffs { - if d.Kind == diff.Delete && d.Name == "b.example.com." { - sawDelete = true - } + if len(fp2.applied.Diffs) != 1 || fp2.applied.Diffs[0].Kind != diff.Update { + t.Fatalf("expected only the selected update applied, got %+v", fp2.applied.Diffs) } - if !sawDelete { - t.Fatalf("prune must be applied when ApplyPrunes=true, applied: %+v", fp2.applied.Diffs) + + // Nothing selected -> nothing applied. + svc3, fp3 := setup(t, actual, tmpl) + if _, err := svc3.Apply(context.Background(), uuid.New(), uuid.New(), ApplyRequest{}); err != nil { + t.Fatal(err) + } + if len(fp3.applied.Diffs) != 0 { + t.Fatalf("expected nothing applied when nothing is selected, got %+v", fp3.applied.Diffs) + } + + // CRITICAL: both selected -> the prune (Delete) must be applied FIRST, + // the update SECOND. Regressing this order reintroduces the + // CNAME/A-conflict bug where the provider rejects the update because the + // stale conflicting record hasn't been deleted yet. + svc4, fp4 := setup(t, actual, tmpl) + if _, err := svc4.Apply(context.Background(), uuid.New(), uuid.New(), ApplyRequest{Updates: []string{updKey}, Prunes: []string{pruneKey}}); err != nil { + t.Fatal(err) + } + if len(fp4.applied.Diffs) != 2 { + t.Fatalf("expected both selected diffs applied, got %+v", fp4.applied.Diffs) + } + if fp4.applied.Diffs[0].Kind != diff.Delete { + t.Fatalf("expected prune (Delete) FIRST in applied order, got %+v", fp4.applied.Diffs) + } + if fp4.applied.Diffs[1].Kind != diff.Update { + t.Fatalf("expected update SECOND in applied order, got %+v", fp4.applied.Diffs) } } From 2f1f5311ad034bd159d9e1175c5a81150c1d487d Mon Sep 17 00:00:00 2001 From: Vassiliy Yegorov Date: Sun, 5 Jul 2026 15:20:09 +0700 Subject: [PATCH 3/4] feat(web): per-record apply checkboxes with select-all; prune opt-in Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/api/client.test.ts | 7 +- web/src/api/types.ts | 3 +- web/src/components/DiffView.test.tsx | 81 ++++++++++++++++++++-- web/src/components/DiffView.tsx | 98 ++++++++++++++++++++++++--- web/src/pages/DomainDiffPage.test.tsx | 47 ++++++++++--- web/src/pages/DomainDiffPage.tsx | 95 ++++++++++++++------------ 6 files changed, 259 insertions(+), 72 deletions(-) diff --git a/web/src/api/client.test.ts b/web/src/api/client.test.ts index ced6d5d..2eedff8 100644 --- a/web/src/api/client.test.ts +++ b/web/src/api/client.test.ts @@ -99,12 +99,13 @@ describe("api client", () => { await expect(api.listDomains(PROJECT_ID)).rejects.toThrow(UnauthorizedError) }) - it("applies with prune flag using projectId, id, body order", async () => { + it("applies with selected record keys using projectId, id, body order", async () => { const spy = mockFetch({ updates: [], prunes: [], readOnly: [], inSyncCount: 0 }) - await api.applyDomain(PROJECT_ID, "d1", { applyUpdates: true, applyPrunes: true }) + await api.applyDomain(PROJECT_ID, "d1", { updates: ["A a."], prunes: ["A b."] }) const [url, opts] = spy.mock.calls[0] expect(url).toBe(`/api/v1/projects/${PROJECT_ID}/domains/d1/apply`) - expect(String((opts as RequestInit).body)).toContain("applyPrunes") + expect(String((opts as RequestInit).body)).toContain("prunes") + expect(JSON.parse(String((opts as RequestInit).body))).toEqual({ updates: ["A a."], prunes: ["A b."] }) }) it("checkDomain(projectId, id) hits project-scoped check path", async () => { diff --git a/web/src/api/types.ts b/web/src/api/types.ts index 4c8c540..97c0ece 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -38,6 +38,7 @@ export interface CreateChannelInput { type: string; config: object; secret: stri export interface CheckRun { id?: string; createdAt: string; result: object } export interface RecordView { + key: string // stable "TYPE name." identifier — used to select this record for Apply kind: string // add | update | delete | in_sync type: string name: string @@ -51,4 +52,4 @@ export interface ChangesetResponse { readOnly: RecordView[] inSyncCount: number } -export interface ApplyRequest { applyUpdates: boolean; applyPrunes: boolean } +export interface ApplyRequest { updates: string[]; prunes: string[] } diff --git a/web/src/components/DiffView.test.tsx b/web/src/components/DiffView.test.tsx index 5bfce43..b5e5a6a 100644 --- a/web/src/components/DiffView.test.tsx +++ b/web/src/components/DiffView.test.tsx @@ -1,16 +1,34 @@ import { render, screen } from "@testing-library/react" +import userEvent from "@testing-library/user-event" import { DiffView } from "./DiffView" import type { ChangesetResponse } from "@/api/types" const cs: ChangesetResponse = { - updates: [{ kind: "update", type: "A", name: "www.example.com.", desired: ["1.1.1.1"], actual: ["9.9.9.9"], readOnly: false }], - prunes: [{ kind: "delete", type: "A", name: "old.example.com.", actual: ["2.2.2.2"], readOnly: false }], - readOnly: [{ kind: "update", type: "NS", name: "example.com.", desired: ["ns1."], actual: ["ns2."], readOnly: true }], + updates: [{ key: "A www.example.com.", kind: "update", type: "A", name: "www.example.com.", desired: ["1.1.1.1"], actual: ["9.9.9.9"], readOnly: false }], + prunes: [{ key: "A old.example.com.", kind: "delete", type: "A", name: "old.example.com.", actual: ["2.2.2.2"], readOnly: false }], + readOnly: [{ key: "NS example.com.", kind: "update", type: "NS", name: "example.com.", desired: ["ns1."], actual: ["ns2."], readOnly: true }], inSyncCount: 3, } +function noop() { /* unused in most tests */ } + +function renderDiff(overrides: Partial[0]> = {}) { + return render( + , + ) +} + test("renders all sections with counts", () => { - render() + renderDiff() expect(screen.getByText(/www\.example\.com\./)).toBeInTheDocument() expect(screen.getByText(/old\.example\.com\./)).toBeInTheDocument() // Anchored (vs. the brief's bare /example\.com\./) — "www.example.com." and @@ -23,7 +41,7 @@ test("renders all sections with counts", () => { }) test("marks read-only records", () => { - render() + renderDiff() expect(screen.getByText(/NS/)).toBeInTheDocument() }) @@ -35,6 +53,7 @@ test("renders a very long unbreakable value (DKIM key) without crashing", () => const csWithDkim: ChangesetResponse = { updates: [ { + key: "TXT default._domainkey.example.com.", kind: "update", type: "TXT", name: "default._domainkey.example.com.", @@ -47,7 +66,7 @@ test("renders a very long unbreakable value (DKIM key) without crashing", () => readOnly: [], inSyncCount: 0, } - render() + renderDiff({ changeset: csWithDkim, selectedUpdates: new Set() }) expect(screen.getByText(new RegExp(longValue))).toBeInTheDocument() }) @@ -60,7 +79,55 @@ test("does not crash when changeset fields are null", () => { readOnly: null, inSyncCount: 5, } as unknown as ChangesetResponse - render() + renderDiff({ changeset: nullish, selectedUpdates: new Set() }) expect(screen.getByText(/5/)).toBeInTheDocument() expect(screen.getByText(/in sync/)).toBeInTheDocument() }) + +test("renders a checkbox for update and prune rows but not for read-only rows", () => { + renderDiff() + // 2 select-all (update + prune headers) + 2 row checkboxes (one update, one prune). + // Read-only section contributes none: no select-all, no row checkbox. + const checkboxes = screen.getAllByRole("checkbox") + expect(checkboxes).toHaveLength(4) + + const updateRowCheckbox = screen.getByRole("checkbox", { name: /www\.example\.com\./ }) + expect(updateRowCheckbox).toBeInTheDocument() + const pruneRowCheckbox = screen.getByRole("checkbox", { name: /old\.example\.com\./ }) + expect(pruneRowCheckbox).toBeInTheDocument() + + expect(screen.queryByRole("checkbox", { name: /example\.com\..*NS|NS.*example\.com\./ })).not.toBeInTheDocument() +}) + +test("clicking an update row checkbox calls onToggleUpdate with the record key", async () => { + const onToggleUpdate = vi.fn() + const user = userEvent.setup() + renderDiff({ onToggleUpdate }) + + await user.click(screen.getByRole("checkbox", { name: /www\.example\.com\./ })) + expect(onToggleUpdate).toHaveBeenCalledWith("A www.example.com.") +}) + +test("clicking a prune row checkbox calls onTogglePrune with the record key", async () => { + const onTogglePrune = vi.fn() + const user = userEvent.setup() + renderDiff({ onTogglePrune }) + + await user.click(screen.getByRole("checkbox", { name: /old\.example\.com\./ })) + expect(onTogglePrune).toHaveBeenCalledWith("A old.example.com.") +}) + +test("select-all header checkbox is checked when all rows in the section are selected", () => { + renderDiff({ selectedUpdates: new Set(["A www.example.com."]) }) + const selectAll = screen.getByRole("checkbox", { name: /выбрать все.*updates/i }) + expect(selectAll).toHaveAttribute("aria-checked", "true") +}) + +test("select-all header checkbox calls onToggleAllUpdates(true) when clicked while none selected", async () => { + const onToggleAllUpdates = vi.fn() + const user = userEvent.setup() + renderDiff({ selectedUpdates: new Set(), onToggleAllUpdates }) + + await user.click(screen.getByRole("checkbox", { name: /выбрать все.*updates/i })) + expect(onToggleAllUpdates).toHaveBeenCalledWith(true) +}) diff --git a/web/src/components/DiffView.tsx b/web/src/components/DiffView.tsx index 185344a..463fa51 100644 --- a/web/src/components/DiffView.tsx +++ b/web/src/components/DiffView.tsx @@ -1,6 +1,7 @@ import type { ReactNode } from "react" import { ArrowRight, CircleCheck, Lock, Pencil, Trash2 } from "lucide-react" import { Badge } from "@/components/ui/badge" +import { Checkbox } from "@/components/ui/checkbox" import { cn } from "@/lib/utils" import type { ChangesetResponse, RecordView } from "@/api/types" @@ -40,9 +41,23 @@ function Values({ values }: { values?: string[] }) { return <>{values.join(", ")} } -function RecordRow({ record, tone }: { record: RecordView; tone: Tone }) { +function RecordRow({ + record, + tone, + checked, + onToggle, +}: { + record: RecordView + tone: Tone + checked?: boolean + onToggle?: (key: string) => void +}) { const meta = TONE_META[tone] const showArrow = tone !== "delete" + // Read-only records aren't selectable — onToggle is only passed for + // update/delete sections. Presence of onToggle is the selectability flag, + // not the tone, so this stays correct if a tone's selectability ever changes. + const selectable = onToggle !== undefined return (
- {/* Top line: type badge, name, read-only flag — always single-line, - never affected by how long the record values are. */} + {/* Top line: (optional) checkbox, type badge, name, read-only flag — + always single-line, never affected by how long the record values are. */}
+ {selectable && ( + onToggle!(record.key)} + aria-label={`${tone === "delete" ? "Удалить" : "Применить"} ${record.type} ${record.name}`} + className="shrink-0" + /> + )} + + to align under the name (badge width + gap, plus checkbox width + + gap when this row is selectable). */} + )} @@ -145,17 +201,41 @@ function Section({ export function DiffView({ changeset, + selectedUpdates, + selectedPrunes, + onToggleUpdate, + onTogglePrune, + onToggleAllUpdates, + onToggleAllPrunes, footerExtra, }: { changeset: ChangesetResponse + selectedUpdates: Set + selectedPrunes: Set + onToggleUpdate: (key: string) => void + onTogglePrune: (key: string) => void + onToggleAllUpdates: (checked: boolean) => void + onToggleAllPrunes: (checked: boolean) => void footerExtra?: ReactNode }) { // Defensive: a field may arrive as null (e.g. a nil slice from an older // backend) — normalise to [] so Section never calls .length/.map on null. return (
-
-
+
+
diff --git a/web/src/pages/DomainDiffPage.test.tsx b/web/src/pages/DomainDiffPage.test.tsx index ba9e758..708cd3d 100644 --- a/web/src/pages/DomainDiffPage.test.tsx +++ b/web/src/pages/DomainDiffPage.test.tsx @@ -41,10 +41,10 @@ beforeEach(() => { vi.spyOn(api, "listDomains").mockResolvedValue([domainWithTemplate]) }) -test("apply sends applyPrunes=false by default, true only after opting in", async () => { +test("default selection: updates checked, prunes unchecked; apply sends only selected keys", async () => { vi.spyOn(api, "checkDomain").mockResolvedValue({ - updates: [{ kind: "update", type: "A", name: "a.", desired: ["1"], actual: ["2"], readOnly: false }], - prunes: [{ kind: "delete", type: "A", name: "b.", actual: ["3"], readOnly: false }], + updates: [{ key: "A a.", kind: "update", type: "A", name: "a.", desired: ["1"], actual: ["2"], readOnly: false }], + prunes: [{ key: "A b.", kind: "delete", type: "A", name: "b.", actual: ["3"], readOnly: false }], readOnly: [], inSyncCount: 0, }) const applySpy = vi.spyOn(api, "applyDomain").mockResolvedValue({ updates: [], prunes: [], readOnly: [], inSyncCount: 0 }) @@ -52,22 +52,51 @@ test("apply sends applyPrunes=false by default, true only after opting in", asyn const user = userEvent.setup() renderPage() - const applyBtn = await screen.findByRole("button", { name: /apply/i }) + const updateRowCheckbox = await screen.findByRole("checkbox", { name: /a\.$/ }) + const pruneRowCheckbox = screen.getByRole("checkbox", { name: /b\.$/ }) + expect(updateRowCheckbox).toHaveAttribute("aria-checked", "true") + expect(pruneRowCheckbox).toHaveAttribute("aria-checked", "false") + expect(screen.queryByText(/будет удалено записей/i)).not.toBeInTheDocument() + + const applyBtn = screen.getByRole("button", { name: /apply/i }) await user.click(applyBtn) await waitFor(() => expect(applySpy).toHaveBeenCalled()) - expect(applySpy.mock.calls[0]).toEqual([PROJECT_ID, "d1", { applyUpdates: true, applyPrunes: false }]) + expect(applySpy.mock.calls[0]).toEqual([PROJECT_ID, "d1", { updates: ["A a."], prunes: [] }]) + + // отметить prune → появляется предупреждение с количеством, и Apply шлёт его ключ тоже + await user.click(pruneRowCheckbox) + const warning = screen.getByRole("alert") + expect(warning).toHaveTextContent(/будет удалено записей:\s*1/i) - // включить prune и применить снова - const pruneToggle = screen.getByRole("checkbox", { name: /prune|удал/i }) - await user.click(pruneToggle) await user.click(screen.getByRole("button", { name: /apply/i })) await waitFor(() => expect(applySpy).toHaveBeenCalledTimes(2)) - expect(applySpy.mock.calls[1]).toEqual([PROJECT_ID, "d1", { applyUpdates: true, applyPrunes: true }]) + expect(applySpy.mock.calls[1]).toEqual([PROJECT_ID, "d1", { updates: ["A a."], prunes: ["A b."] }]) // домен с шаблоном: записи зоны не нужны для диффа — запрос не должен уходить к провайдеру expect(zoneRecordsSpy).not.toHaveBeenCalled() }) +test("deselecting all records disables Apply", async () => { + vi.spyOn(api, "checkDomain").mockResolvedValue({ + updates: [{ key: "A a.", kind: "update", type: "A", name: "a.", desired: ["1"], actual: ["2"], readOnly: false }], + prunes: [], + readOnly: [], inSyncCount: 0, + }) + vi.spyOn(api, "applyDomain").mockResolvedValue({ updates: [], prunes: [], readOnly: [], inSyncCount: 0 }) + const user = userEvent.setup() + renderPage() + + const applyBtn = await screen.findByRole("button", { name: /apply/i }) + expect(applyBtn).not.toBeDisabled() + expect(screen.getByText(/готово к применению/i)).toBeInTheDocument() + + const updateRowCheckbox = screen.getByRole("checkbox", { name: /a\.$/ }) + await user.click(updateRowCheckbox) + + expect(applyBtn).toBeDisabled() + expect(screen.getByText(/изменений для применения нет/i)).toBeInTheDocument() +}) + test("пока список доменов грузится — показан общий лоадер, а не баннер об отсутствии шаблона", async () => { let resolveListDomains: (domains: Domain[]) => void vi.spyOn(api, "listDomains").mockReturnValue( diff --git a/web/src/pages/DomainDiffPage.tsx b/web/src/pages/DomainDiffPage.tsx index aaccd1b..eac410f 100644 --- a/web/src/pages/DomainDiffPage.tsx +++ b/web/src/pages/DomainDiffPage.tsx @@ -1,11 +1,9 @@ -import { useId, useState } from "react" +import { useEffect, useState } from "react" import { useParams } from "react-router-dom" import { AlertTriangle, Loader2, Play, RefreshCw, TriangleAlert } from "lucide-react" import { DiffView } from "@/components/DiffView" import { DomainHistory } from "@/components/DomainHistory" import { Button } from "@/components/ui/button" -import { Checkbox } from "@/components/ui/checkbox" -import { Label } from "@/components/ui/label" import { Table, TableBody, @@ -37,17 +35,48 @@ export function DomainDiffPage() { // (успешный ответ), что шаблона нет. const zoneRecords = useZoneRecords(id, !domains.isPending && !domains.isError && !hasTemplate) const createTemplateFromZone = useCreateTemplateFromZone() - const [applyPrunes, setApplyPrunes] = useState(false) - const pruneCheckboxId = useId() + const [selectedUpdates, setSelectedUpdates] = useState>(new Set()) + const [selectedPrunes, setSelectedPrunes] = useState>(new Set()) const changeset = check.data - const hasPrunes = (changeset?.prunes?.length ?? 0) > 0 - const hasUpdates = (changeset?.updates?.length ?? 0) > 0 - const pruneWarning = applyPrunes && hasPrunes + + // Re-derive the selection whenever the changeset changes (initial load, + // recheck, or apply's own invalidation): updates default to fully selected + // (safe — they only bring the zone in line with the template), prunes + // default to empty (deletion is opt-in and irreversible). + useEffect(() => { + setSelectedUpdates(new Set((changeset?.updates ?? []).map((r) => r.key))) + setSelectedPrunes(new Set()) + }, [changeset]) + const recordList = zoneRecords.data ?? [] + const hasSelection = selectedUpdates.size + selectedPrunes.size > 0 + + function toggleUpdate(key: string) { + setSelectedUpdates((prev) => { + const next = new Set(prev) + if (next.has(key)) next.delete(key) + else next.add(key) + return next + }) + } + function togglePrune(key: string) { + setSelectedPrunes((prev) => { + const next = new Set(prev) + if (next.has(key)) next.delete(key) + else next.add(key) + return next + }) + } + function toggleAllUpdates(checked: boolean) { + setSelectedUpdates(checked ? new Set((changeset?.updates ?? []).map((r) => r.key)) : new Set()) + } + function toggleAllPrunes(checked: boolean) { + setSelectedPrunes(checked ? new Set((changeset?.prunes ?? []).map((r) => r.key)) : new Set()) + } function onApply() { - apply.mutate({ applyUpdates: true, applyPrunes }) + apply.mutate({ updates: [...selectedUpdates], prunes: [...selectedPrunes] }) } function onCreateTemplateFromZone() { @@ -197,36 +226,18 @@ export function DomainDiffPage() { {hasTemplate && changeset && ( <> - +
- - - {pruneWarning && ( + {selectedPrunes.size > 0 && (
- Будет безвозвратно удалено записей:{" "} - {changeset.prunes.length}. Действие необратимо. + Будет удалено записей:{" "} + {selectedPrunes.size}. Действие необратимо.
)} @@ -252,12 +263,10 @@ export function DomainDiffPage() { ) : ( - {hasUpdates || (applyPrunes && hasPrunes) - ? "Готово к применению" - : "Изменений для применения нет"} + {hasSelection ? "Готово к применению" : "Изменений для применения нет"} )} -