refactor: migrate Group-4 get commands to make_get_command (#588)

CHANGELOG.md

@@ -2,6 +2,39 @@
 
 ## Unreleased
 
+**Internal — `make_get_command` covers the complex Group-4 `get`s (#588):**
+
+- `adgroups get`, `keywords get`, `creatives get`, `strategies get` and
+  `audiencetargets get` now register through the shared `make_get_command`
+  factory (net −345 lines). They reuse the existing `extra_options` /
+  `criteria_builder` / `criteria_limits` / `require_criteria_message` /
+  `nested_field_options` hooks — the `--status`/`--statuses` mutual-exclusion
+  guard (adgroups/keywords) lives in the per-module `criteria_builder`, not a
+  new factory flag. adgroups (8) and strategies (16) exercise the factory's
+  largest nested-`*FieldNames` sets to date.
+- Factory ordering fix: nested `*FieldNames` are now parsed (and their
+  provided-but-empty-CSV `UsageError` raised) *before* both the criteria-limit
+  enforcement and the empty-criteria `require_criteria_message` guard, matching
+  the order every hand-rolled command used. So a `--<nested> ""` combined with
+  either no filter at all or an over-limit array reports the nested error, not
+  the require/limit one (pinned by a new `test_dry_run` regression test). The
+  parsed dict is still merged after the common params, so payload key order is
+  unchanged, and this also restores the pre-factory nested-before-limits order
+  for the already-migrated `bidmodifiers get`. No module lacking both a nested
+  option and a `criteria_limits`/`require_criteria_message` is affected.
+- No CLI surface change: `--help` (option order, all nested-field options, the
+  `--is-archived` choice/default), `--dry-run` payloads, and the error-precedence
+  edge cases (empty-nested vs. no-filter and vs. over-limit) are byte-identical,
+  verified against 33 pre-migration baselines captured via CliRunner from the
+  pre-migration tree. As with every prior `make_get_command` migration, the
+  factory resolves the API client only on the live path, so these five `get`s
+  now honor `--dry-run` as a token-free test seam (no behavior change vs. the
+  other factory-backed commands).
+- Two Group-4 `get`s stay hand-rolled as documented carve-outs (see `_get.py`):
+  `ads get` (its `TextAdFieldNames` is always emitted with a per-field default,
+  like the carved-out `keywordbids`) and `campaigns get` (its `--fields` rejects
+  an explicitly empty CSV instead of falling back to the default `FieldNames`).
+
 **Internal — `make_get_command` covers the criteria-limit `get`s `bids` / `bidmodifiers` (completes #587):**
 
 - `bids get` (criteria-limit + require-at-least-one-filter, no `--ids`) and

direct_cli/commands/_get.py

@@ -14,15 +14,19 @@
 and the live path runs the module's real ``create_client`` (the VCR cassette
 read-tests replay it at the ``requests`` transport, so they are unaffected).
 
-Three ``get`` commands deliberately stay hand-rolled (issue #587), each for a
-structural reason this factory does not encode:
+Five ``get`` commands deliberately stay hand-rolled (issues #587, #588), each
+for a structural reason this factory does not encode:
 
 * ``leads get`` — ``--datetime-from`` / ``--datetime-to`` sit between ``--limit``
   and ``--fetch-all``, a bespoke option order the shared ``get_options`` stack
   cannot reproduce (it always renders ``--limit`` / ``--fetch-all`` adjacent).
-* ``keywordbids get`` — its nested ``SearchFieldNames`` / ``NetworkFieldNames``
-  carry per-field defaults and are *always* emitted, unlike the provided-only
-  nested projections ``nested_field_options`` builds.
+* ``keywordbids get`` and ``ads get`` — they emit a nested ``*FieldNames``
+  (``Search``/``NetworkFieldNames``; ``TextAdFieldNames``) with a per-field
+  default that is *always* present, unlike the provided-only nested projections
+  ``nested_field_options`` builds.
+* ``campaigns get`` — its ``--fields`` rejects an explicitly empty CSV
+  (``_parse_csv_option``) instead of the factory's silent fall-back to the
+  default ``FieldNames``.
 * ``reports get`` — a custom non-RPC TSV stream, not a JSON-RPC ``get``.
 """
 
@@ -184,6 +188,18 @@ def get(
             default_fields_key
         )
         criteria = build_criteria(ids, **kwargs)
+        # Validate provided-but-empty nested *FieldNames CSVs (parse_nested_field_names
+        # rejects them) before the criteria-limit enforcement and the empty-criteria
+        # guard, matching the hand-rolled order of every migrated command: with both
+        # an over-limit array and an empty "--<nested> ''" (or a "--<nested> ''" and
+        # no filter at all), the nested error is the one reported. The parsed dict is
+        # merged after the common params below, so the payload key order is unchanged.
+        parsed_nested = {}
+        if nested_specs:
+            raw_nested = tuple(
+                (wsdl_key, kwargs[kwarg]) for wsdl_key, kwarg in nested_specs
+            )
+            parsed_nested = parse_nested_field_names(raw_nested)
         if criteria_limits:
             enforce_criteria_array_limits(
                 criteria, criteria_limits, command_name=f"{svc} get"
@@ -199,11 +215,8 @@ def get(
             params = build_common_params(
                 criteria=criteria, field_names=field_names, limit=limit
             )
-        if nested_specs:
-            raw_nested = tuple(
-                (wsdl_key, kwargs[kwarg]) for wsdl_key, kwarg in nested_specs
-            )
-            params.update(parse_nested_field_names(raw_nested))
+        if parsed_nested:
+            params.update(parsed_nested)
         if adextensions_wire_layout and limit:
             # Page after the nested *FieldNames, matching adextensions' layout.
             params["Page"] = {"Limit": limit}