Admin Settings: move Latest Features to the bottom and split the settings template - #1297
Open
Paul Lizer (paullizer) wants to merge 7 commits into
Open
Conversation
Admin Settings had grown to 13,526 lines in a single 1 MB template, which
made the upcoming information architecture rework impractical to do safely.
Each tab pane now lives in application/single_app/templates/admin/_panes/
and is pulled back in with {% include %}. The parent template keeps the
global form, the modals, and the script blocks, so the submitted payload is
unchanged: every field name, and all 109 configuration card ids, are
byte-identical to before the split.
Functional tests that read admin_settings.html straight from disk would now
only see the parent shell, so they route through a new shared helper,
test_support.templates, which inlines the Admin Settings partials and leaves
every other repository file untouched.
Adds test_admin_settings_template_composition.py to pin the contract: the
parent delegates to partials, no partial is orphaned, composition restores
the full card inventory, and no functional test asserts on a partial-backed
card without composing the template first.
Verified against the 75 functional test files that touch admin_settings.html:
32 pre-existing failures before and after, identical sets, zero regressions.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Latest Features was pinned first in both the top-tab strip and the admin sidebar, and its pane was hard-coded as the default active tab, so it opened on every single visit to Admin Settings. That made a curated release-notes tab behave like the landing page. It now sits last in both navigations, after Send Feedback, and General is the default landing tab instead. The Latest Features markup itself is unchanged, including its New badge and the hide/unhide dropdown. Three places drove the old behaviour and all three are updated: the top-tab button ordering and active class, the tab-pane show active class, and the sidebar bootstrap default in admin_sidebar_nav.js. Test assertions that pinned the old ordering are inverted to pin the new intent, so a future change cannot silently restore the landing behaviour. Field names and card ids remain byte-identical, and the 75 functional test files covering admin_settings.html show the same 32 pre-existing failures as before the change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Two Admin Settings features were rendered as afterthoughts compared to every other tab. Global Identities was a bare JavaScript mount point: no heading, no description, and no intro copy, so the tab rendered as an unlabelled widget. It now has a heading and explains that identities are deployment-wide and that secrets are held in Key Vault when Key Vault storage is configured, which matches the behaviour in functions_workspace_identities.py. File Sync is one of the larger settings surfaces at roughly 400 lines and 32 inputs, but it was the only tab with no sidebar submenu, so none of its sub-areas could be reached or searched from the navigation. Its source-type and per-workspace-type areas now carry ids and are exposed as sidebar destinations. The existing data-testid hooks are left in place. The composition guard added with the template split is extended to cover form field names as well as card ids. That immediately surfaced seven more tests that read admin_settings.html without composing its partials, including ones asserting on fields rather than cards, and those are now routed through the shared helper. Field names and card ids remain byte-identical, and the 75 functional test files covering admin_settings.html show the same 32 pre-existing failures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The classification banner live preview never worked. Its script sat between
{% endblock %} and {% block scripts %} in a child template, and Jinja drops
content outside blocks in a child template, so the code was never rendered.
Moving it inside the scripts block makes the preview respond to banner text,
background colour, and text colour changes.
Bumps VERSION to 0.260.004 and adds the fix write-up plus release notes for
this delivery stage: the template split, the Latest Features reposition, the
Global Identities and File Sync surfaces, and this preview fix.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
scrollToSection resolves a sidebar target with sectionMap[sectionId] || sectionId, so an entry that maps a key to itself does nothing. The map had grown to 72 entries: 66 were self-referencing no-ops, two were referenced by no sidebar link, and control-center-admin-section pointed at an element that does not exist anywhere in the template. Every new admin section had been registered twice, once as a sidebar link and once in a map that did not need it, so the map drifted out of sync with the markup without anything failing. Reduced to the six genuine aliases. A new test in the sidebar parity suite fails if a no-op, dangling, or unreferenced entry is added back, so the map cannot rot again. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Two alerts were reported against this branch. js/xss-through-dom (high) in admin_settings.html: showPreview fell back to assigning the raw editor contents to innerHTML when the Markdown editor had not initialized, so text typed into the editor was reinterpreted as HTML. The comment already stated the intent was to just show raw text, so the fallback now uses textContent, which removes the sink and matches that intent. The Markdown branch in the same function also wrote rendered HTML straight to innerHTML. It is now passed through DOMPurify, following the pattern the User Agreement preview in this same template already uses. DOMPurify comes from the locally served static bundle, so no external asset is introduced. py/unused-import in test_admin_settings_template_composition.py: resolve_template_includes was imported but only appeared as a string literal inside COMPOSITION_HELPERS, so the import was genuinely unused. Bumps VERSION to 0.260.005 with release notes and fix documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The previous fix assigned DOMPurify output through an intermediate variable, which cleared CodeQL but tripped the repository's own xss-sink-check. That checker only treats an identifier as safe when it is declared directly from a sanitizer call, so reassigning inside an availability guard left the innerHTML sink looking unsanitized on the changed line. DOMPurify is loaded globally from the local vendored bundle in base.html, before the page script block, so the guard was not buying anything. Sanitizing inline at the sink is both simpler and visible to the checker. Verified locally with scripts/check_xss_sinks.py and functional_tests/test_xss_guardrails_checker.py, both passing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
Copilot resolve the merge conflicts in this pull request |
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
2 participants
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.
First delivery stage of the Admin Settings information architecture rework. Everything here is verified non-breaking: no settings behaviour changes and no test regressions.
Why
Admin Settings had grown organically into 17 flat tabs in a single 13,526-line, 1 MB template. The most visible symptom was that Latest Features opened on every single visit — a curated release-notes tab behaving like the landing page.
What changed
Latest Features no longer hijacks the page
Three separate places forced this and all three are fixed:
<li>,class="nav-link active"class="tab-pane fade show active"admin_sidebar_nav.jsshowAdminTab('latest-features')Latest Features now sits last in both navigations, after Send Feedback, and General is the landing tab. Its content, New badge, and hide/unhide option are untouched.
Template split into per-tab partials
Each tab pane moved verbatim into
templates/admin/_panes/and is pulled back in with{% include %}. The parent keeps the global<form>, the modals, and the script blocks, and drops from 13,526 lines to about 2,200.Two features that were rendered as afterthoughts
Bugs found along the way
{% endblock %}and{% block scripts %}, and Jinja discards content outside blocks in a child template, so it was never rendered..valuetoinnerHTMLdespite the comment saying "just show raw text" (CodeQLjs/xss-through-dom, high). NowtextContent, with the Markdown branch sanitized inline viaDOMPurify.sanitize(...)at the sink.sectionMaphad rotted. 72 entries, of which 66 mapped a key to itself (already handled by the|| sectionIdfallback), two were unreferenced, andcontrol-center-admin-sectionpointed at an element that doesn't exist. Now 6 real aliases, with a test preventing regrowth.Verification
The save payload cannot have changed. Every
name=attribute and card id was fingerprinted before any edit and compared after each stage:Zero test regressions. All 75 functional test files referencing
admin_settings.html, run against the pre-change baseline and after every stage: 32 failures before, 32 after, identical sets. Those 32 are pre-existing and unrelated (workflow route registration, Cosmos document-access wiring, Send Feedback docs).All 20 admin templates parse under Jinja.
scripts/check_xss_sinks.pyandfunctional_tests/test_xss_guardrails_checker.pyboth pass. CodeQL reports 0 open alerts.Note for reviewers
Tests that read
templates/admin_settings.htmldirectly now see only the parent shell. Usetest_support.templates.read_admin_settings_template(), orcompose_if_admin_settings()inside a shared file-reading helper.The new
test_admin_settings_template_composition.pyenforces this — it fails if a test references a partial-backed card or form field while reading the template uncomposed. It caught 10 such tests, including 3 that were passing only because the assertion happened to be satisfied by the parent shell.check-release-notesfails with403 Resource not accessible by integrationwhile trying to post its own comment. That is a fork-PR token permission issue, not a code problem; the comment it wanted to leave is an explicitly non-blocking reminder.Still to come
The wider IA rework (14 groups / 42 tabs) builds on this split: the group navigation level, re-homing cards into groups, splitting
system-settings-section(which mixes upload, chat, session, and access-control settings in one card), a consolidated Security → Access and Roles view mirroring the tenrequire_member_of_*toggles currently spread across six tabs, and a legacy hash redirect map.Implementation surfaced three things that make the re-homing riskier than the original plan assumed, so it is deliberately held for a follow-up rather than rushed into this PR:
conversation-contents-drawer-section,content-understanding-section,office-embedded-image-section) and cannot move independently.Version
0.260.005. Fix documentation indocs/explanation/fixes/ADMIN_SETTINGS_NAVIGATION_AND_TEMPLATE_STRUCTURE_FIX.md.