feat(settings): redesigned UI, new field types, decoded-value API#211
Merged
Conversation
Rewrite SettingField.tsx as thin dispatcher and add per-type field components under components/fields/ with clean decoded-value contract (no JSON encode/decode).
Deploying simplemodule-website with
|
| Latest commit: |
ff709fe
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5e7b561f.simplemodule-website.pages.dev |
| Branch Preview URL: | https://settings-refactor-v2.simplemodule-website.pages.dev |
… has no results BUG-7: handleSave, handleReset, and handleBulkSave in both AdminSettings and UserSettings now check response.ok before updating local state. On failure, an inline danger alert surfaces the error detail parsed from ValidationProblemDetails (falling back to the HTTP status code). Local state is only updated after a confirmed successful response. BUG-8: AdminSettings.renderGroups() always renders SettingsLayout (toolbar + sidebar nav) regardless of result count. The empty state (using EmptyState from @simplemodule/ui with a "Clear search" action) is rendered inside the content area so the search input and Bulk edit toggle are never unmounted. UserSettings applies the same fix: the sticky toolbar already survived empty results, but the sidebar nav was conditional; it now always renders, and the empty state uses EmptyState with the same clear-search action. New i18n keys: AdminSettings.NoResults, AdminSettings.SaveError, AdminSettings.SaveErrorTitle, UserSettings.NoResults, UserSettings.SaveError, UserSettings.SaveErrorTitle.
…r-scope write
- GAP-PERM: UpdateSetting, DeleteSetting, BulkUpdateSettings now require
Settings.Update permission (403 for authenticated users without it)
- BUG-3: GET /api/settings/{key} without ?scope now returns 400 instead of 500
- BUG-4: DELETE /api/settings/{key} without ?scope now returns 400 instead of 500
- BUG-6: PUT /api/settings and PUT /api/settings/bulk reject scope=User with 400,
directing callers to use /api/settings/me instead
- Tests: update existing write tests to use Settings.Update permission; add new
tests covering 403, 400-missing-scope, and 400-user-scope cases
…an reject BUG-1: Skip ValidatePattern for Email/Url/Color/DateTime types to prevent duplicate validation errors when a Pattern is also configured. BUG-2: URL validator now requires http/https scheme, rejecting file:// URIs and relative paths like /foo/bar. BUG-5: SetManyAsync now checks for User-scope entries upfront and throws SettingValidationException (→ 400) instead of InvalidOperationException (→ 500). Tests updated and new regression tests added for all three bugs.
…on, API contract)
Both relied on prior state and would flake under parallel execution. Now each pre-cleans via API and the Bulk discard test picks a distinct value that's guaranteed to differ from any persisted state.
PR #211 gated /api/settings on Settings.Update permission and made the value field a decoded JsonElement (not a JSON-encoded string). The durability test needed both: an admin client with the permission claim, and a plain 'hello' instead of '"hello"'.
antosubash
added a commit
that referenced
this pull request
May 24, 2026
…er endpoint (#215) * feat(core): add Form Request classes for typed binding + validation per endpoint (#163) Introduces Laravel-style FormRequest<TSelf> base class that bundles parameter binding, authorization, validation rules, and data normalization into a single class. The endpoint handler receives an already-valid request object. Pipeline: Bind → Authorize (403) → Prepare → Validate (422) → Handler - FormRequest base + [FormRequest] attribute in SimpleModule.Core - Source generator discovers [FormRequest] types, validates shape (SM0056/SM0057), emits TypeScript interfaces (same path as [Dto]) - FormRequestEndpointFilter auto-applied to all module route groups - FluentValidation under the hood via RuleConfigurator<T> - Validator cached per type for performance - 422 Unprocessable Entity with RFC 7807 problem+json for validation failures - Email module CreateTemplateEndpoint refactored as reference implementation - 16 new xUnit tests covering binding, validation, authorization, and prepare hooks - Constitution updated with FormRequest rules and SM0056/SM0057 diagnostics * fix(core): address architecture review — filter coverage, shared Inertia errors, testability - Fix C3: modules without RoutePrefix now get a group with AddFormRequestFilter() instead of mapping directly to app (no-prefix endpoints were silently skipping the FormRequest validation pipeline) - Fix C1: ConfigureEndpoints escape hatch now wraps in a group with AddFormRequestFilter() so FormRequest types work in escape-hatch modules - Fix M4: extract shared InertiaErrorResult utility used by both FormRequestEndpointFilter and GlobalExceptionHandler (removes duplication) - Fix m2: add public ValidateRulesAsync() method on FormRequest so consumers can unit-test their validation rules without constructing filter context - Update generator tests to match new generated code shape * fix(core): address code review findings — dedup, auth, caching, API gaps - Fix duplicate TS interfaces when both [Dto] and [FormRequest] on same class (DtoFinder now deduplicates FQNs before the [FormRequest] scan) - Restore .RequirePermission() on CreateTemplateEndpoint for ASP.NET auth metadata visibility (OpenAPI/Swagger, policy-based audit); remove redundant FormRequest.Authorize() override from CreateTemplateFormRequest - Revert escape-hatch ConfigureEndpoints to receive WebApplication (not RouteGroupBuilder) — avoids breaking contract for modules that cast to IApplicationBuilder - ValidateRulesAsync now calls Prepare() before validation, consistent with the filter pipeline - Add When/Unless forwarding to RuleConfigurator for grouped conditional rules - Replace ConcurrentDictionary validator cache with volatile + Interlocked (single-entry-per-type, simpler, avoids redundant dictionary overhead) * feat(settings): convert Settings endpoints to FormRequest pattern Convert 4 Settings module endpoints to use FormRequest for input validation: - UpdateSettingEndpoint (PUT /api/settings) — validates key format, scope enum - UpdateMySettingEndpoint (PUT /api/settings/me) — reuses UpdateSettingFormRequest - CreateMenuItemEndpoint (POST /api/settings/menus) — validates label, URL length - UpdateMenuItemEndpoint (PUT /api/settings/menus/{id}) — validates same as create Demonstrates dual-validation: FormRequest validates shape (key format, field lengths, enum membership), service layer validates domain (setting type/range against definitions). FormRequest returns 422, service exceptions return 400. Symmetric validation: both Create and Update menu endpoints now share the same rules; both admin and user setting endpoints use the same FormRequest. Includes 16 new integration tests verifying 422 responses, Prepare normalization, RFC 7807 shape, and auth-before-validation ordering. * fix(settings): address code review — regex, scope, cascade, test semantics - Fix regex to allow camelCase/PascalCase keys (email.defaultFromAddress, FileStorage.MaxFileSizeMb, etc.) and enforce segment-based validation that rejects trailing dots and empty segments - Create dedicated UpdateMySettingFormRequest (Key + Value only) for PUT /api/settings/me — removes misleading Scope field that was validated but silently ignored - Fix double error messages for empty key by splitting rules with .When() guard so regex only fires on non-empty keys - Rename misleading test: NullValue "clears setting" → actually stores empty string (documents real behavior, not aspirational) - Add tests for camelCase keys and trailing-dot rejection * fix(settings): adapt FormRequest tests to upstream API changes Upstream PR #211 changed UpdateSettingRequest.Value from string? to JsonElement and added .RequirePermission(SettingsPermissions.Update). Update FormRequest integration tests to: - Use JsonSerializer.Deserialize<JsonElement>() for Value fields - Pass SettingsPermissions.Update to CreateAuthenticatedClient - Replace hyphenated test keys with camelCase (regex rejects hyphens)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Select,Color,Url,Email,Password,MultilineText,DateTimeextend the existing 4.SettingDefinitiongainsAllowedValues,Min/Max,Pattern,Required,Sensitive,Order,Placeholder.SettingValueDto/UserSettingValueDtocarry decodedJsonElementvalues. New endpoints:PUT /api/settings/bulk,DELETE /api/settings/{key}(reset),GET /api/settings/{key}/resolved. Server-side validation against the newSettingDefinitionrules. Sensitive values masked.components/fields/—SettingField.tsxis now a thin dispatcher.Card/Section/SearchInput/Badge/EmptyStateprimitives.PUT/DELETE/bulk) now gated onSettings.Updatepermission (was bare.RequireAuthorization()— any authed user could write System settings).GET/DELETE /api/settings/{key}return 400 instead of 500 when?scope=missing.PUT /api/settingsrejectsscope=User(prevents ghostUserId=nullrows).SetManyAsyncthrowsSettingValidationExceptionfor User scope → endpoint converts to 400.SettingValidator: Email/Url/Color/DateTime skip the userPatternregex (no duplicate errors); URL requireshttp/httpsscheme (notfile://on Linux).handleSave/handleReset/handleBulkSavecheckres.ok, parseValidationProblemDetails, show inline danger alert (was silent failure).Verification
Admin Settings — System tab
Admin Settings — search + new ColorField
User Settings — inheritance display + SelectField
Local CI
dotnet format --verify-no-changes✓npm run check(biome + typecheck across 13 modules) ✓dotnet build -warnaserror✓ (0 errors, 0 warnings)dotnet test✓ — 1,107/1,108 tests pass. Settings module: 104/104.npx playwright test(settings) ✓ — 66/66 settings e2e pass.Known pre-existing issues on
mainThese fail on
maintoo (verified by stashing and re-running); not introduced by this branch:EventDurabilityE2ETests.PublishedEvent_Survives_The_Durable_Pipeline_End_To_End— durability E2E flake (also fails in this PR's CI build).feature-flags-crude2e, 2×filestorage-crudunauthenticated, 1×permissions.spec.ts › can access settings page(404 on/settings— wrong route), 1×permissions.spec.ts › admin API rejects unauthenticated request.Test plan
/settings/manageand confirms System/Application tabs, group sidebar, search filter, scope badges, and the new sample settings (Primary Color, Support Email, Welcome Message)/settings/meand confirms the inheritance line, Display Density Select, and Reset to default flowPUT /api/settingsas a non-admin user (expect 403)EventDurabilityE2ETestsflake)