Feature/html lib updates #113 - #119
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds automated CDN dependency auditing and issue reporting, upgrades frontend libraries to Bootstrap 5-compatible versions, refreshes related templates and styles, and adds ignore rules for Claude artifacts and Python virtual environments. ChangesCDN audit and frontend refresh
Workspace exclusions
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions as GitHub Actions
participant Audit as check_cdn_updates.py
participant NPM as npm registry
participant CDN as jsDelivr
participant GitHubIssues as GitHub Issues
GitHubActions->>Audit: run scheduled CDN check
Audit->>NPM: fetch latest versions and peer dependencies
Audit->>CDN: resolve assets and calculate SRI hashes
Audit-->>GitHubActions: return report and exit code
GitHubActions->>GitHubIssues: create or update CDN dependency issue
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
templates/_static/feedback_pipeline.css (1)
307-313: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate/conflicting filter rules.
#table_packages_filter, .dataTables_filteris styled here and again at Lines 15-20 with differentborder-radius(0.375rem vs 4px) and padding; the input rule is likewise duplicated at Lines 22-28 and 319-337. Whichever wins depends purely on source order — consider consolidating into a single ruleset to avoid drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/_static/feedback_pipeline.css` around lines 307 - 313, Consolidate the duplicate filter styling in the stylesheet by merging the `#table_packages_filter`, .dataTables_filter, and corresponding input rules into single canonical rulesets. Remove conflicting repeated declarations, preserve the intended shared appearance, and ensure the resulting selectors retain the required styling without relying on source-order overrides.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/check-cdn-updates.yml:
- Around line 34-39: Update the workflow step containing the
scripts/check_cdn_updates.py invocation to disable errexit with set +e before
running the script, allowing exit codes 1 and 2 to be captured. Preserve the
existing EXIT_CODE assignment, GITHUB_OUTPUT write, and final exit behavior.
- Around line 12-14: Update the workflow permissions and issue-reporting step in
check-cdn-updates.yml so they are consistent: either enable issues: write for
the GitHub API calls while preserving the existing fork/repository guard, or
remove the issue-listing/comment/creation step if the summary is the sole
intended output.
In `@scripts/check_cdn_updates.py`:
- Around line 388-402: Update the DataTables-specific pattern in
extract_version_from_url to match the datatables.net package followed by either
@ or / before the semantic version, so URLs such as datatables.net@2.3.8 and
datatables.net/2.3.8 are both recognized while leaving the other patterns
unchanged.
In `@templates/layout.html`:
- Around line 30-32: Update the Axios script reference in the layout to use the
UMD browser build at dist/axios.min.js and replace its integrity hash with the
matching value, preserving the global axios API used by the templates.
---
Nitpick comments:
In `@templates/_static/feedback_pipeline.css`:
- Around line 307-313: Consolidate the duplicate filter styling in the
stylesheet by merging the `#table_packages_filter`, .dataTables_filter, and
corresponding input rules into single canonical rulesets. Remove conflicting
repeated declarations, preserve the intended shared appearance, and ensure the
resulting selectors retain the required styling without relying on source-order
overrides.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e12d113c-3489-41d6-b8bf-c0d66adf25bb
📒 Files selected for processing (11)
.dockerignore.github/workflows/check-cdn-updates.yml.gitignorescripts/check_cdn_updates.pytemplates/_static/feedback_pipeline.csstemplates/homepage.htmltemplates/layout.htmltemplates/repo-split-overview.htmltemplates/repo-split.htmltemplates/view.htmltemplates/view_menu.html
yselkowitz
left a comment
yselkowitz
left a comment
There was a problem hiding this comment.
- Is there a running instance with these changes included (to see the difference)?
- Please address the CodeRabbit comments.
- Is there no way to use Dependabot (or the like) for updating the JS?
| workflow_dispatch: # Allow manual triggering | ||
| push: | ||
| branches: | ||
| - 'feature/*' # Requires a feature branch |
There was a problem hiding this comment.
Mainly a habit I have of using feature branches, but your right it should cover all branches
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/check-cdn-updates.yml:
- Around line 45-46: Add an id to the “Create or update issue” workflow step and
expose a success flag from that step. Update the success-summary condition to
check that step’s outcome is successful rather than relying only on
steps.check.outputs.exit_code, while preserving the existing behavior for
permission or GitHub API failures.
- Line 8: Update the condition guarding the “Create or update issue” step so it
requires both the canonical repository (`github.repository ==
'fedora-eln/content-resolver'`) and the repository’s default branch, while
preserving auditing for pushes on all branches.
In `@scripts/check_cdn_updates.py`:
- Line 395: Remove the unnecessary f-string prefix from the DataTables regex
pattern in the CDN update checks, while retaining the raw-string prefix and
identical matching behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 394b4d0b-9372-43da-bb04-14b2d6660bd7
📒 Files selected for processing (3)
.github/workflows/check-cdn-updates.ymlscripts/check_cdn_updates.pytemplates/layout.html
🚧 Files skipped from review as they are similar to previous changes (1)
- templates/layout.html
Not at the moment, this will run on with dnf-4 and previously when I ran two dnf-4 instances side by side it caused issues and we were unable to access the server for a period, my presumption was it was memory usage was high.
Updated
Dependabot only works with package.json dependencies, not CDN-hosted ones. We could migrate to npm + a bundler (webpack/vite), which would enable Dependabot. However, this adds build complexity for what's currently a simple static site. Renovate Bot has a better CDN support, but wont check compatibility of libraries and versions |
1be05da to
cf068ec
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/check-cdn-updates.yml:
- Around line 44-47: Restrict the “Create or update issue” step to runs on the
canonical repository’s default branch, using an explicit repository and branch
condition in its if expression. Scope any issues: write permission to this gated
job or remove the issue-creation step; do not allow untrusted forks or feature
branches to execute it.
- Around line 109-112: Update the summary-generation logic in the step using
steps.create-issue.outcome so a missing cdn-update-report.md is treated as
issue-creation failure: detect the absent report, emit an error, and fail the
script before reporting success. Preserve the existing success summary only when
the report exists and issue creation succeeded.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 42a1b67d-7ddf-4fa2-868d-b3a2247e6ddb
📒 Files selected for processing (2)
.github/workflows/check-cdn-updates.ymlscripts/check_cdn_updates.py
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/check_cdn_updates.py
| if [ "${{ steps.create-issue.outcome }}" = "success" ]; then | ||
| echo "✅ **Issue created/updated** - safe updates are available" >> $GITHUB_STEP_SUMMARY | ||
| else | ||
| echo "⚠️ **Updates available but issue creation failed** (check permissions)" >> $GITHUB_STEP_SUMMARY |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching workflow:"
fd -a 'check-cdn-updates.yml' .github/workflows 2>/dev/null || true
echo
echo "Relevant workflow excerpt:"
if [ -f .github/workflows/check-cdn-updates.yml ]; then
sed -n '1,170p' .github/workflows/check-cdn-updates.yml | cat -n
fi
echo
echo "Search for create-issue step usages/options:"
rg -n "create-issue|github-script|no report file found|cdn-update-report" .github . 2>/dev/null || trueRepository: fedora-eln/content-resolver
Length of output: 6307
Treat a missing report as issue-creation failure.
Under the scheduled/fork fallback path, this step is only referenced during summary generation. If cdn-update-report.md is missing in a scenario where safe updates were supposed to be created, returning normally makes the summary say “Issue created/ updated” even though no issue was created. Fail the script with an error for the missing report.
Proposed fix
- console.log('No report file found');
- return;
+ throw new Error('No report file found');🧰 Tools
🪛 zizmor (1.26.1)
[info] 109-109: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/check-cdn-updates.yml around lines 109 - 112, Update the
summary-generation logic in the step using steps.create-issue.outcome so a
missing cdn-update-report.md is treated as issue-creation failure: detect the
absent report, emit an error, and fail the script before reporting success.
Preserve the existing success summary only when the report exists and issue
creation succeeded.
cf068ec to
b29d87a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/check-cdn-updates.yml (1)
45-47:⚠️ Potential issue | 🟠 MajorThe issue step still lacks write permission.
The workflow grants only
contents: read, so unspecified permissions—includingissues—arenone; the REST issue creation/comment calls will fail on the canonical run, whilecontinue-on-errorhides the failure. Keep this audit job read-only and move issue creation to a separate trusted job withissues: write, rather than granting write access to this all-branch job. (docs.github.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/check-cdn-updates.yml around lines 45 - 47, Split the issue creation logic from the read-only audit job into a separate trusted job that runs only for the canonical master repository context and grants issues: write permission; keep the existing audit job’s contents: read-only permissions and pass the check result or required data between jobs, preserving the current create-issue condition without relying on continue-on-error to hide permission failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/check-cdn-updates.yml:
- Around line 109-113: Update the conditional around the create-issue outcome in
the workflow summary: report success for a successful create-issue step, show
the permissions warning only when steps.create-issue.outcome is failure, and let
skipped outcomes fall through to the existing report summary path.
---
Duplicate comments:
In @.github/workflows/check-cdn-updates.yml:
- Around line 45-47: Split the issue creation logic from the read-only audit job
into a separate trusted job that runs only for the canonical master repository
context and grants issues: write permission; keep the existing audit job’s
contents: read-only permissions and pass the check result or required data
between jobs, preserving the current create-issue condition without relying on
continue-on-error to hide permission failures.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0840461a-287a-43e5-9540-ae4acffcafea
📒 Files selected for processing (2)
.github/workflows/check-cdn-updates.ymlscripts/check_cdn_updates.py
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/check_cdn_updates.py
b29d87a to
bba9369
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/check-cdn-updates.yml:
- Around line 38-43: Update the exit-status handling around
scripts/check_cdn_updates.py so only completed audits use codes 0, 1, or 2.
Ensure unhandled exceptions return a distinct non-audit error code, and make the
workflow’s exit_code output and downstream classification explicitly report/fail
that status rather than treating it as has_updates or up to date.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 37ff1f40-b9dc-4995-b18d-65f0021de2f0
📒 Files selected for processing (1)
.github/workflows/check-cdn-updates.yml
bba9369 to
f8ae729
Compare
yselkowitz
left a comment
yselkowitz
left a comment
There was a problem hiding this comment.
Some minor nitpicks, but I won't pretend to understand all of this.
format file move imports to top of file
f8ae729 to
ea5fc02
Compare
00732ad to
de3cebe
Compare
de3cebe to
79e7a45
Compare
yselkowitz
left a comment
yselkowitz
left a comment
There was a problem hiding this comment.
I'm seeing a weird effect in the test instance when loading the Packages (or more briefly, Source Packages) tab of a view, the chart Loading... is accompanied not only by a spinning wheel but another word Loading... spinning with it. IIUC #123 is why there is this loading delay/failure in the first place, but it seems odd regardless, and doesn't happen in the prod instance.
yselkowitz
left a comment
yselkowitz
left a comment
There was a problem hiding this comment.
Another thing: the gray background for the Buildroot level 2 (or higher) label should be a lighter gray than that of Buildroot level 1. You can (sort of) see this in the view chart, although the BL2+ category is very thin nowadays.
79e7a45 to
ce53297
Compare
| .buildroot-level-3 { | ||
| background-color: #d5d5d5 !important; | ||
| } |
There was a problem hiding this comment.
While even BL3 is high (we were at only 2 for a while), there is always the possibility that the number of levels could jump due to an unwanted dep sneaking in and pulling in a bunch more deps. Historically though we've only separated between buildroot base, L1, and L2+.
There was a problem hiding this comment.
have updated now to keep the same colour for 2+
Update highlighting for highlighting dependency level indicators
5568664 to
c962d08
Compare
2 participants
Frontend Library Updates - Bootstrap 5 Migration & CDN Monitoring
Closes #113
Summary
Upgrade the Content Resolver frontend by migrating from Bootstrap 4 to Bootstrap 5 and implementing automated CDN dependency update monitoring to keep frontend libraries current and secure.
Key Changes
1. Bootstrap 5 Migration
Updated Frontend Dependencies:
Template Updates:
2. Automated CDN Update Monitoring
GitHub Action Workflow:
Smart Update Detection:
templates/layout.htmlto extract all CDN resourcesIssue Management:
dependencies,cdn,enhancementlabelsExit Codes:
Breaking Changes
None - Bootstrap 5 changes are backward compatible with the existing template structure. Custom CSS has been updated to maintain visual consistency.
Security Improvements
Summary by CodeRabbit