Skip to content

Extract hard-coded routes into centralized constants (resolves #747)#1127

Open
BenjaminMichaelis wants to merge 4 commits into
mainfrom
benjaminmichaelis/extract-content-page-routes
Open

Extract hard-coded routes into centralized constants (resolves #747)#1127
BenjaminMichaelis wants to merge 4 commits into
mainfrom
benjaminmichaelis/extract-content-page-routes

Conversation

@BenjaminMichaelis
Copy link
Copy Markdown
Member

Motivation

Hard-coded route strings were scattered across multiple frontend and backend files (useSiteShell.js, SidebarPanel.vue, SitemapXmlHelpers.cs, HomeController.cs), making route changes brittle and error-prone. Issue #747 specifically called out the isContentPage logic and suggested extracting routes into a constant and using Array.includes() for clarity.

Approach

Two new constant files serve as the single source of truth per layer:

Backend -- EssentialCSharp.Web/Constants/RouteConstants.cs

  • StaticPages inner class with const string values for each non-content route
  • NonContentRoutes HashSet for O(1) membership checks
  • SeoMetadata.RouteConfig dictionary mapping routes to (ChangeFrequency, priority) tuples, replacing the two switch expressions in SitemapXmlHelpers.cs

Frontend -- EssentialCSharp.Web/src/constants/routes.js

  • ROUTES object with named route constants
  • NON_CONTENT_ROUTES array for Array.includes()/.some() checks
  • NAVIGATION_LINKS array (with icon class, label, activePaths) replacing the inline definition in SidebarPanel.vue
  • isContentPagePath() helper function for reuse

Consumers updated:

  • useSiteShell.js -- isContentPage now uses NON_CONTENT_ROUTES.some() as the primary check, with percentComplete !== null as a secondary guard
  • SidebarPanel.vue -- imports NAVIGATION_LINKS instead of defining the array inline
  • SitemapXmlHelpers.cs -- GetChangeFrequencyForRoute / GetPriorityForRoute use dictionary lookup instead of switch expressions
  • HomeController.cs -- [Route()] attributes reference RouteConstants.StaticPages constants

Notes

  • The percentComplete !== null fallback is intentionally preserved in isContentPage so that pages not in the static list still correctly opt in/out of content-page features based on server-side data.
  • Adding a new static page now requires updating only the two constant files; all consumers pick up the change automatically.

Closes #747

- Add Constants/RouteConstants.cs with StaticPages string constants,
  NonContentRoutes HashSet, and SeoMetadata dictionary for sitemap config
- Add src/constants/routes.js mirroring the C# constants for the frontend,
  exporting ROUTES, NON_CONTENT_ROUTES, NAVIGATION_LINKS, and isContentPagePath
- Refactor useSiteShell.js isContentPage to use NON_CONTENT_ROUTES.some()
  with Array path matching as suggested in issue #747, with percentComplete
  as a secondary check
- Refactor SidebarPanel.vue to import NAVIGATION_LINKS from routes.js,
  removing the inline hard-coded navLinks definition
- Refactor SitemapXmlHelpers.cs to use RouteConstants.SeoMetadata.RouteConfig
  dictionary lookups instead of switch expressions
- Update HomeController.cs Route attributes to reference RouteConstants.StaticPages
  string constants instead of hard-coded string literals
Copilot AI review requested due to automatic review settings May 17, 2026 15:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR centralizes previously hard-coded route strings into shared constants on both the frontend and backend to reduce brittleness and simplify future route updates (including the isContentPage logic called out in #747).

Changes:

  • Added centralized route constants/config for backend (RouteConstants.cs) and frontend (routes.js).
  • Updated frontend consumers (useSiteShell.js, SidebarPanel.vue) to use shared route/navigation constants.
  • Updated sitemap and controller routing code (SitemapXmlHelpers.cs, HomeController.cs) to reference centralized constants.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
EssentialCSharp.Web/src/constants/routes.js Adds frontend route constants, non-content route list, sidebar navigation definitions, and a reusable isContentPagePath() helper.
EssentialCSharp.Web/src/composables/useSiteShell.js Switches isContentPage detection to use centralized NON_CONTENT_ROUTES logic.
EssentialCSharp.Web/src/components/SidebarPanel.vue Replaces inline nav link definitions with imported NAVIGATION_LINKS.
EssentialCSharp.Web/Helpers/SitemapXmlHelpers.cs Replaces switch expressions with lookup into centralized route SEO metadata config.
EssentialCSharp.Web/Controllers/HomeController.cs Replaces hard-coded [Route] templates with RouteConstants.StaticPages.*.
EssentialCSharp.Web/Constants/RouteConstants.cs Introduces backend centralized static routes, non-content route set, and sitemap SEO route metadata mapping.
Comments suppressed due to low confidence (3)

EssentialCSharp.Web/Helpers/SitemapXmlHelpers.cs:97

  • Same key-normalization issue as GetChangeFrequencyForRoute: normalizedRoute strips the leading '/', but RouteConstants.SeoMetadata.RouteConfig keys include it (e.g. /guidelines). This makes the lookup always miss and forces the default priority (0.5) for every controller route.
    private static decimal GetPriorityForRoute(string route)
    {
        var normalizedRoute = route.TrimStart('/').ToLowerInvariant();
        
        if (RouteConstants.SeoMetadata.RouteConfig.TryGetValue(normalizedRoute, out var config))
        {
            return config.Item2;
        }
        
        return 0.5M;

EssentialCSharp.Web/Helpers/SitemapXmlHelpers.cs:85

  • There are existing tests for GenerateSitemapXml, but none appear to validate the per-route ChangeFrequency/Priority values for controller routes (e.g., /guidelines priority 0.9, /termsofservice yearly). Adding assertions for these values would prevent regressions in the new RouteConfig lookup/normalization logic.
    private static ChangeFrequency GetChangeFrequencyForRoute(string route)
    {
        var normalizedRoute = route.TrimStart('/').ToLowerInvariant();
        
        if (RouteConstants.SeoMetadata.RouteConfig.TryGetValue(normalizedRoute, out var config))
        {
            return (ChangeFrequency)(int)config.Item1;
        }
        
        return ChangeFrequency.Monthly;

EssentialCSharp.Web/Constants/RouteConstants.cs:75

  • IsContentPage currently does a direct NonContentRoutes.Contains(path) check, so paths like /home/ (trailing slash) or mixed-case inputs will be misclassified as content pages even though the frontend logic treats them as static. Consider normalizing path (trim whitespace, ensure leading '/', remove trailing '/', and handle null/empty) before checking the set.
    /// <summary>
    /// Determines if the given path represents a content page (from sitemap)
    /// versus a static page (non-content).
    /// </summary>
    public static bool IsContentPage(string path)
    {
        return !NonContentRoutes.Contains(path);
    }

Comment thread EssentialCSharp.Web/Helpers/SitemapXmlHelpers.cs
Comment thread EssentialCSharp.Web/Constants/RouteConstants.cs Outdated
Comment thread EssentialCSharp.Web/src/constants/routes.js
Comment thread EssentialCSharp.Web/src/composables/useSiteShell.js
- Use DotnetSitemapGenerator.ChangeFrequency directly in RouteConfig,
  eliminating the fragile local enum and the (ChangeFrequency)(int) cast
- Use FrozenSet<string> and FrozenDictionary for NonContentRoutes and
  RouteConfig to make collections truly immutable
- Remove TrimStart('/') from GetChangeFrequencyForRoute and
  GetPriorityForRoute so dictionary keys ('/home', etc.) actually match
- Use isContentPagePath() from routes.js in useSiteShell.js instead of
  duplicating the same inline logic
- Document why TERMS_OF_SERVICE is excluded from NAVIGATION_LINKS
- Add per-route SEO metadata tests to SitemapXmlHelpersTests
Pass StringComparer.OrdinalIgnoreCase explicitly to ToFrozenSet() to
guarantee case-insensitive lookup, rather than relying on comparer
propagation from the source HashSet.
Copilot AI review requested due to automatic review settings May 18, 2026 05:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread EssentialCSharp.Web/src/composables/useSiteShell.js
Comment thread EssentialCSharp.Web/Controllers/HomeController.cs
- SidebarPanel.vue: switch v-for :key from navLink.href to navLink.key
  so the key is stable and uses the dedicated identifier from NAVIGATION_LINKS
- Home.cshtml: normalize href=/Announcements to /announcements to match
  the canonical casing in RouteConstants.StaticPages.Announcements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants