test(frontend): pin the admin settings form's wiring - #7426
Open
aglinxinyuan wants to merge 1 commit into
Open
Conversation
The settings form is four near-identical Save/Reset cards, three near-identical upload blocks, and twelve switches whose keys include two confusable singular/plural pairs: workflow_enabled vs workflows_enabled, and dataset_enabled vs datasets_enabled. Cross-wiring from copy-paste is the realistic defect, and the existing suite calls the component's methods directly without ever rendering an interaction. Adds 12 tests. The central one walks the twelve switches in template order and asserts each flips exactly one setting and no other, which is what catches a swap between the confusable pairs. Alongside it: the Hub and Your Work children being locked until their section is on, the three section switches never being locked, each number input owning its own field, each card's Save and Reset reaching that card's handler, each picker opening its own hidden input, each file input tagging its change with its own setting key, and the previews appearing only for images that have been chosen. No production file is touched.
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7426 +/- ##
============================================
+ Coverage 85.00% 85.15% +0.15%
Complexity 4148 4148
============================================
Files 1169 1169
Lines 46740 46740
Branches 5202 5202
============================================
+ Hits 39731 39803 +72
+ Misses 5289 5216 -73
- Partials 1720 1721 +1
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds interaction-level unit tests that render AdminSettingsComponent and validate the template wiring for the sidebar switches, numeric inputs, card Save/Reset buttons, branding upload pickers, and preview rendering—specifically targeting copy/paste cross-wiring risks in the admin settings template.
Changes:
- Introduces a new “wiring” test suite that drives the template via DOM events (switch toggles, input changes, button clicks, file input changes).
- Adds assertions that each sidebar switch flips exactly its intended config key (including the singular/plural near-collisions).
- Adds coverage for enable/disable gating of child switches, per-field numeric bindings, upload picker/input routing, and conditional preview rendering.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+468
to
+472
| fixture.detectChanges(); | ||
| // ngOnInit loads the settings; answer it so the form starts from a known state. | ||
| http.expectOne("/api/config/settings").flush({}); | ||
| fixture.detectChanges(); | ||
| }); |
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
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.
What changes were proposed in this PR?
The settings form is four near-identical Save/Reset cards, three near-identical upload blocks, and twelve sidebar switches. All 31 of its template listeners and all 34 of its branches were unhit, because the existing suite calls the component's methods directly and never renders an interaction.
The realistic defect in a template shaped like this is cross-wiring from copy-paste, and two switch keys are one character apart:
workflow_enabledvsworkflows_enableddataset_enabledvsdatasets_enabledA swap between either pair is invisible on screen and silently toggles the wrong sidebar entry.
Adds 12 tests. The central one walks the twelve switches in template order and asserts each flips exactly one setting and no other — that is what catches a swap between the confusable pairs. Alongside it:
Verified by mutation, all reverted (template diff empty):
workflowsswitch writeworkflow_enableddatasetsswitch writedataset_enabledlogomaxFileSizeMiBThe preview mutation survived its first run: the test set
logoData, so removing that image's*ngIfchanged nothing. A no-data case now covers it — without the guard a card renders a broken image on first load.No production file is touched.
Any related issues, documentation, discussions?
Closes #7423
How was this PR tested?
12 new on top of the existing 24.
yarn format:cipasses.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)