chore(write-slice): align cross-module conventions post-#122 review

CHANGELOG.md

@@ -9,6 +9,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Cross-module convention alignment, post-#122-review parity sweep:
+  - `account.AccountList` / `mailaccount.MailAccountList` now render
+    the `used_*_space` column with a `" MB"` suffix as part of the
+    value (matching the singular detail row) and use a bare `USED`
+    header. Consistent with the database slice after #122 followups
+    — all three modules now share a single unit-rendering convention.
+  - `ddns.DDNSUser.in_progress` is no longer marked `omitempty`,
+    aligning with the majority of read modules. Captured fixtures
+    have always carried `in_progress`; the empty-string fallback on
+    an older account is harmless.
+  - `cronjob.Client.Add`, `ddns.Client.Add`, `ftpuser.Client.Add`,
+    `sambauser.Client.Add`, `mailinglist.Client.Add` and
+    `mailforward.Client.Add` now emit per-field validation errors
+    (`requires a non-empty <field>`) instead of a single combined
+    message ("requires a non-empty X, Y and Z"). A caller hitting
+    the domain validator can now tell which field actually broke,
+    matching the convention introduced by the database slice in
+    #122 followups.
+- `docs/usage/destructive-writes.md` refactored: a single
+  "per-slice baseline" section captures the contract every wired
+  slice carries (gating policy, dry-run/audit/redaction,
+  generated-login printing) and a "per-slice deviations" table
+  calls out only what each slice changes (mailforward's target-list
+  phrasing, database's louder delete verb + optional
+  `--allowed-hosts` wildcard, ddns's no-`_new_password` split, …).
+  Replaces eight near-identical paragraphs that had grown to
+  roughly copy-paste.
+
 - `kasapi-cli databases add` and `… update` now bind disjoint flag
   sets (mirroring the ddnsuser slice). The flag names are identical
   on both subcommands, but the help text of each reflects its own

docs/usage/destructive-writes.md

@@ -10,85 +10,40 @@ This page documents the behavioural contract (issue
 v0.2.0 write phase, [#13](https://github.com/chmmou/kasapi-cli/issues/13)).
 The read commands are non-destructive and are not gated.
 
-The first write endpoints —
-[`mail forwards`](https://github.com/chmmou/kasapi-cli/issues/115)
-`add` / `update` / `delete` — wire this contract. `delete` and
-`update` are gated by the confirmation prompt (an `update_mailforward`
-replaces the whole target list and is therefore irreversible); `add`
-creates and is reversible, so it is **not** prompted. All three honour
-`--dry-run` and emit an audit record.
-
-[`mail lists`](https://github.com/chmmou/kasapi-cli/issues/117) `add` /
-`update` / `delete` wire the same contract with the same policy:
-`update` and `delete` are gated; `add` is reversible and not prompted.
-`update` is gated because the fields it sets — subscriber,
-restrict-post and config — are each replaced wholesale (the
-confirmation prompt phrases this as replacing the list's *settings*).
-The list password passed to `add` is redacted in the `--dry-run`
-preview and the audit record.
-
-[`cronjobs`](https://github.com/chmmou/kasapi-cli/issues/118) `add` /
-`update` / `delete` wire the same contract with the same policy:
-`update` and `delete` are gated; `add` is reversible and not prompted.
-`update` is gated because every field it sets is replaced wholesale
-(the confirmation prompt phrases this as replacing the cronjob's
-*settings*) and `update` sends only the explicitly-changed flags. Any
-`--http-password` passed to `add`/`update` is redacted in the
-`--dry-run` preview and the audit record.
-
-[`ftpusers`](https://github.com/chmmou/kasapi-cli/issues/119) `add` /
-`update` / `delete` wire the same contract with the same policy:
-`update` and `delete` are gated; `add` is reversible and not prompted.
-`update` is gated because every field it sets is replaced wholesale
-(the confirmation prompt phrases this as replacing the FTP user's
-*settings*) and `update` sends only the explicitly-changed flags.
-`add_ftpuser` takes no login — KAS generates it and the command prints
-it on success. The `--password` flag is redacted in the `--dry-run`
-preview and the audit record for both `add` and `update` (it maps to
-`ftp_password` on add, `ftp_new_password` on update).
-
-[`sambausers`](https://github.com/chmmou/kasapi-cli/issues/120) `add` /
-`update` / `delete` wire the same contract with the same policy:
-`update` and `delete` are gated; `add` is reversible and not prompted.
-`update` is gated because every field it sets is replaced wholesale
-(the confirmation prompt phrases this as replacing the Samba user's
-*settings*) and `update` sends only the explicitly-changed flags.
-`add_sambauser` takes no login — KAS generates it and the command
-prints it on success. The `--password` flag is redacted in the
-`--dry-run` preview and the audit record for both `add` and `update`
-(it maps to `samba_password` on add, `samba_new_password` on update).
-
-[`databases`](https://github.com/chmmou/kasapi-cli/issues/122) `add` /
-`update` / `delete` wire the same contract with one important
-loudness adjustment: `delete_database`'s confirmation prompt uses the
-verb `"permanently delete"` rather than the bare `"delete"` every
-other slice uses, because dropping a database also drops every row
-it contains — the loudest data-loss surface of the v0.2.0 write
-phase. `update` and `delete` are gated; `add` is reversible and not
-prompted. `update` is gated because every field it sets is replaced
-wholesale (the confirmation prompt phrases this as replacing the
-database's *settings*) and `update` sends only the explicitly-changed
-flags. `add_database` takes no login — KAS generates it (the login
-equals the database name on creation, e.g. `d0123460`) and the
-command prints it on success. The `--password` flag is redacted in
-the `--dry-run` preview and the audit record for both `add` and
-`update` (it maps to `database_password` on add,
-`database_new_password` on update).
-
-[`ddnsusers`](https://github.com/chmmou/kasapi-cli/issues/121) `add` /
-`update` / `delete` wire the same contract with the same policy:
-`update` and `delete` are gated; `add` is reversible and not prompted.
-`update` is gated because every field it sets is replaced wholesale
-(the confirmation prompt phrases this as replacing the DDNS user's
-*settings*) and `update` sends only the explicitly-changed flags.
-`add_ddnsuser` takes no login — KAS generates it and the command
-prints it on success. Unlike the FTP/Samba slices there is no
-`_new_password` split: `--password` maps to `dyndns_password` on both
-`add` and `update`, and is redacted in the `--dry-run` preview and the
-audit record. `update_ddnsuser` accepts `--target-ipv4` /
-`--target-ipv6` instead of `add`'s legacy `--target-ip`; the ipv4/ipv6
-keys are undocumented in the KAS API but verified to work against the
-live system (the captured update request fixture is authoritative).
+## Per-slice baseline
+
+Every wired write slice carries the same baseline policy. Listing it
+once here keeps the per-slice notes below short — they only call out
+the deviations.
+
+- `add` is reversible (creates a new resource) → **not** gated; no
+  prompt.
+- `update` and `delete` are irreversible (wholesale field replacement
+  on `update`; resource removal on `delete`) → **gated** by the #109
+  prompt. The prompt phrases `update` as replacing the resource's
+  *settings*.
+- `update` sends **only the explicitly-changed flags** (keyed on
+  cobra `Changed`), so an empty value is a deliberate "clear", not
+  "leave unchanged".
+- All three subcommands honour `--dry-run` (#132) and emit a #131
+  audit record. Secret request parameters are redacted in **both**
+  sinks.
+- Subcommands whose KAS action generates the login server-side
+  (`add_ftpuser`, `add_sambauser`, `add_ddnsuser`, `add_database`,
+  `add_mailinglist`, `add_mailforward`) print the generated identifier
+  on success.
+
+## Per-slice deviations
+
+| Slice (issue) | Deviation from the baseline above |
+|---|---|
+| [`mail forwards`](https://github.com/chmmou/kasapi-cli/issues/115) | First slice to wire #109/#131/#132. `update_mailforward` replaces the whole *target list*; the prompt phrases this as replacing the forward's *targets* (not "settings"). |
+| [`mail lists`](https://github.com/chmmou/kasapi-cli/issues/117) | The list password passed to `add` is redacted in the dry-run preview and audit record. |
+| [`cronjobs`](https://github.com/chmmou/kasapi-cli/issues/118) | Any `--http-password` passed to `add`/`update` is redacted. |
+| [`ftpusers`](https://github.com/chmmou/kasapi-cli/issues/119) | Password key splits between actions: `--password` → `ftp_password` on add, `ftp_new_password` on update. |
+| [`sambausers`](https://github.com/chmmou/kasapi-cli/issues/120) | Password key splits between actions: `--password` → `samba_password` on add, `samba_new_password` on update. Note: the KAS docs wrongly list `samba_new_password` for the create call; the captured fixture confirms the real key is `samba_password`. |
+| [`databases`](https://github.com/chmmou/kasapi-cli/issues/122) | **Louder delete prompt**: `delete_database` uses the verb `"permanently delete"` (vs the bare `"delete"` every other slice uses) because the action drops the database AND every row in it — the loudest data-loss surface of the v0.2.0 write phase. Password key splits between actions: `--password` → `database_password` on add, `database_new_password` on update. `--allowed-hosts` is **optional**: an empty value is the KAS API's documented "any host may connect" wildcard, sent verbatim on the wire. |
+| [`ddnsusers`](https://github.com/chmmou/kasapi-cli/issues/121) | **No `_new_password` split**: `--password` maps to `dyndns_password` on both `add` and `update`. `update_ddnsuser` accepts `--target-ipv4` / `--target-ipv6` instead of `add`'s legacy `--target-ip`; the ipv4/ipv6 keys are undocumented in the KAS API docs but verified to work against the live system (the captured update request fixture is authoritative). |
 
 ## The contract
 

internal/account/table.go

@@ -14,14 +14,17 @@ type AccountList []Account
 
 // TableHeaders returns the column names for the account list view.
 func (AccountList) TableHeaders() []string {
-	return []string{"LOGIN", "COMMENT", "MAIL", "MAX_DOMAIN", "MAX_WEBSPACE", "USED_MB", "2FA", "IN_PROGRESS"}
+	return []string{"LOGIN", "COMMENT", "MAIL", "MAX_DOMAIN", "MAX_WEBSPACE", "USED", "2FA", "IN_PROGRESS"}
 }
 
 // TableRows returns one row per account, in the order returned by KAS.
 // KAS returns used_account_space in KiB (the phpdoc at
 // https://kasapi.kasserver.com/dokumentation/phpdoc/ does not state the
 // unit, but real responses have magnitudes and fractional digits
-// consistent with bytes/1024); /1024 yields MiB, displayed as MB.
+// consistent with bytes/1024); /1024 yields MiB, displayed as MB. The
+// unit travels with the value (matching the singular detail row and
+// the cross-module convention) so list and detail views share a
+// single unit-rendering rule.
 func (l AccountList) TableRows() [][]string {
 	rows := make([][]string, 0, len(l))
 	for _, a := range l {
@@ -31,7 +34,7 @@ func (l AccountList) TableRows() [][]string {
 			a.AccountContactMail,
 			strconv.Itoa(a.MaxDomain),
 			strconv.Itoa(a.MaxWebspace),
-			strconv.FormatFloat(a.UsedAccountSpace/1024, 'f', 1, 64),
+			strconv.FormatFloat(a.UsedAccountSpace/1024, 'f', 1, 64) + " MB",
 			a.Account2FA,
 			a.InProgress,
 		})

internal/cronjob/write.go

@@ -80,11 +80,23 @@ type Spec struct {
 // (minute_syntax_incorrect, time_not_allowed, …) surface verbatim
 // through the Caller.
 func (cl *Client) Add(ctx context.Context, s Spec) (string, error) {
-	if s.Protocol == "" || s.HTTPURL == "" || s.Comment == "" {
-		return "", errors.New("cronjob: add_cronjob requires a non-empty protocol, http url and comment")
-	}
-	if s.Minute == "" || s.Hour == "" || s.DayOfMonth == "" || s.Month == "" || s.DayOfWeek == "" {
-		return "", errors.New("cronjob: add_cronjob requires --minute and --hour (and non-empty day_of_month/month/day_of_week)")
+	switch {
+	case s.Protocol == "":
+		return "", errors.New("cronjob: add_cronjob requires a non-empty protocol")
+	case s.HTTPURL == "":
+		return "", errors.New("cronjob: add_cronjob requires a non-empty http url")
+	case s.Comment == "":
+		return "", errors.New("cronjob: add_cronjob requires a non-empty comment")
+	case s.Minute == "":
+		return "", errors.New("cronjob: add_cronjob requires a non-empty minute schedule")
+	case s.Hour == "":
+		return "", errors.New("cronjob: add_cronjob requires a non-empty hour schedule")
+	case s.DayOfMonth == "":
+		return "", errors.New("cronjob: add_cronjob requires a non-empty day_of_month schedule")
+	case s.Month == "":
+		return "", errors.New("cronjob: add_cronjob requires a non-empty month schedule")
+	case s.DayOfWeek == "":
+		return "", errors.New("cronjob: add_cronjob requires a non-empty day_of_week schedule")
 	}
 	resp, err := kaswrite.Call(ctx, cl.c, "cronjob", addAction, AddParams(s))
 	if err != nil {

internal/ddns/ddns.go

@@ -28,8 +28,13 @@ type Caller = kasread.Caller
 // `dyndns_target_ip` alongside the explicit `dyndns_target_ipv4` and
 // `dyndns_target_ipv6` dual-stack fields (see
 // testdata/ddns/get_ddnsuser*_response_success.xml). The ipv4/ipv6
-// pair plus `in_progress` may still be absent on older accounts, so
-// all three are flagged with omitempty.
+// pair may still be absent on older accounts, so those two carry
+// omitempty.
+//
+// `in_progress` is no longer omitempty for parity with the majority
+// of read modules (account, mailaccount, mailinglist, sambauser,
+// ftpuser, database): every captured fixture row carries it and a
+// stray empty-string on an older account is harmless.
 type DDNSUser struct {
 	Login    string `json:"dyndns_login" yaml:"dyndns_login"`
 	Password string `json:"dyndns_password,omitempty" yaml:"dyndns_password,omitempty"`
@@ -43,7 +48,7 @@ type DDNSUser struct {
 	DualStack string `json:"dyndns_dual_stack" yaml:"dyndns_dual_stack"`
 	Comment   string `json:"dyndns_comment" yaml:"dyndns_comment"`
 
-	InProgress string `json:"in_progress,omitempty" yaml:"in_progress,omitempty"`
+	InProgress string `json:"in_progress" yaml:"in_progress"`
 }
 
 // FQDN returns the fully-qualified DDNS hostname (label + zone) for

internal/ddns/write.go

@@ -85,8 +85,17 @@ type Spec struct {
 // dyndns_target_ip_syntax_incorrect, …) is left to the API and surfaces
 // verbatim through the Caller.
 func (cl *Client) Add(ctx context.Context, s Spec) (string, error) {
-	if s.Password == "" || s.Zone == "" || s.Label == "" || s.TargetIP == "" || s.Comment == "" {
-		return "", errors.New("ddns: add_ddnsuser requires a non-empty password, zone, label, target IP and comment")
+	switch {
+	case s.Password == "":
+		return "", errors.New("ddns: add_ddnsuser requires a non-empty password")
+	case s.Zone == "":
+		return "", errors.New("ddns: add_ddnsuser requires a non-empty zone")
+	case s.Label == "":
+		return "", errors.New("ddns: add_ddnsuser requires a non-empty label")
+	case s.TargetIP == "":
+		return "", errors.New("ddns: add_ddnsuser requires a non-empty target IP")
+	case s.Comment == "":
+		return "", errors.New("ddns: add_ddnsuser requires a non-empty comment")
 	}
 	resp, err := kaswrite.Call(ctx, cl.c, "ddns", addAction, AddParams(s))
 	if err != nil {

internal/ftpuser/write.go

@@ -68,8 +68,11 @@ type Spec struct {
 // (ftp_path_syntax_incorrect, password_syntax_incorrect, …) is left to
 // the API and surfaces verbatim through the Caller.
 func (cl *Client) Add(ctx context.Context, s Spec) (string, error) {
-	if s.Password == "" || s.Comment == "" {
-		return "", errors.New("ftpuser: add_ftpuser requires a non-empty password and comment")
+	switch {
+	case s.Password == "":
+		return "", errors.New("ftpuser: add_ftpuser requires a non-empty password")
+	case s.Comment == "":
+		return "", errors.New("ftpuser: add_ftpuser requires a non-empty comment")
 	}
 	resp, err := kaswrite.Call(ctx, cl.c, "ftpuser", addAction, AddParams(s))
 	if err != nil {

internal/mailaccount/mailaccount.go

@@ -138,17 +138,19 @@ func DecodeMailAccounts(returnInfo soap.Value) (MailAccountList, error) {
 // TableHeaders returns the columns used by --output=table for
 // MailAccountList.
 func (MailAccountList) TableHeaders() []string {
-	return []string{"LOGIN", "ADDRESS", "USED_MB", "RESPONDER", "ACTIVE", "IN_PROGRESS"}
+	return []string{"LOGIN", "ADDRESS", "USED", "RESPONDER", "ACTIVE", "IN_PROGRESS"}
 }
 
-// TableRows emits one row per MailAccount entry.
+// TableRows emits one row per MailAccount entry. used_mailaccount_space
+// carries the " MB" unit as part of the value to match the singular
+// detail row and the cross-module convention.
 func (l MailAccountList) TableRows() [][]string {
 	rows := make([][]string, 0, len(l))
 	for _, a := range l {
 		rows = append(rows, []string{
 			a.Login,
 			a.Addresses,
-			strconv.FormatFloat(a.UsedSpace, 'f', 2, 64),
+			strconv.FormatFloat(a.UsedSpace, 'f', 2, 64) + " MB",
 			a.Responder,
 			a.IsActive,
 			a.InProgress,
@@ -183,7 +185,7 @@ func (a MailAccount) TableRows() [][]string {
 		{"mail_xlist_trash", a.XListTrash},
 		{"mail_xlist_spam", a.XListSpam},
 		{"mail_xlist_archiv", a.XListArchiv},
-		{"used_mailaccount_space", strconv.FormatFloat(a.UsedSpace, 'f', 2, 64)},
+		{"used_mailaccount_space", strconv.FormatFloat(a.UsedSpace, 'f', 2, 64) + " MB"},
 		{"mail_is_active", a.IsActive},
 		{"show_password", a.ShowPassword},
 		{"mail_allow_nets", a.AllowNets},

internal/mailforward/write.go

@@ -28,10 +28,12 @@ const (
 // value is rejected before any SOAP call so the CLI can surface a fast
 // validation error.
 func (cl *Client) Add(ctx context.Context, localPart, domainPart string, targets []string) (string, error) {
-	if localPart == "" || domainPart == "" {
-		return "", errors.New("mailforward: add_mailforward requires a non-empty local and domain part")
-	}
-	if len(targets) == 0 {
+	switch {
+	case localPart == "":
+		return "", errors.New("mailforward: add_mailforward requires a non-empty local part")
+	case domainPart == "":
+		return "", errors.New("mailforward: add_mailforward requires a non-empty domain part")
+	case len(targets) == 0:
 		return "", errors.New("mailforward: add_mailforward requires at least one target")
 	}
 	resp, err := kaswrite.Call(ctx, cl.c, "mailforward", addAction, AddParams(localPart, domainPart, targets))

internal/mailinglist/write.go

@@ -43,10 +43,12 @@ const (
 // rejected before any SOAP call so the CLI can surface a fast
 // validation error.
 func (cl *Client) Add(ctx context.Context, name, domain, password string) (string, error) {
-	if name == "" || domain == "" {
-		return "", errors.New("mailinglist: add_mailinglist requires a non-empty name and domain")
-	}
-	if password == "" {
+	switch {
+	case name == "":
+		return "", errors.New("mailinglist: add_mailinglist requires a non-empty name")
+	case domain == "":
+		return "", errors.New("mailinglist: add_mailinglist requires a non-empty domain")
+	case password == "":
 		return "", errors.New("mailinglist: add_mailinglist requires a non-empty password")
 	}
 	resp, err := kaswrite.Call(ctx, cl.c, "mailinglist", addAction, AddParams(name, domain, password))

internal/sambauser/write.go

@@ -62,8 +62,13 @@ type Spec struct {
 // (path_syntax_incorrect, password_syntax_incorrect, …) is left to the
 // API and surfaces verbatim through the Caller.
 func (cl *Client) Add(ctx context.Context, s Spec) (string, error) {
-	if s.Password == "" || s.Comment == "" || s.Path == "" {
-		return "", errors.New("sambauser: add_sambauser requires a non-empty password, comment and path")
+	switch {
+	case s.Password == "":
+		return "", errors.New("sambauser: add_sambauser requires a non-empty password")
+	case s.Comment == "":
+		return "", errors.New("sambauser: add_sambauser requires a non-empty comment")
+	case s.Path == "":
+		return "", errors.New("sambauser: add_sambauser requires a non-empty path")
 	}
 	resp, err := kaswrite.Call(ctx, cl.c, "sambauser", addAction, AddParams(s))
 	if err != nil {