Website: queries » reports - #43871
Merged
Hidden character warning
The head ref may contain hidden characters: "website-query\u00bbreport"
Merged
Conversation
fleet-release
added
the
~ga4-annotation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
fleet-release
previously approved these changes
Apr 21, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Renames the fleetdm.com website “Queries” docs section and related UI to “Reports”, introducing new /reports and /report-generator URLs and adding redirects from the legacy query URLs to preserve compatibility/SEO.
Changes:
- Introduce new report library/details pages and controllers, and route
/reports+/reports/:slugto them. - Update navigation, sitemap generation, and assorted site copy to use “reports” terminology.
- Add redirects from
/queries,/queries/:slug, and/query-generatorto the new canonical URLs.
Reviewed changes
Copilot reviewed 12 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| website/views/pages/homepage.ejs | Updates marketing tooltip copy from queries → reports. |
| website/views/pages/docs/vital-details.ejs | Renames “built-in queries” copy to “built-in reports”. |
| website/views/pages/docs/report-library.ejs | New Reports library page (replacing query library). |
| website/views/pages/docs/report-details.ejs | New Report details page (replacing query detail). |
| website/views/pages/device-management.ejs | Updates marketing tooltip copy from queries → reports. |
| website/views/pages/admin/query-generator.ejs | Rebrands query generator page copy to “Report robot”. |
| website/views/layouts/layout.ejs | Swaps included docs scripts from query-* to report-*. |
| website/config/routes.js | Adds /reports routes and redirects from legacy query routes. |
| website/assets/styles/pages/docs/report-library.less | Renames CSS root selector for report library page. |
| website/assets/styles/pages/docs/report-details.less | Renames CSS selectors for report details page. |
| website/assets/styles/importer.less | Replaces imported query-* LESS with report-* LESS. |
| website/assets/js/pages/docs/report-library.page.js | Registers Parasails page under report-library. |
| website/assets/js/pages/docs/report-details.page.js | Registers Parasails page under report-details. |
| website/assets/js/components/docs-nav-and-search.component.js | Updates docs top-nav link label/route to Reports. |
| website/api/controllers/download-sitemap.js | Switches sitemap entries from /queries to /reports. |
| website/api/controllers/docs/view-report-library.js | Adds controller to serve report library page. |
| website/api/controllers/docs/view-report-details.js | Updates controller to serve report details + metadata. |
| website/api/controllers/docs/view-query-library.js | Removes old query library controller. |
Comments suppressed due to low confidence (5)
website/views/pages/docs/report-details.ejs:27
- This copy still refers to "queries" and the SQL tab label is lowercase "report". For consistency with the rename and other tabs, consider updating the sentence to refer to reports and capitalizing the tab label (e.g., "Report").
website/api/controllers/docs/view-report-details.js:76 - The fallback meta description still says "standard query library" even though this page is now "Report details". Update the fallback description to refer to the report library so page metadata remains accurate when a report is missing description text.
website/views/pages/docs/report-library.ejs:52 - The Linux tab is rendering
windowsReports(and the Windows tab below renderslinuxReports), so the platform filters will show the wrong report lists. Swap these to uselinuxReportsfor the Linux block.
website/views/pages/docs/report-library.ejs:67 - The Windows tab is rendering
linuxReports, which will show the wrong list when users select Windows. It should iterate overwindowsReportshere.
website/views/pages/docs/report-library.ejs:10 - This link still points to
/query-generator, which will add an extra redirect hop now that the canonical URL is/report-generator. Update the href to the new route to keep internal links canonical.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+59
to
65
| 'GET /reports': { | ||
| action: 'docs/view-report-library', | ||
| locals: { | ||
| currentSection: 'more', | ||
| pageTitleForMeta: 'Queries', | ||
| pageDescriptionForMeta: 'A growing collection of optional queries you can run anytime to ask questions about your devices using Fleet and osquery.' | ||
| } |
There was a problem hiding this comment.
/reports now serves the renamed reports page, but the meta title/description locals still say "Queries" and describe queries. Update these locals to reflect "Reports" to avoid incorrect page metadata (SEO/social previews).
| 'GET /query-generator': '/report-generator', | ||
| 'GET /queries/:slug': { | ||
| fn: (req, res) => { | ||
| return res.redirect('/reports/' + req.param('slug')); |
There was a problem hiding this comment.
This redirect for the old /queries/:slug URLs uses the default redirect status (typically 302). For a rename intended to be permanent (and for SEO), use a 301 redirect here (consistent with other redirects in this file that call res.redirect(301, ...)).
Suggested change
| return res.redirect('/reports/' + req.param('slug')); | |
| return res.redirect(301, '/reports/' + req.param('slug')); |
| <div class="invalid-feedback" v-if="formErrors.naturalLanguageQuestion" focus-first>Ask your question.</div> | ||
| </div> | ||
| <cloud-error v-if="cloudError">An error occurred while generating your queries. Please <a href="/query-generator">reload this page and try again</a>.</cloud-error> | ||
| <cloud-error v-if="cloudError">An error occurred while generating your queries. Please <a href="/report-generator">reload this page and try again</a>.</cloud-error> |
There was a problem hiding this comment.
The page is now branded as the report generator, but this error message still says "generating your queries". Update the copy to "generating your reports" to avoid confusing users.
Suggested change
| <cloud-error v-if="cloudError">An error occurred while generating your queries. Please <a href="/report-generator">reload this page and try again</a>.</cloud-error> | |
| <cloud-error v-if="cloudError">An error occurred while generating your reports. Please <a href="/report-generator">reload this page and try again</a>.</cloud-error> |
Contributor
WalkthroughThis PR renames the website docs surface from "queries" to "reports." It removes the Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
website/views/layouts/layout.ejs (1)
435-435:⚠️ Potential issue | 🟡 MinorUpdate admin generator links to the new report route.
These nav entries still show “Generate queries” and link to
/query-generator, so admins see stale terminology and hit the redirect instead of the new route.Suggested fix
- <a purpose="mobile-dropdown-link" href="/query-generator">Generate queries</a> + <a purpose="mobile-dropdown-link" href="/report-generator">Generate reports</a>- <a purpose="admin-link" style="text-decoration: none; line-height: 23px;" href="/query-generator">Generate queries</a> + <a purpose="admin-link" style="text-decoration: none; line-height: 23px;" href="/report-generator">Generate reports</a>Also applies to: 718-718
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/views/layouts/layout.ejs` at line 435, Update the nav anchors that still say "Generate queries" and point to "/query-generator": locate the <a> elements with purpose="mobile-dropdown-link" (and the matching desktop nav anchor around the same code block) and change the link text to "Generate reports" and the href to the new report route (replace "/query-generator" with the new report route used by the app); apply the same change to the other occurrence referenced in the comment so both mobile and desktop nav entries are updated.
🧹 Nitpick comments (1)
website/views/pages/docs/vital-details.ejs (1)
80-80: Backend properties still use "query" terminology despite UI being updated to "report".Several template references still use "query" terminology, but verification confirms the backend properties themselves were not renamed:
- Line 87:
thisVital.queryproperty is still namedqueryin view-vital-details.js- Lines 95, 101, 113:
queryLibraryYmlRepoPathis stillqueryLibraryYmlRepoPathin the backend (not renamed toreportLibraryYmlRepoPath)Since line 8 now describes vitals as "built-in reports," either:
- The backend properties should be renamed for consistency (e.g.,
thisVital.report,reportLibraryYmlRepoPath), or- The UI terminology should remain as "Query" to match the backend property names
Update to align backend and frontend terminology.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/views/pages/docs/vital-details.ejs` at line 80, The frontend UI was changed to "report" but backend properties still use "query" (e.g., thisVital.query and queryLibraryYmlRepoPath) causing a mismatch; pick one consistent naming scheme—preferably rename backend properties to "report" and update frontend bindings accordingly: rename backend fields queryLibraryYmlRepoPath -> reportLibraryYmlRepoPath and thisVital.query -> thisVital.report (and any API payloads/DB/serializers using those names), then update view-vital-details.js and the template (the tab text and any selectedTab values such as the "sql" tab binding) to use the new report names so all references (thisVital.report, reportLibraryYmlRepoPath, selectedTab values) match end-to-end.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/assets/js/components/docs-nav-and-search.component.js`:
- Line 41: The Reports nav link uses currentSection === 'reports' (see the
anchor with purpose="docs-top-nav-menu-link" in
docs-nav-and-search.component.js) but the routes set currentSection: 'more' for
/reports and /reports/:slug, so the active class never appears; fix by either
updating those route locals in routes.js to currentSection: 'reports' or
broadening the check in the component (e.g., treat 'more' as reports or check
both 'reports' and 'more') so the Reports link becomes active on report list and
detail pages.
In `@website/config/routes.js`:
- Around line 59-65: The route definition for 'GET /reports' (action
'docs/view-report-library') still uses the old metadata ("Queries" and a
query-focused description); update the locals.pageTitleForMeta and
locals.pageDescriptionForMeta values to the new copy for the /reports page by
editing the locals object in the 'GET /reports' route entry (change
pageTitleForMeta from 'Queries' and replace the pageDescriptionForMeta string
with the updated description).
In `@website/views/pages/admin/query-generator.ejs`:
- Line 15: Update the user-facing error message in the admin query-generator
view: replace the phrase "generating your queries" in the <cloud-error> block
that renders when cloudError is truthy so it refers to reports (e.g.,
"generating your report(s)") or neutral wording like "generating your report"
and keep the existing reload link to /report-generator; edit the cloud-error
element in website/views/pages/admin/query-generator.ejs that contains
cloudError to change the displayed copy accordingly.
In `@website/views/pages/docs/report-library.ejs`:
- Line 10: Update the hardcoded anchor href that points to the deprecated route
"/query-generator" so it links directly to the canonical "/report-generator"
route; locate the anchor element containing the string "/query-generator" in the
report-library template (the <a href="/query-generator">report robot</a>
occurrence) and change its href value to "/report-generator" so the page emits
the new route instead of a redirect.
- Around line 49-67: The platform branches are rendering the wrong arrays: in
the v-else-if where selectedPlatform === 'linux' the for loop iterates
windowsReports, and where selectedPlatform === 'windows' it iterates
linuxReports; update the for loops so the Linux branch iterates linuxReports and
the Windows branch iterates windowsReports (update the two for(let report of
...) occurrences that currently reference the wrong arrays while keeping the
report variable and the surrounding markup unchanged).
---
Outside diff comments:
In `@website/views/layouts/layout.ejs`:
- Line 435: Update the nav anchors that still say "Generate queries" and point
to "/query-generator": locate the <a> elements with
purpose="mobile-dropdown-link" (and the matching desktop nav anchor around the
same code block) and change the link text to "Generate reports" and the href to
the new report route (replace "/query-generator" with the new report route used
by the app); apply the same change to the other occurrence referenced in the
comment so both mobile and desktop nav entries are updated.
---
Nitpick comments:
In `@website/views/pages/docs/vital-details.ejs`:
- Line 80: The frontend UI was changed to "report" but backend properties still
use "query" (e.g., thisVital.query and queryLibraryYmlRepoPath) causing a
mismatch; pick one consistent naming scheme—preferably rename backend properties
to "report" and update frontend bindings accordingly: rename backend fields
queryLibraryYmlRepoPath -> reportLibraryYmlRepoPath and thisVital.query ->
thisVital.report (and any API payloads/DB/serializers using those names), then
update view-vital-details.js and the template (the tab text and any selectedTab
values such as the "sql" tab binding) to use the new report names so all
references (thisVital.report, reportLibraryYmlRepoPath, selectedTab values)
match end-to-end.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 94ae2a5f-ec96-4ad8-aa5a-7d3d8d84087d
📒 Files selected for processing (18)
website/api/controllers/docs/view-query-library.jswebsite/api/controllers/docs/view-report-details.jswebsite/api/controllers/docs/view-report-library.jswebsite/api/controllers/download-sitemap.jswebsite/assets/js/components/docs-nav-and-search.component.jswebsite/assets/js/pages/docs/report-details.page.jswebsite/assets/js/pages/docs/report-library.page.jswebsite/assets/styles/importer.lesswebsite/assets/styles/pages/docs/report-details.lesswebsite/assets/styles/pages/docs/report-library.lesswebsite/config/routes.jswebsite/views/layouts/layout.ejswebsite/views/pages/admin/query-generator.ejswebsite/views/pages/device-management.ejswebsite/views/pages/docs/report-details.ejswebsite/views/pages/docs/report-library.ejswebsite/views/pages/docs/vital-details.ejswebsite/views/pages/homepage.ejs
💤 Files with no reviewable changes (1)
- website/api/controllers/docs/view-query-library.js
| <a :class="[currentSection === 'controls' ? 'active' : '']" purpose="docs-top-nav-menu-link" href="/mdm-commands" style="text-decoration: none; text-decoration-line: none;">Controls</a> | ||
| <a :class="[currentSection === 'vitals' ? 'active' : '']" purpose="docs-top-nav-menu-link" href="/vitals" style="text-decoration: none; text-decoration-line: none;">Vitals</a> | ||
| <a :class="[currentSection === 'queries' ? 'active' : '']" purpose="docs-top-nav-menu-link" href="/queries" style="text-decoration: none; text-decoration-line: none;">Queries</a> | ||
| <a :class="[currentSection === 'reports' ? 'active' : '']" purpose="docs-top-nav-menu-link" href="/reports" style="text-decoration: none; text-decoration-line: none;">Reports</a> |
Contributor
There was a problem hiding this comment.
Align the Reports active state with route locals.
/reports and /reports/:slug currently provide currentSection: 'more', so this link never gets the active class on report pages. Update those route locals to currentSection: 'reports' or make this check match the value being passed.
Suggested fix
- <a :class="[currentSection === 'reports' ? 'active' : '']" purpose="docs-top-nav-menu-link" href="/reports" style="text-decoration: none; text-decoration-line: none;">Reports</a>
+ <a :class="[currentSection === 'reports' ? 'active' : '']" purpose="docs-top-nav-menu-link" href="/reports" style="text-decoration: none; text-decoration-line: none;">Reports</a>And in website/config/routes.js:
'GET /reports': {
action: 'docs/view-report-library',
locals: {
- currentSection: 'more',
+ currentSection: 'reports',
pageTitleForMeta: 'Queries',
pageDescriptionForMeta: 'A growing collection of optional queries you can run anytime to ask questions about your devices using Fleet and osquery.'
}
},
'GET /reports/:slug': {
action: 'docs/view-report-details',
locals: {
- currentSection: 'more',
+ currentSection: 'reports',
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <a :class="[currentSection === 'reports' ? 'active' : '']" purpose="docs-top-nav-menu-link" href="/reports" style="text-decoration: none; text-decoration-line: none;">Reports</a> | |
| <a :class="[currentSection === 'reports' ? 'active' : '']" purpose="docs-top-nav-menu-link" href="/reports" style="text-decoration: none; text-decoration-line: none;">Reports</a> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/assets/js/components/docs-nav-and-search.component.js` at line 41,
The Reports nav link uses currentSection === 'reports' (see the anchor with
purpose="docs-top-nav-menu-link" in docs-nav-and-search.component.js) but the
routes set currentSection: 'more' for /reports and /reports/:slug, so the active
class never appears; fix by either updating those route locals in routes.js to
currentSection: 'reports' or broadening the check in the component (e.g., treat
'more' as reports or check both 'reports' and 'more') so the Reports link
becomes active on report list and detail pages.
| <p>Want to create your own? Our <a href="/query-generator">query robot</a> can help.</p> | ||
| <h2>Reports</h2> | ||
| <p class="mb-1">A collection of optional reports you can run anytime. Contributions welcome <a target="_blank" href="https://github.com/fleetdm/fleet/edit/main/docs/queries.yml" no-icon>over on GitHub.</a></p> | ||
| <p>Want to create your own? Our <a href="/query-generator">report robot</a> can help.</p> |
Contributor
There was a problem hiding this comment.
Link directly to the canonical report generator.
Line 10 still emits /query-generator, which now redirects to /report-generator. Use the new route directly to avoid shipping deprecated internal links.
Proposed fix
- <p>Want to create your own? Our <a href="/query-generator">report robot</a> can help.</p>
+ <p>Want to create your own? Our <a href="/report-generator">report robot</a> can help.</p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/views/pages/docs/report-library.ejs` at line 10, Update the hardcoded
anchor href that points to the deprecated route "/query-generator" so it links
directly to the canonical "/report-generator" route; locate the anchor element
containing the string "/query-generator" in the report-library template (the <a
href="/query-generator">report robot</a> occurrence) and change its href value
to "/report-generator" so the page emits the new route instead of a redirect.
Comment on lines
49
to
67
| <div v-else-if="selectedPlatform === 'linux'"> | ||
| <% // Linux policies (server-side-rendered) | ||
| for(let query of linuxQueries) { | ||
| for(let report of windowsReports) { | ||
| %> | ||
| <div purpose="policy" class="d-flex flex-lg-row flex-column justify-content-between"> | ||
| <div purpose="policy-name-and-description" class="d-flex flex-column"> | ||
| <div class="d-flex flex-column"> | ||
| <p purpose="policy-name"><a href="/queries/<%- query.slug%>"><%- query.name %></a></p> | ||
| <p purpose="policy-name"><a href="/reports/<%- report.slug%>"><%- report.name %></a></p> | ||
| </div> | ||
| <div purpose="policy-description"><p><%- query.description %></p></div> | ||
| <div purpose="read-more-link"><animated-arrow-button arrow-color="#192147" href="/queries/<%- query.slug%>">Read more</animated-arrow-button></div> | ||
| <div purpose="policy-description"><p><%- report.description %></p></div> | ||
| <div purpose="read-more-link"><animated-arrow-button arrow-color="#192147" href="/reports/<%- report.slug%>">Read more</animated-arrow-button></div> | ||
| </div> | ||
| </div> | ||
| <% } %> | ||
| </div> | ||
| <div v-else-if="selectedPlatform === 'windows'"> | ||
| <% // Windows policies (server-side-rendered) | ||
| for(let query of windowsQueries) { | ||
| for(let report of linuxReports) { | ||
| %> |
Contributor
There was a problem hiding this comment.
Fix the swapped Linux and Windows report lists.
The Linux branch renders windowsReports, and the Windows branch renders linuxReports, so users see the wrong report set after selecting either platform.
Proposed fix
<div v-else-if="selectedPlatform === 'linux'">
<% // Linux policies (server-side-rendered)
- for(let report of windowsReports) {
+ for(let report of linuxReports) {
%>
@@
<div v-else-if="selectedPlatform === 'windows'">
<% // Windows policies (server-side-rendered)
- for(let report of linuxReports) {
+ for(let report of windowsReports) {
%>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div v-else-if="selectedPlatform === 'linux'"> | |
| <% // Linux policies (server-side-rendered) | |
| for(let query of linuxQueries) { | |
| for(let report of windowsReports) { | |
| %> | |
| <div purpose="policy" class="d-flex flex-lg-row flex-column justify-content-between"> | |
| <div purpose="policy-name-and-description" class="d-flex flex-column"> | |
| <div class="d-flex flex-column"> | |
| <p purpose="policy-name"><a href="/queries/<%- query.slug%>"><%- query.name %></a></p> | |
| <p purpose="policy-name"><a href="/reports/<%- report.slug%>"><%- report.name %></a></p> | |
| </div> | |
| <div purpose="policy-description"><p><%- query.description %></p></div> | |
| <div purpose="read-more-link"><animated-arrow-button arrow-color="#192147" href="/queries/<%- query.slug%>">Read more</animated-arrow-button></div> | |
| <div purpose="policy-description"><p><%- report.description %></p></div> | |
| <div purpose="read-more-link"><animated-arrow-button arrow-color="#192147" href="/reports/<%- report.slug%>">Read more</animated-arrow-button></div> | |
| </div> | |
| </div> | |
| <% } %> | |
| </div> | |
| <div v-else-if="selectedPlatform === 'windows'"> | |
| <% // Windows policies (server-side-rendered) | |
| for(let query of windowsQueries) { | |
| for(let report of linuxReports) { | |
| %> | |
| <div v-else-if="selectedPlatform === 'linux'"> | |
| <% // Linux policies (server-side-rendered) | |
| for(let report of linuxReports) { | |
| %> | |
| <div purpose="policy" class="d-flex flex-lg-row flex-column justify-content-between"> | |
| <div purpose="policy-name-and-description" class="d-flex flex-column"> | |
| <div class="d-flex flex-column"> | |
| <p purpose="policy-name"><a href="/reports/<%- report.slug%>"><%- report.name %></a></p> | |
| </div> | |
| <div purpose="policy-description"><p><%- report.description %></p></div> | |
| <div purpose="read-more-link"><animated-arrow-button arrow-color="#192147" href="/reports/<%- report.slug%>">Read more</animated-arrow-button></div> | |
| </div> | |
| </div> | |
| <% } %> | |
| </div> | |
| <div v-else-if="selectedPlatform === 'windows'"> | |
| <% // Windows policies (server-side-rendered) | |
| for(let report of windowsReports) { | |
| %> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/views/pages/docs/report-library.ejs` around lines 49 - 67, The
platform branches are rendering the wrong arrays: in the v-else-if where
selectedPlatform === 'linux' the for loop iterates windowsReports, and where
selectedPlatform === 'windows' it iterates linuxReports; update the for loops so
the Linux branch iterates linuxReports and the Windows branch iterates
windowsReports (update the two for(let report of ...) occurrences that currently
reference the wrong arrays while keeping the report variable and the surrounding
markup unchanged).
fleet-release
approved these changes
Apr 21, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
website/config/routes.js (1)
1082-1087:⚠️ Potential issue | 🟡 MinorUse explicit 301 redirects for all permanent renamed routes.
Lines 1082–1083 use string shortcuts (
'GET /queries': '/reports') which default to 302 temporary redirects in Sails.js. Line 1086 correctly uses an explicit 301. Since these routes represent a permanent rename, all three should consistently use 301 to preserve canonical signals for SEO and user expectations.Convert the string shortcuts to explicit handlers:
Redirect consistency fix
- 'GET /queries': '/reports', - 'GET /query-generator': '/report-generator', + 'GET /queries': (req, res) => { return res.redirect(301, '/reports'); }, + 'GET /query-generator': (req, res) => { return res.redirect(301, '/report-generator'); }, 'GET /queries/:slug': { fn: (req, res) => { return res.redirect(301, '/reports/' + req.param('slug'));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/config/routes.js` around lines 1082 - 1087, The routes using string shortcut redirects ('GET /queries' -> '/reports' and 'GET /query-generator' -> '/report-generator') currently produce 302s; replace those shortcuts with explicit handler objects like the existing 'GET /queries/:slug' entry so they return res.redirect(301, '<target>') to enforce permanent redirects. In other words, change the two string-mapped routes into objects with fn: (req, res) => res.redirect(301, '/reports') and fn: (req, res) => res.redirect(301, '/report-generator') respectively, matching the pattern used by the 'GET /queries/:slug' handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/config/routes.js`:
- Around line 68-69: The concrete route 'GET
/reports/state-of-device-management' (handled by the
reports/view-state-of-device-management action) must be declared before the
dynamic catch-all 'GET /reports/:slug' (handled by docs/view-report-details) so
the specific path routes to its intended controller; move the concrete
'/reports/state-of-device-management' route block above the '/reports/:slug'
entry and delete the duplicate '/reports/state-of-device-management' block so
only the specific route exists before the dynamic one.
---
Duplicate comments:
In `@website/config/routes.js`:
- Around line 1082-1087: The routes using string shortcut redirects ('GET
/queries' -> '/reports' and 'GET /query-generator' -> '/report-generator')
currently produce 302s; replace those shortcuts with explicit handler objects
like the existing 'GET /queries/:slug' entry so they return res.redirect(301,
'<target>') to enforce permanent redirects. In other words, change the two
string-mapped routes into objects with fn: (req, res) => res.redirect(301,
'/reports') and fn: (req, res) => res.redirect(301, '/report-generator')
respectively, matching the pattern used by the 'GET /queries/:slug' handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 699a1017-542e-4b74-b634-c77081c48b29
📒 Files selected for processing (2)
website/config/routes.jswebsite/views/pages/admin/query-generator.ejs
✅ Files skipped from review due to trivial changes (1)
- website/views/pages/admin/query-generator.ejs
Comment on lines
+68
to
+69
| 'GET /reports/:slug': { | ||
| action: 'docs/view-report-details',// Meta title and description set in view action |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm the concrete /reports/state-of-device-management route is declared before /reports/:slug.
python - <<'PY'
from pathlib import Path
text = Path('website/config/routes.js').read_text()
dynamic = text.index("'GET /reports/:slug'")
concrete = text.index("'GET /reports/state-of-device-management'")
print("dynamic /reports/:slug index:", dynamic)
print("concrete /reports/state-of-device-management index:", concrete)
if concrete > dynamic:
raise SystemExit("Concrete /reports/state-of-device-management route is declared after /reports/:slug and may be shadowed.")
PYRepository: fleetdm/fleet
Length of output: 270
Move the concrete /reports/state-of-device-management route above /reports/:slug.
The dynamic catch-all route at line 68 is declared before the concrete /reports/state-of-device-management route (line 344). In order-sensitive Sails/Express-style routing, requests to /reports/state-of-device-management will match the dynamic :slug route first, sending them to docs/view-report-details with slug = "state-of-device-management" instead of to the intended reports/view-state-of-device-management controller.
Proposed fix
Move the concrete route block above the dynamic route, then remove the duplicate at lines 344-350:
+ 'GET /reports/state-of-device-management': {
+ action: 'reports/view-state-of-device-management',
+ locals: {
+ pageTitleForMeta: 'State of device management',
+ pageDescriptionForMeta: 'We surveyed 200+ security practitioners to discover the state of device management in 2022. Click here to learn about their struggles and best practices.',
+ }
+ },
+
'GET /reports/:slug': {
action: 'docs/view-report-details',// Meta title and description set in view action
locals: {
currentSection: 'more',
}
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/config/routes.js` around lines 68 - 69, The concrete route 'GET
/reports/state-of-device-management' (handled by the
reports/view-state-of-device-management action) must be declared before the
dynamic catch-all 'GET /reports/:slug' (handled by docs/view-report-details) so
the specific path routes to its intended controller; move the concrete
'/reports/state-of-device-management' route block above the '/reports/:slug'
entry and delete the duplicate '/reports/state-of-device-management' block so
only the specific route exists before the dynamic one.
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.
Closes: #43578
Changes:
Summary by CodeRabbit
New Features
Refactor
Chores