Skip to content

Hotfix rename woff files so redirect works better#1128

Merged
lodewiges merged 4 commits into
stagingfrom
hotfix/serve-woff-files
Nov 23, 2025
Merged

Hotfix rename woff files so redirect works better#1128
lodewiges merged 4 commits into
stagingfrom
hotfix/serve-woff-files

Conversation

@lodewiges
Copy link
Copy Markdown
Contributor

@lodewiges lodewiges commented Nov 22, 2025

Checklist

  • Merged database migrations into 1 database migration.
  • Tested database migrations from origin/staging (git checkout staging ; git pull ; bundle exec rails db:reset ; git checkout BRANCH ; bundle exec rails db:migrate).

Summary

Shortly summarize the changes in this pull request. Does it concern changes in the UI, add some screenshots. Are there related issues solved? Please, mention them (with 'fixes #xyz', see https://github.com/blog/1506-closing-issues-via-pull-requests), so they can be resolved automatically when merging this pull request.

Other information

If there is some other relevant and important information for this pull request, mention it here. For example, related pull requests or newly introduced conventions, packages or other dependencies.

Summary by CodeRabbit

  • Chores
    • Add a production precompile step that creates short-name copies of font and asset files when missing, preserving metadata.
    • Stop registering specific font MIME types so they are no longer explicitly declared.
    • Emit assets using their original base names + extension to align asset filenames with existing fingerprinting.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 22, 2025

Caution

Review failed

The head commit changed during the review from 747c676 to 214c579.

Walkthrough

Removes three custom font mime registrations, adds a Dockerfile post-assets-precompile step that creates short-name aliases for fingerprinted assets in production-like builds, and sets webpack's output.assetModuleFilename to "[name][ext]" so emitted assets keep original base names with extensions.

Changes

Cohort / File(s) Summary
Dockerfile asset preprocessing
Dockerfile
Added a post-assets-precompile step (RAILS_ENV-aware) that finds fingerprinted files under public/assets/*-*.* and copies them to short-name equivalents (e.g., font-abc123.wofffont.woff), preserving metadata and skipping if the target exists.
Font mime type removal
config/initializers/mime_types.rb
Removed three Mime::Type.register lines for font/woff, font/woff2, and font/ttf.
Webpack asset naming
webpack.config.js
Added output.assetModuleFilename: "[name][ext]" so webpack-emitted asset modules use the original base name plus extension (no fingerprinting in emitted assetModule filenames).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer / CI
  participant Docker as Docker build
  participant Rails as Rails asset precompile
  participant Post as Post-assets step
  participant Webpack as Webpack (asset emit)

  Dev->>Docker: start image build
  Docker->>Rails: run assets:precompile
  Rails->>Webpack: invoke bundling/emit
  Webpack->>Webpack: emit assets using assetModuleFilename "[name][ext]"
  Rails-->>Docker: precompile output (fingerprinted files)
  Docker->>Post: run post-assets-precompile step (RAILS_ENV check)
  Post->>Post: find public/assets/*-*.* → copy to short-name (preserve metadata, skip existing)
  Post-->>Docker: finish
  Docker-->>Dev: image built
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review the Dockerfile bash script for edge cases (glob handling, filename collisions, permission/ownership, RAILS_ENV guard).
  • Verify removal of the mime registrations doesn't regress serving fonts in environments that relied on those explicit registrations.
  • Confirm webpack assetModuleFilename change aligns with expected asset pipeline behavior and does not conflict with other asset fingerprinting or CDN/public path expectations.

Poem

🐰
I hopped through builds at break of day,
Short names sprouted where hashes lay.
Webpack hums and Docker sings,
Fonts find homes on tiny wings.
A joyous nibble — assets saved the day!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description contains only the template placeholders with no actual summary, implementation details, related issues, or other concrete information filled in. Fill in the Summary section with a concise description of the changes made (Dockerfile, webpack.config.js, and MIME type removals) and explain why these changes fix the redirect issue. Reference any related issues using 'fixes #xyz' format if applicable.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Hotfix rename woff files so redirect works better' is specific and directly related to the main changes in the PR—renaming font files and removing font MIME type registrations to fix asset handling.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f38db6 and 2e97565.

📒 Files selected for processing (2)
  • Dockerfile (1 hunks)
  • config/initializers/mime_types.rb (0 hunks)
💤 Files with no reviewable changes (1)
  • config/initializers/mime_types.rb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (1)
Dockerfile (1)

49-55: Asset filename pattern is correct for Rails 7.2.3.

Verification confirms that Rails 7.2.3 uses the standard fingerprinting format <name>-<fingerprint>.<ext> (with SHA-256 64-character digests). The script's glob pattern *-*.* correctly targets these versioned files, and the parameter expansion ${base%%-*} correctly extracts the short name by removing everything from the first hyphen onward. This works correctly even when asset names contain multiple hyphens (e.g., some-font-abc123...woff2some.woff2).

Comment thread Dockerfile Outdated
Comment on lines +48 to +55
# create short-name copies like "abcdef-....woff2" -> "abcdef.woff2" for fonts and other assets
bash -lc 'for f in public/assets/*-*.*; do \
[ -f \"$f\" ] || continue; \
base=$(basename \"$f\"); \
ext=\"${base##*.}\"; \
short=\"${base%%-*}.${ext}\"; \
if [ ! -e \"public/assets/$short\" ]; then cp --preserve=mode,timestamps \"$f\" \"public/assets/$short\"; fi; \
done'; \
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add explicit error handling to the bash script.

The post-assets-precompile script lacks error handling. If cp fails (e.g., due to permission issues), the Docker build continues silently with incomplete asset setup, which could cause runtime issues.

Apply this diff to add error handling:

    # create short-name copies like "abcdef-....woff2" -> "abcdef.woff2" for fonts and other assets
    bash -lc 'for f in public/assets/*-*.*; do \
      [ -f "$f" ] || continue; \
      base=$(basename "$f"); \
      ext="${base##*.}"; \
      short="${base%%-*}.${ext}"; \
      if [ ! -e "public/assets/$short" ]; then cp --preserve=mode,timestamps "$f" "public/assets/$short"; fi; \
    done';

Suggested fix — wrap the script with error handling:

    # create short-name copies like "abcdef-....woff2" -> "abcdef.woff2" for fonts and other assets
    bash -lc 'set -e; \
    for f in public/assets/*-*.*; do \
      [ -f "$f" ] || continue; \
      base=$(basename "$f"); \
      ext="${base##*.}"; \
      short="${base%%-*}.${ext}"; \
      if [ ! -e "public/assets/$short" ]; then cp --preserve=mode,timestamps "$f" "public/assets/$short"; fi; \
    done'; \

This ensures the build fails immediately if any cp command encounters an error, rather than silently continuing with partial state.

🤖 Prompt for AI Agents
In Dockerfile around lines 48 to 55, the bash loop that copies short-name asset
files lacks error handling so a failing cp will be ignored and the build can
continue with incomplete assets; modify the script to fail fast (e.g., wrap the
command with a bash -euo pipefail environment or prepend set -e; set -o
pipefail) and ensure each cp failure causes the whole command to exit non-zero
(alternatively check cp's exit status and exit 1 on failure), so the Docker
build stops immediately if any copy fails.

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.27%. Comparing base (7f38db6) to head (214c579).
⚠️ Report is 1 commits behind head on staging.

Additional details and impacted files
@@           Coverage Diff            @@
##           staging    #1128   +/-   ##
========================================
  Coverage    75.27%   75.27%           
========================================
  Files           50       50           
  Lines         1076     1076           
========================================
  Hits           810      810           
  Misses         266      266           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lodewiges lodewiges merged commit 485b144 into staging Nov 23, 2025
6 checks passed
@lodewiges lodewiges deleted the hotfix/serve-woff-files branch November 23, 2025 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant