fix(settings): drop legacy DELETE /settings/{id} - matchit conflict with /settings/{category} #88

Merged

YousifShkara merged 1 commit from fix/settings-route-conflict into main

2026-06-05 09:21:07 +02:00

YousifShkara commented

2026-06-05 09:21:04 +02:00

Owner

The PMS-113 PR (#87) added GET /settings/{category} for the new category-scoped tenant_settings endpoint but left the legacy DELETE /settings/{id} route registered. Axum 0.8's matchit router treats /settings/{id} and /settings/{category} as the same path shape (the param name doesn't matter, only the segment count) and refuses to register both at router build time. Every integration test that boots the router now panics:

Invalid route "/settings/{category}": Insertion failed due to
conflict with previously registered route: /settings/{id}

at src/modules/settings/routes.rs:44 - 10 of 10 auth tests fail in CI.

The new per-key DELETE /settings/{category}/{key} (already shipped in PMS-113) is the canonical delete path; the legacy by-id form was redundant. Drop the route + the orphaned delete_setting handler + the orphaned SettingsService::delete_tenant_setting method (no other callers).

Verified locally:

cargo check --tests
cargo clippy --tests -- -Dwarnings
cargo fmt --all --check

cargo test --test auth deferred to CI; the dev-01 dev stack is still down.

The mokosh-clients SPA does not consume DELETE /api/v1/settings/{id} anywhere (verified by grep); the only /settings/... references in the SPA are client-side Dioxus router paths, not API consumers.

The PMS-113 PR (#87) added `GET /settings/{category}` for the new category-scoped tenant_settings endpoint but left the legacy `DELETE /settings/{id}` route registered. Axum 0.8's matchit router treats `/settings/{id}` and `/settings/{category}` as the same path shape (the param name doesn't matter, only the segment count) and refuses to register both at router build time. Every integration test that boots the router now panics: Invalid route "/settings/{category}": Insertion failed due to conflict with previously registered route: /settings/{id} at src/modules/settings/routes.rs:44 - 10 of 10 auth tests fail in CI. The new per-key `DELETE /settings/{category}/{key}` (already shipped in PMS-113) is the canonical delete path; the legacy by-id form was redundant. Drop the route + the orphaned `delete_setting` handler + the orphaned `SettingsService::delete_tenant_setting` method (no other callers). Verified locally: - cargo check --tests - cargo clippy --tests -- -Dwarnings - cargo fmt --all --check cargo test --test auth deferred to CI; the dev-01 dev stack is still down. The mokosh-clients SPA does not consume `DELETE /api/v1/settings/{id}` anywhere (verified by grep); the only `/settings/...` references in the SPA are client-side Dioxus router paths, not API consumers.

YousifShkara self-assigned this

2026-06-05 09:21:04 +02:00

YousifShkara added 1 commit

2026-06-05 09:21:04 +02:00

fix(settings): drop legacy DELETE /settings/{id} - matchit conflict with /settings/{category}

Create release / Create release from merged PR (pull_request) Has been skipped

Details

Build OCI container / Build and push mokosh-api image (push) Successful in 3m17s

Details

Check / fmt + clippy + compile + tests (pull_request) Successful in 1m28s

Details

a0d3d918f5

The PMS-113 PR (#87) added `GET /settings/{category}` for the new category-scoped tenant_settings endpoint but left the legacy `DELETE /settings/{id}` route registered. Axum 0.8's matchit router treats `/settings/{id}` and `/settings/{category}` as the same path shape (the param name doesn't matter, only the segment count) and refuses to register both at router build time. Every integration test that boots the router now panics:

    Invalid route "/settings/{category}": Insertion failed due to
    conflict with previously registered route: /settings/{id}

at src/modules/settings/routes.rs:44 - 10 of 10 auth tests fail in CI.

The new per-key `DELETE /settings/{category}/{key}` (already shipped in PMS-113) is the canonical delete path; the legacy by-id form was redundant. Drop the route + the orphaned `delete_setting` handler + the orphaned `SettingsService::delete_tenant_setting` method (no other callers).

Verified locally:
- cargo check --tests
- cargo clippy --tests -- -Dwarnings
- cargo fmt --all --check

cargo test --test auth deferred to CI; the dev-01 dev stack is still down.

The mokosh-clients SPA does not consume `DELETE /api/v1/settings/{id}` anywhere (verified by grep); the only `/settings/...` references in the SPA are client-side Dioxus router paths, not API consumers.

YousifShkara merged commit 4b4c1f4b10 into main

2026-06-05 09:21:07 +02:00

YousifShkara deleted branch fix/settings-route-conflict

2026-06-05 09:21:08 +02:00

YousifShkara referenced this pull request from a commit

2026-06-05 09:21:09 +02:00

Merge pull request 'fix(settings): drop legacy DELETE /settings/{id} - matchit conflict with /settings/{category}' (#88) from fix/settings-route-conflict into main