Remove uncalled code and unreachable validations, and bring the Ash resource snapshots up to date #631

New issue

Open

opened 2026-10-30 14:38:49 +01:00 by simon · 0 comments

simon commented

2026-10-30 14:38:49 +01:00

Owner

Motivation

#605 and #609 left a few code paths with no caller, validations that can never run, a cache that no test exercises, and Ash snapshots that drifted from the code. None is user-visible, but each misleads the next reader or makes mix ash.codegen --check noisy.

Collected from: #605 leftovers (three items), #609 findings 2 and 9. Verified on main c3d99735 (2026-10-25).

Findings

Uncalled Mv.Constants.vereinfacht_required_member_fields/0 (lib/mv/constants.ex:69,79), documented in docs/vereinfacht-api.md:30,57 and the CODE_GUIDELINES tree. Remove it with the docs, or keep it and state why in its doc.
Setting actions without a production caller: :update_member_field_visibility and :update_single_member_field_visibility (lib/mv/membership/setting.ex:205; wrappers in lib/mv/membership/membership.ex:212,246, used only by tests and doctests). Drop actions and wrappers together per CODE_GUIDELINES §1.3, or document why they stay.
Unreachable "cannot be changed after creation" validations in CustomField (lib/mv/membership/custom_field.ex:92) and MembershipFeeType (lib/mv/membership_fees/membership_fee_type.ex:83): the :update actions do not accept value_type/interval, so updates fail earlier with NoSuchInput. Remove the validations; the tests already pin NoSuchInput.
Mv.Helpers.SystemActor cache has no test path. Under the SQL sandbox get_system_actor_result/0 always loads directly (lib/mv/helpers/system_actor.ex:115), so the Agent cache and invalidate_cache/0 never run in tests. Make the sandbox bypass injectable, or test the Agent path with a non-sandbox loader; if neither is worth it, drop the cache.
Ash snapshot drift (mix ash.codegen --check reports pending generation on main; not part of just ci-dev): priv/resource_snapshots/repo/extensions.json is at ash_functions_version 5 while ash_postgres expects 7; stale properties/property_types snapshot dirs ask to drop those tables; join_requests.reviewed_by_display was added by a hand-written migration without a snapshot. Regenerate, review the resulting migration (it must not drop live data), and consider adding mix ash.codegen --check to just ci-dev.

Acceptance criteria

Each item is removed or documented as intentionally kept; just ci-dev green.
mix ash.codegen --check reports nothing pending; the generated migration is reviewed and drops nothing in use.

## Motivation #605 and #609 left a few code paths with no caller, validations that can never run, a cache that no test exercises, and Ash snapshots that drifted from the code. None is user-visible, but each misleads the next reader or makes `mix ash.codegen --check` noisy. Collected from: #605 leftovers (three items), #609 findings 2 and 9. Verified on `main` `c3d99735` (2026-10-25). ## Findings - [ ] **Uncalled `Mv.Constants.vereinfacht_required_member_fields/0`** (`lib/mv/constants.ex:69,79`), documented in `docs/vereinfacht-api.md:30,57` and the CODE_GUIDELINES tree. Remove it with the docs, or keep it and state why in its doc. - [ ] **Setting actions without a production caller:** `:update_member_field_visibility` and `:update_single_member_field_visibility` (`lib/mv/membership/setting.ex:205`; wrappers in `lib/mv/membership/membership.ex:212,246`, used only by tests and doctests). Drop actions and wrappers together per CODE_GUIDELINES §1.3, or document why they stay. - [ ] **Unreachable "cannot be changed after creation" validations** in `CustomField` (`lib/mv/membership/custom_field.ex:92`) and `MembershipFeeType` (`lib/mv/membership_fees/membership_fee_type.ex:83`): the `:update` actions do not accept `value_type`/`interval`, so updates fail earlier with `NoSuchInput`. Remove the validations; the tests already pin `NoSuchInput`. - [ ] **`Mv.Helpers.SystemActor` cache has no test path.** Under the SQL sandbox `get_system_actor_result/0` always loads directly (`lib/mv/helpers/system_actor.ex:115`), so the Agent cache and `invalidate_cache/0` never run in tests. Make the sandbox bypass injectable, or test the Agent path with a non-sandbox loader; if neither is worth it, drop the cache. - [ ] **Ash snapshot drift** (`mix ash.codegen --check` reports pending generation on `main`; not part of `just ci-dev`): `priv/resource_snapshots/repo/extensions.json` is at `ash_functions_version` 5 while ash_postgres expects 7; stale `properties`/`property_types` snapshot dirs ask to drop those tables; `join_requests.reviewed_by_display` was added by a hand-written migration without a snapshot. Regenerate, review the resulting migration (it must not drop live data), and consider adding `mix ash.codegen --check` to `just ci-dev`. ## Acceptance criteria - Each item is removed or documented as intentionally kept; `just ci-dev` green. - `mix ash.codegen --check` reports nothing pending; the generated migration is reviewed and drops nothing in use.

simon added this to the Code and Test Refactoring | TI I milestone

2026-10-30 14:38:49 +01:00

simon added this to the Sprint 18: Oktober 2026 project

2026-10-30 14:38:49 +01:00

simon changed title from ~~# Remove uncalled code and unreachable validations, and bring the Ash resource snapshots up to date~~ to Remove uncalled code and unreachable validations, and bring the Ash resource snapshots up to date

2026-10-30 14:39:03 +01:00