.pr_agent_accepted_suggestions - TACC/Core-CMS GitHub Wiki

                     PR 1155 (2026-05-18)                    
[correctness] Byline setting not exported
Byline setting not exported `blog_meta.html` now depends on `settings.PORTAL_BLOG_SHOW_BYLINE`, but that setting is not included in `SETTINGS_EXPORT`, so it will resolve as missing/falsy in templates and the byline will never render. This defeats the intended default (`True`) and removes author bylines site-wide.

Issue description

Templates access settings.PORTAL_BLOG_SHOW_BYLINE, but django_settings_export only exposes variables listed in SETTINGS_EXPORT. Because PORTAL_BLOG_SHOW_BYLINE is missing from that list, the template condition will evaluate falsy and never display the byline.

Issue Context

  • django_settings_export.settings_export is enabled as a template context processor.
  • SETTINGS_EXPORT_VARIABLE_NAME is set to settings and SETTINGS_EXPORT enumerates which keys are accessible in templates.

Fix Focus Areas

  • taccsite_cms/settings/settings.py[720-743]
  • Add 'PORTAL_BLOG_SHOW_BYLINE' to SETTINGS_EXPORT (near the other PORTAL_BLOG_SHOW_* keys).

[correctness] Share URL not encoded
Share URL not encoded Social share links interpolate the page URL directly into query parameters without URL-encoding, so pages with `?`, `&`, or `#` in the URL can produce broken share/mailto links. Since social sharing is now enabled by default, this will affect most deployments immediately.

Issue description

