Skip to content

Conversation

@Oaphi
Copy link
Member

@Oaphi Oaphi commented Jan 13, 2026

closes #1941
closes #964

Primariy, this PR adds two new site settings: CSSPath and JSPath and makes sure requests for those assets are only made if these new settings are set:

Screenshot from 2026-01-13 05-13-18

To address security concerns raised in #964, these settings are of a new type uri_path that only allows domain-relative URLs (f.e., /assets/community/codegolf.js). Unexpected values are fixed up under the hood or dropped entirely if invalid.

The PR also heavily reworks seeds.rb to simplify working with it going forward as well as enables linting for db/* source files (most of the fixes are from autocorrectable suggestions - manual fixes to pay more attention to are indicated via comments here).

Our production Judaism and Code Golf settings will be automatically populated upon running seeds - I've tested locally, but we'll need to double check that upon deployment to prod.

@Oaphi Oaphi changed the title 0valt/1941/community assets settings Make community assets paths configurable Jan 13, 2026
Comment on lines +8 to +9
in_sql = "(select id from posts where community_id = #{community.id} and category = '$cat')"
sql = "select tag_id from posts_tags where post_id in #{in_sql}"
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a manual line length fix

Comment on lines +7 to +9
configured_service = ActiveStorage::Blob.service.name

if configured_service
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a manual fix for assignment in condition

@codecov
Copy link

codecov bot commented Jan 13, 2026

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 79.67%. Comparing base (b8d48e6) to head (e62f1bc).

Files with missing lines Patch % Lines
app/helpers/seeds_helper.rb 88.88% 1 Missing ⚠️
Additional details and impacted files
Components Coverage Δ
controllers 74.70% <ø> (ø)
helpers 84.95% <88.88%> (+0.03%) ⬆️
jobs 80.59% <ø> (ø)
models 90.09% <100.00%> (ø)
tasks 61.11% <ø> (ø)

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

# Lifesaving reference: https://stackoverflow.com/q/1294117/3160466

tables_columns = ActiveRecord::Base.connection.tables.map { |t| [t, ActiveRecord::Base.connection.columns(t)] }.to_h
tables_columns = ActiveRecord::Base.connection.tables.to_h { |t| [t, ActiveRecord::Base.connection.columns(t)] }
Copy link
Member Author

Choose a reason for hiding this comment

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

map.to_h -> to_h with block - tested locally (note that I've also made the script resilient to errors as it turns out it fails on our new FULLTEXT index columns [title, body] for posts - will be addressed in a separate PR)

Copy link
Member

Choose a reason for hiding this comment

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

Above my pay grade to evaluate. :-)

Comment on lines +22 to +26
sql = "ALTER TABLE `#{t}` MODIFY `#{c.name}` #{c.sql_type} CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;"
ActiveRecord::Base.connection.execute sql
rescue => e
puts "failed to reset collations on #{t}"
puts "message: #{e.message}"
Copy link
Member Author

Choose a reason for hiding this comment

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

Manual line length fix + robustness improvement mentioned above

@Oaphi Oaphi requested review from a team, ArtOfCode-, cellio and trichoplax January 13, 2026 02:43
Copy link
Member

@cellio cellio left a comment

Choose a reason for hiding this comment

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

I tested locally and it looks good to me. I tried valid and invalid values and got the expected behaviors. Somebody with more experience should review the seeds changes.

# Lifesaving reference: https://stackoverflow.com/q/1294117/3160466

tables_columns = ActiveRecord::Base.connection.tables.map { |t| [t, ActiveRecord::Base.connection.columns(t)] }.to_h
tables_columns = ActiveRecord::Base.connection.tables.to_h { |t| [t, ActiveRecord::Base.connection.columns(t)] }
Copy link
Member

Choose a reason for hiding this comment

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

Above my pay grade to evaluate. :-)

category: SiteDetails
description: >
Path to a file containing site-specific JavaScript.
Only accepts domain-relative URLs (such as `/assets/community.js`).
Copy link
Member

Choose a reason for hiding this comment

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

How about: Must be a domain-relative URL (such as...) ? It does accept others; it just quietly ignores them. For the previous setting we use the "must be" language, so we could use it for these two too.

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.

Global site setting for community assets Avoid redundant requests for community specific JS and CSS files

3 participants