share_on_social.html injects {{ url }} directly into query parameters (Facebook/LinkedIn/Bluesky) and mailto: body without URL encoding. Any URL containing special characters (e.g. /news/?tag=a&b=c#section) will break the generated share link.

Issue Context

Sharing is now enabled by default via PORTAL_SOCIAL_SHARE_PLATFORMS, increasing the likelihood this bug is hit in production.

Fix Focus Areas

  • taccsite_cms/templates/share_on_social.html[3-56]
  • Build the URL via request.build_absolute_uri (or include request.get_full_path) and apply |urlencode when embedding into query parameters (e.g. ...?u={{ url|urlencode }} and mailto:?body={{ url|urlencode }}).
  • taccsite_cms/settings/settings.py[290-295]
  • (Optional) Keep defaults, but verify share links work for URLs with query strings/fragments after encoding changes.

[maintainability] Instagram default unsupported
Instagram default unsupported The default `PORTAL_SOCIAL_SHARE_PLATFORMS` now includes `'instagram'`, but `share_on_social.html` has no rendering branch for it, so it is silently ignored. If a site configures only unsupported platforms, the template will still render the “share this:” prefix with zero icons/links.

Issue description

Default configuration includes 'instagram' but the share template does not implement an Instagram share link, so the platform is ignored. This creates a mismatch between settings and rendered output and can produce a UI stub (prefix text, no icons) for certain configurations.

Issue Context

share_on_social.html currently supports: facebook, linkedin, bluesky, email.

Fix Focus Areas

  • taccsite_cms/settings/settings.py[290-295]
  • Remove 'instagram' from the default list or replace it with a supported platform.
  • taccsite_cms/templates/share_on_social.html[6-57]
  • (Optional hardening) Adjust logic so the prefix only renders when at least one supported platform link will render, and/or handle unknown platforms explicitly.


                     PR 1152 (2026-05-12)                    
[correctness] `PORTAL_BLOG_SHOW_MAIN_IMAGE` undefined
`PORTAL_BLOG_SHOW_MAIN_IMAGE` undefined `taccsite_cms/settings/settings.py` contains a bare reference to `PORTAL_BLOG_SHOW_MAIN_IMAGE` at module scope with no assignment, which will raise a `NameError` when Django imports settings and prevent the app (and Django commands) from starting. This breaks the documented verification commands (e.g., tests/collectstatic) that require importing settings.

Issue description

taccsite_cms/settings/settings.py contains a bare identifier PORTAL_BLOG_SHOW_MAIN_IMAGE with no assignment, which triggers a NameError during settings import and blocks Django startup and management commands.

Issue Context

Django imports settings at startup and for commands like python manage.py test ... and python manage.py collectstatic --no-input, so this import-time NameError breaks the documented verification workflow (PR Compliance ID 5). The stray line may have been accidentally left behind during a rename involving PORTAL_BLOG_SHOW_ABSTRACT_TAG.

Fix Focus Areas

  • taccsite_cms/settings/settings.py[315-327]

[reliability] Old setting override ignored
Old setting override ignored The deprecated remapping logic still maps `TACC_BLOG_SHOW_ABSTRACT_TAG` to `PORTAL_BLOG_SHOW_ABSTRACT_TAG`, but templates now read `PORTAL_BLOG_TAG_FOR_EXTERNAL_ARTICLES`, so any existing configuration using the old key will be silently ignored.

Issue description

After renaming the setting, the templates and exported settings now use PORTAL_BLOG_TAG_FOR_EXTERNAL_ARTICLES. However, the deprecated-settings remapping code still converts legacy TACC_BLOG_SHOW_ABSTRACT_TAG into PORTAL_BLOG_SHOW_ABSTRACT_TAG, meaning legacy overrides won’t affect the new behavior.

Issue Context

  • Templates read settings.PORTAL_BLOG_TAG_FOR_EXTERNAL_ARTICLES.
  • SETTINGS_EXPORT exports PORTAL_BLOG_TAG_FOR_EXTERNAL_ARTICLES.
  • Deprecated remapping currently only performs a generic TACC_PORTAL_ rename, which produces the old portal key (PORTAL_BLOG_SHOW_ABSTRACT_TAG).

Fix Focus Areas

  • taccsite_cms/settings/settings.py[680-703]
  • taccsite_cms/settings/settings.py[723-744]

Suggested fix

In the deprecated remapping loop, special-case TACC_BLOG_SHOW_ABSTRACT_TAG to set PORTAL_BLOG_TAG_FOR_EXTERNAL_ARTICLES (and optionally also set PORTAL_BLOG_SHOW_ABSTRACT_TAG as an alias for extra compatibility). Alternatively, remove TACC_BLOG_SHOW_ABSTRACT_TAG from the deprecated remap list if you explicitly want to drop backward support, but then ensure documentation reflects that behavior clearly.



                     PR 1147 (2026-04-13)                    
[reliability] Git SSH dependency install
Git SSH dependency install @tacc/core-styles is now sourced from GitHub and resolves to a `git+ssh://[email protected]/...` URL, so `npm ci` will fail unless the environment has GitHub SSH credentials configured. This risks breaking the documented CSS build step that runs `npm ci` in a stock Node container.

Issue description

@tacc/core-styles is pinned to GitHub, but npm resolves it to a git+ssh://[email protected]/... URL, requiring SSH credentials. This can break npm ci in CI/dev environments.

Issue Context

The project’s setup/build scripts run npm ci inside a vanilla Node container; no SSH credentials/config are established by the repo.

Fix Focus Areas

  • package.json[23-25]
  • package-lock.json[1405-1408]

Suggested fix

  • Change the dependency spec to an HTTPS git URL (or set git-protocol=https via .npmrc) so the lock resolves to git+https://....
  • Regenerate package-lock.json to ensure resolved uses HTTPS, then re-run npm ci in the documented container flow.

[correctness] Aria-label hides link text
Aria-label hides link text footer.html assigns the same `aria-label="Opens in new window."` to multiple links, overriding their accessible names so assistive tech users encounter three identical, non-descriptive links. This makes the footer links effectively unusable for screen-reader navigation.

Issue description

The footer links set identical aria-label values that override link text, causing screen readers to announce indistinguishable links.

Issue Context

This footer is included site-wide via base.html.

Fix Focus Areas

  • taccsite_cms/templates/footer.html[3-6]

Suggested fix

  • Remove the aria-label attributes entirely (letting the visible link text become the accessible name), OR
  • Make each aria-label unique and descriptive, e.g. "Texas Advanced Computing Center (opens in a new window)", OR
  • Prefer adding an accessible hint via visually-hidden text/aria-describedby instead of replacing the accessible name.


                     PR 1146 (2026-04-09)                    
[reliability] Git SSH dep breaks builds
Git SSH dep breaks builds The new `github:TACC/Core-Styles#...` dependency is resolved in package-lock.json as a `git+ssh://[email protected]/...` URL, so `npm ci` will try to clone via SSH and fail in Docker/CI environments without GitHub SSH keys. This can block Docker image builds because the Dockerfile runs `npm ci` during the build stage.

Issue description

@tacc/core-styles is specified via github: which is currently being locked as git+ssh://[email protected]/... in package-lock.json. This makes npm ci fail in Docker/CI environments that do not have GitHub SSH credentials.

Issue Context

The Docker image build runs npm ci, which strictly uses package-lock.json.

Fix Focus Areas

  • package.json[23-25]
  • package-lock.json[1405-1410]
  • Dockerfile[39-46]

Suggested fix

  1. Change the dependency spec to an HTTPS-based git URL (and preferably a full commit SHA), e.g. git+https://github.com/TACC/Core-Styles.git#<full_sha>.
  2. Re-run npm install to regenerate package-lock.json so the resolved field is no longer git+ssh://....
  3. Verify docker build succeeds without injecting SSH keys.

[maintainability] Short git SHA pin
Short git SHA pin package.json pins `@tacc/core-styles` using a short SHA (`9bc64ca`), which is less explicit and can become ambiguous over time. Using the full 40-character commit SHA improves clarity and reduces future maintenance risk when regenerating the lockfile.

Issue description

The dependency is pinned with a short commit SHA in package.json, which is less explicit than the full commit hash.

Issue Context

package-lock.json already contains the full 40-character commit hash for this dependency.

Fix Focus Areas

  • package.json[23-25]

Suggested fix

Replace github:TACC/Core-Styles#9bc64ca with github:TACC/Core-Styles#9bc64cab17108a89775eae0cd2d6f5d8ea4ef870 (or switch to an HTTPS git URL while doing this), then ensure package-lock.json remains consistent.



                     PR 1119 (2026-03-13)                    
[correctness] Invalid dropdown ARIA
Invalid dropdown ARIA `cms_menu.html` renders dropdown links with `role="menuitem"` inside a dropdown container that no longer has `role="menu"`, producing invalid ARIA semantics. This can cause assistive tech and automated accessibility checks (WCAG 4.1.2) to report the menu structure incorrectly.

Issue description

The dropdown menu container was changed to &amp;lt;nav class=&amp;quot;dropdown-menu&amp;quot;&amp;gt; and no longer has role=&amp;quot;menu&amp;quot;, but the child links still use role=&amp;quot;menuitem&amp;quot;. This produces invalid ARIA semantics (menuitems without a menu) and may be flagged by accessibility tooling.

Issue Context

  • The header already uses a &amp;lt;nav id=&amp;quot;s-header&amp;quot;&amp;gt;; introducing nested &amp;lt;nav&amp;gt; elements for each dropdown can also add landmark noise.
  • The toggle change from &amp;lt;a role=&amp;quot;button&amp;quot;&amp;gt; to &amp;lt;button&amp;gt; is good and can remain.

Fix Focus Areas

  • taccsite_cms/templates/cms_menu.html[33-51]

Suggested approach

Option A (minimal change):

  • Change &amp;lt;nav class=&amp;quot;dropdown-menu&amp;quot; ...&amp;gt; back to &amp;lt;div class=&amp;quot;dropdown-menu&amp;quot; role=&amp;quot;menu&amp;quot; ...&amp;gt; (matching prior semantics). Option B (Bootstrap-style semantics):
  • Keep the container element/tag, but remove role=&amp;quot;menuitem&amp;quot; from the &amp;lt;a&amp;gt; elements (and keep aria-labelledby), so links are announced as normal links.


                     PR 1109 (2026-03-05)                    
[reliability] ES version mismatch
ES version mismatch The dev compose was bumped to Elasticsearch 7.17.9 for the cgroups v2 crash, but the example compose path (`make example`) still pins 7.17.0, so users following the example workflow can still hit the same crash.

Issue description

docker-compose.dev.yml was upgraded to ES 7.17.9 to avoid the documented cgroups v2 crash, but docker-compose.example-cms.yml (used by make example) still pins 7.17.0.

Issue Context

make example uses docker-compose.example.yml, which extends docker-compose.example-cms.yml for the Elasticsearch service.

Fix Focus Areas

  • docker-compose.example-cms.yml[32-34]
  • docker-compose.example.yml[21-24]
  • Makefile[27-30]

[reliability] ES version mismatch
ES version mismatch The dev compose was bumped to Elasticsearch 7.17.9 for the cgroups v2 crash, but the example compose path (`make example`) still pins 7.17.0, so users following the example workflow can still hit the same crash.

Issue description

docker-compose.dev.yml was upgraded to ES 7.17.9 to avoid the documented cgroups v2 crash, but docker-compose.example-cms.yml (used by make example) still pins 7.17.0.

Issue Context

make example uses docker-compose.example.yml, which extends docker-compose.example-cms.yml for the Elasticsearch service.

Fix Focus Areas

  • docker-compose.example-cms.yml[32-34]
  • docker-compose.example.yml[21-24]
  • Makefile[27-30]

[reliability] Postgres secrets note wrong
Postgres secrets note wrong AGENTS.md claims the Postgres `./conf/postgres/*.secret` files can be ignored, but `docker-compose.dev.yml` bind-mounts them from gitignored paths and the setup flow doesn’t create them, so fresh setups are prone to failing or requiring manual local files.

Issue description

AGENTS.md says Postgres ./conf/postgres/*.secret files “can be ignored”, but docker-compose.dev.yml bind-mounts them from gitignored paths and bin/setup-cms.sh doesn’t create them. This contradiction makes first-time setup brittle.

Issue Context

  • The secret files are gitignored, so new clones won’t have them.
  • The dev compose currently bind-mounts them anyway.

Fix Focus Areas

  • docker-compose.dev.yml[22-26]
  • docker-compose.example-cms.yml[22-26]
  • bin/setup-cms.sh[118-123]
  • AGENTS.md[40-41]
  • .gitignore[37-38]

[reliability] Postgres secrets note wrong
Postgres secrets note wrong AGENTS.md claims the Postgres `./conf/postgres/*.secret` files can be ignored, but `docker-compose.dev.yml` bind-mounts them from gitignored paths and the setup flow doesn’t create them, so fresh setups are prone to failing or requiring manual local files.

Issue description

AGENTS.md says Postgres ./conf/postgres/*.secret files “can be ignored”, but docker-compose.dev.yml bind-mounts them from gitignored paths and bin/setup-cms.sh doesn’t create them. This contradiction makes first-time setup brittle.

Issue Context

  • The secret files are gitignored, so new clones won’t have them.
  • The dev compose currently bind-mounts them anyway.

Fix Focus Areas

  • docker-compose.dev.yml[22-26]
  • docker-compose.example-cms.yml[22-26]
  • bin/setup-cms.sh[118-123]
  • AGENTS.md[40-41]
  • .gitignore[37-38]


⚠️ **GitHub.com Fallback** ⚠️