Fix checkbox icon: update Font Awesome 5 to 6 - #1429
Open
superturbo wants to merge 52 commits into
Open
Conversation
Metafield names are unique only within a namespace, but Site#find_metafield resolved a field by name across all namespaces. When two namespaces declared the same field name with different `localized` flags (a localized contacts.address and a plain mailer_settings.address), SiteMetafieldsService picked the wrong definition and persisted a per-locale Hash where a plain scalar was expected — later crashing the mailer with "no implicit conversion of BSON::Document into String". - update_all resolves the field within the namespace being written - cast_metafields resolves schema/field within the namespace (read path now consistent with write path) - names normalized through a single Site#normalize_metafield_name, mirroring the backoffice form, so uppercase/spaced schema names still match - CurrentSiteMetafieldsHelper reuses the same normalization
- replace webdrivers with selenium-webdriver >= 4.28, < 5 so the suite uses Selenium Manager with current Chrome - resolve locomotive.local inside Chrome and disable HTTPS upgrades for the plain-http Capybara server - wait for the forgot-password post-submit page state instead of sleeping before reading ActionMailer deliveries - temporarily pin Steam to a Liquid-4-compatible ref with Nokogiri ~> 1.19 for Ruby 3.4; the Steam 2.0/Liquid 5 commit will replace this pin Before: system specs could not run because Chrome could not resolve locomotive.local. After: system specs completed in this run: 23 examples, 0 failures. A browser-race flake remains in forgot-password/public-form flows and is tracked separately.
- liquid parser cache: full marshal round-trip against a persisted site graph (extends + snippet + section); a cache hit must render the same output as the cold parse through the registers required at render time - public_submission_title: first coverage of this Liquid render path (entry/site assigns, extra options, blank template) - page parsing service: template and snippet syntax errors are swallowed, logged, and the service returns nil, whatever error class the Liquid/Steam version in use raises
script/benchmarks/rendering_benchmark.rb measures the engine layer only (PageParsingService orchestration; LiquidParserWithCacheService cold parse + cache write + render vs warm cache read + Marshal.load + render); raw Liquid parse/render throughput is covered by steam's own benchmark and deliberately not repeated here. The warm scenario renders the marshal-loaded template through the render-time registers required by snippet/section rendering and aborts if the snippet content is missing, so a broken cache path cannot go unnoticed. page_parse_* scenarios measure the repeated backoffice parse path (editable elements already exist after warmup). Deterministic seed in a dedicated database (name must contain 'benchmark'; Mongoid.override_database + Locomotive::Steam.configuration.adapter retargeted before the Steam Mongo session is memoized); 5 warmup + 30 measured runs, p50/p95 wall time and allocations per run, human table on stdout plus a JSON artifact for the PR comparison table.
- rails '>= 7.1' -> '>= 7.2.3.1', '< 8' (the ecosystem is capped at ActiveSupport < 8 by locomotivecms_common/custom_fields; 7.2.3.1 also clears the actionview/activestorage/activesupport advisories) - mongoid '~> 8.0.7' -> '~> 8.1': has to travel with Rails because mongoid 8.0.x caps activemodel < 7.2; the mongo driver / bson stay put and move in their own commit - rails-i18n '~> 7.0.6' -> '~> 7.0', '>= 7.0.6' (lock resolves to 7.0.10, the last line targeting railties < 8) - rspec-rails '~> 6.0.1' -> '~> 6.1': the 6.0.x railtie still assigns action_mailer preview_path (singular), which Rails 7.2 removed -- development boot failed without it Suite: 1069 non-system examples green, 23 system examples with the same known pre-existing flake profile as on 7.1. Engine-layer benchmark p50: page parse paths -12..-17%, liquid cache paths flat, allocations +1.5%.
- locomotivecms_steam '~> 1.8.0.alpha1' -> '~> 2.0.0.alpha1' (Liquid 4.0.4 -> 5.13.0) - remove the engine-side Liquid AST cleaner; parsed template marshalling is now owned by locomotivecms_steam's Liquid::MarshalCache - cache writes Marshal.dump the parsed template directly; cache hits rely on Steam to strip parse-time service objects and rebuild Liquid 5 parser state on load
- mongo 2.24 / bson 5; rack 3.1.21, puma 7.2, devise 5.0.4, jquery-ui-rails 8.0 (jquery-ui 1.14.1), net-imap, crass, concurrent-ruby, rexml, bcrypt, addressable, erb - carrierwave 1.3 -> 2.2.7: extension_allowlist rename and a CarrierWave 2 port of the SafeRemove patch -- the remove_<column> flow now deletes through Mounter#remove_previous, where the Mongoid dirty-tracking sentinel used to be taken for the file name, silently orphaning files; destroying a record now removes a file shared by two fields - page parsing: unsubscribe the AS::Notifications subscribers on parse errors too (they leaked for the lifetime of the process)
Liquid 5 / steam 2.0 is a breaking site-template runtime change, the engine now requires Rails >= 7.2.3.1 and Ruby >= 3.2, so this starts the 5.0 major line. Parsed-template cache keys include Locomotive::VERSION, so this release automatically busts existing template cache entries.
Move runtime gemspec constraints to current compatible lines: pundit 2.5, simple_form 5.4, slim 5.2, responders 3.2, mongoid-tree 2.3, rails-html-sanitizer 1.7, rack-cache 1.17, multi_json 1.21, devise-encryptable 0.3, font-awesome-sass 6.7, jquery-rails 4.6, sprockets-rails 3.5. Refresh dev/test dependencies: debug 1.11, factory_bot_rails 6.5, email_spec 2.3. The lockfile also picks up related transitive updates including kramdown 2.5, mail 2.9, i18n 1.15, loofah 2.25, ssrf_filter 1.5, tilt 2.8, launchy 3, public_suffix 7, thor 1.5, and zeitwerk 2.8.
Engine follows the compatible dependency bounds: custom_fields ~> 2.14.0.alpha2, mongoid >= 8.1, < 10, and carrierwave-mongoid >= 1.4, < 2.
Upgrade grape from 2.1.3 to 3.3.2 and grape-entity from 0.10.2 to 1.1.0. Grape 3.3 requires Ruby >= 3.3, so the engine Ruby floor moves with it.
Remove the BSON::ObjectId#to_json/#as_json reopening from
lib/locomotive/mongoid/patches.rb and require bson >= 5.2, where
ObjectId#as_json returns the plain id string and as_extended_json keeps
the BSON extended JSON representation.
Add a spec pinning the engine JSON invariant: ObjectIds serialize as
plain id strings in API/Jbuilder output, not as {"$oid": ...}.
Remove the Mongoid::Criteria#first! override from lib/locomotive/mongoid/patches.rb while keeping the unrelated without_sorting helper. Mongoid 8.1 provides Criteria#first! through context delegation: it returns the first matching document and raises Mongoid::Errors::DocumentNotFound when the criteria is empty. That is the contract used by the content entries, content entry imports, and content entry impersonations controllers for unknown content type slugs. Add a characterization spec for the embedded content type criteria path. The native DocumentNotFound message no longer includes the selector, but the engine rescue path is class-based, so HTTP behavior is unchanged.
Constrain Mongoid to >= 8.1, < 9. The previous < 10 bound overstated what the engine currently supports: Mongoid 9 removes Mongoid.legacy_pluck_distinct, the engine still has five production update_attributes call sites, and it still reopens several Mongoid internals.
Refresh compatible dependency lines: json-schema 3.0 -> 6.2 and highline 2.1 -> 3.1 in the gemspec; puma 7 -> 8, rspec-rails 6.1 -> 8, and shoulda-matchers 5.3 -> 8 in dev/test. json-schema is only used by the json_attribute validation concern, and the sections specs still pin the exact JSON::Validator error message. highline is a runtime gemspec dependency, but the engine only uses highline/import from the development rake tasks. The lockfile also picks up patch/minor updates for rspec 3.13.2, rspec-core/mocks/support, error_highlight 0.7.2, date 3.5.1, dry-core/inflector/types, rack_csrf 2.7, rackup 2.3.1, io-console, reline, and pp.
AccountPolicy#permitted_attributes now restricts super_admin and api_key to super admins. PUT /api/v3/accounts/:id filters the declared account parameters through the policy before passing them to AccountForm. Previously, an authenticated non-visitor account could submit super_admin=true when updating itself and gain global administrative access. Regression coverage verifies that privileged attributes are ignored while ordinary profile updates still succeed. Existing coverage confirms that a super admin can still grant super-admin access.
Add regression coverage for observable Mongoid compatibility behavior: - Criteria#indexed_max respects selectors and empty collections - Criteria#each_by yields every matching document once and in order - Criteria#without_sorting removes ordering without mutating the source - symbol operators remain compatible with plain selector clauses - localized form inputs render the value for the current locale - Document#as_json mirrors _id into id
Mongoid::Criterion is absent from the supported Mongoid 8.1.12 line. The reopen therefore creates a replacement namespace and Selector class that Mongoid never references. No engine, steam, custom_fields or common code uses the namespace.
The Site and ImageThumbnail middlewares build raw Rack responses with capitalized header keys, which Rack 3 requires to be lowercase. Update the keys and assert the full raw response header hash in the middleware specs so the casing can't silently regress.
Replace the fragile numeric middleware position with a named Rack::Runtime anchor. ImageThumbnail remains inside the Rails executor and runtime instrumentation while still intercepting requests before application dispatch. Add a middleware-order spec as a regression gate.
Mongoid 9 changes embedded_in to touch its parent by default. Set touch: false explicitly for memberships and editable elements to preserve the existing behavior and avoid unnecessary parent timestamp writes. Add regression coverage proving that updating either embedded document leaves the parent's updated_at unchanged.
Set legacy_pluck_distinct to false so pluck and distinct demongoize their results. For localized fields, this returns the value for the active locale instead of the raw translations hash. Simplify ContentEntry#similar_slug to consume the returned string directly and update the service spec to lock the new localized pluck result shape. Add a non-default-locale slug collision spec proving that similar_slug increments within the active locale.
ContentEntry#add_to_list_bottom is the only engine caller of the Mongoid::Criteria#indexed_max and Mongoid::Findable#indexed_max monkey patches. Replace both global patches with Mongoid's public order_by(...).pick(:_position) API. The query keeps the same scoped descending sort, single-field projection and limit of one while removing private selector API and direct raw-driver access. Move position-assignment coverage to ContentEntry and verify that positions remain isolated per content type.
The custom Criteria#each_by implementation used limit/skip pagination, causing MongoDB to scan increasingly large offsets, and contained an inverted criteria-limit guard. Replace all eleven engine call sites with Mongoid's public cursor batching API and remove Criteria#each_by and its private ordered_clone helper. CSV export and localization paths use batch_size(...).each; PageService also replaces its use of without_sorting with reorder(depth: :asc). The legacy locomotive:upgrade:v3 task uses no_timeout.batch_size(10).each because its nested migration passes can perform substantial work between cursor reads, making a mid-migration cursor timeout worse than holding the cursor open. Existing CSV, content-entry localization and depth-ordered page localization specs continue to cover the affected application behavior.
empty_collection? removes ordering before its existence check to avoid an unnecessary unindexed sort on large criteria. Replace the engine's Criteria#without_sorting monkey patch with Mongoid's public reorder API. Criteria are checked through reorder.empty?, while plain arrays continue to use empty? directly. Remove the patch, leaving the direct Criteria reopen responsible only for to_liquid. Move coverage to empty_collection? itself, including empty and non-empty criteria and the plain-array fallback.
Bump mongoid >= 8.1, < 9 to >= 9.1.0, < 10. The lock change is scoped to Mongoid: its ActiveModel bound widens to < 8.2, its ruby2_keywords dependency is replaced by ostruct (already present in the lock), and the now-unused ruby2_keywords gem is removed. carrierwave-mongoid 1.5.0 and mongoid-tree 2.3.0 already satisfy their constraints and remain unchanged. Two code changes are required by the upgrade: - Remove the locomotive.mongoid initializer because Mongoid 9 removed the legacy_pluck_distinct option and raises NoMethodError when it is assigned. - Under Mongoid 9, SiteMetafieldsService#update_all no longer persists its in-place, symbol-keyed update of a localized metafield. Assign a fresh hash with a string locale key instead. The existing site_metafields_service spec fails before this fix and passes unchanged after it. The remaining Mongoid compatibility code is unchanged. Existing characterization coverage for localized form fields, Origin query operators, editable-element type morphing and document JSON remains green.
The Criteria::Queryable::Selectable#selection override allowed Origin::Key objects from Steam's origin dependency to coexist with Mongoid's own query keys, preserving symbol-operator queries such as where(:position.gte => 1). Mongoid 9.1 query compilation now accepts Origin::Key through the shared expression protocol, so the private selection override is no longer needed. The existing selection spec exercises the current Origin::Key load order and remains green without the patch.
Replace the Rails version derived config.load_defaults value with a literal 7.2 so the dummy application's behavior remains stable when tested against different supported Rails versions. This separates engine compatibility with the installed Rails version from the application defaults compatibility level.
Upgrade Rails 7.2.3.1 -> 8.1.3 and rails-i18n 7.0.10 -> 8.1.0. Use simple_token_authentication from a pinned upstream commit whose Action Pack and Action Mailer constraints permit Rails 8. Apply the compatibility changes required by Rails 8.1: - resolve Locomotive.mounted_on through the route helper so Rails can load its lazy route set before the first request; - keep update_all as an explicit collection route instead of listing it as a non-REST resource action; - remove the obsolete rails/tasks/statistics.rake load; - remove the unused dummy Action Cable scaffold that broke Zeitwerk eager loading.
Move the dummy application from Rails 7.2 to 8.1 defaults, separately from the framework dependency upgrade, so the engine suite exercises the new defaults explicitly. The relevant redirect, JSON/JavaScript escaping and generated hidden-field changes were audited against the engine's call sites. Active Record defaults do not apply to this Mongoid engine; the remaining changes are runtime/performance configuration. Full non-system suite: 1092 examples, 0 failures.
ActionView::Base#escape_json existed only as a Locomotive monkey patch and returned json.html_safe without escaping anything. Rails 7.2 and 8.1 do not provide or call this method, and the engine has no production call sites. Its isolated spec only asserted that U+2028 and U+2029 were returned unchanged. Remove the patch, the one-line locomotive/rails require wrapper, its top-level require and the misleading spec. This removes a global ActionView reopen and an unnecessary html_safe path without changing runtime behavior.
Add regression coverage for the current token issuance, lifecycle and request-authentication behavior: - password issuance accepts case-insensitive email; a wrong password and an unknown email return the same status, body and X-Error-Detail header; - account api_key authentication takes priority over email/password and does not fall back to a valid password when the api_key is wrong; email remains required and Grape returns 422 when it is missing; - authentication tokens are generated and persisted, remain stable across issuance and unrelated account updates, and differ between accounts; - authentication_token is not exposed by AccountEntity and cannot be mass-assigned through the account API; - X-Locomotive-Account-Email and X-Locomotive-Account-Token are validated as a pair, with missing, blank, unknown and cross-account credentials rejected; - a site member remains authenticated on a site-scoped endpoint.
…ken_authenticatable Replace simple_token_authentication's acts_as_token_authenticatable macro with an explicit Account before_save callback and ensure_authentication_token method. When authentication_token is blank, the model generates a Devise.friendly_token and retries while that value already exists in the unscoped accounts collection. This preserves the existing token generation, issuance and request-authentication behavior while removing the Account model's runtime dependency on the STA macro.
The gem's last active engine responsibility was Account token generation, which is now owned by Account#ensure_authentication_token. Remove the dependency and its temporary Rails 8 git bridge, the obsolete STA Entity monkey-patch that normalized the namespaced Locomotive::Account model into token parameter and header names, the unused TokenAuthenticationController concern, and the unsafe_token_authentication configuration consumed only by that concern. The Grape API authenticates independently of STA by reading the X-Locomotive-Account-Email and X-Locomotive-Account-Token headers directly. Regression coverage confirms that the legacy auth_token query parameter does not authenticate API requests. Removing STA also lifts the final dependency constraint preventing the published engine gem from resolving with Rails 8: simple_token_authentication 1.18.1 requires Action Pack and Action Mailer below 8. The supported header-based API contract is unchanged. Host applications that directly included the removed, undocumented TokenAuthenticationController concern must remove that integration.
AuthenticationHelper#current_membership handed any authenticated account a synthetic Membership, defaulting to the author role, when it had no membership for the requested site. This allowed authenticate_locomotive_account! to pass. Combined with SitePolicy#show? being unconditional and PagePolicy and ContentTypePolicy allowing non-hidden resources, a member of one tenant could read another tenant's current_site by handle and individual pages or content types when their identifiers were known. Return nil when an ordinary account has no membership for the requested site, causing site-scoped API authentication to fail with 401. Preserve the existing super-admin cross-tenant behavior, consistent with SitePolicy::Scope, and leave no-site endpoints such as /my_account and /sites unchanged. Also require super_admin? or site_staff? in SitePolicy#show? as an additional authorization guard.
TokenResource rescued Exception and classified every failure, including unexpected application and non-application exceptions, as a 401 while exposing the exception message in the response body and X-Error-Detail header. Introduce Account::AuthenticationError for the expected credential rejection paths in Account.create_api_token. Rescue only that exception in /tokens and return a uniform 401 response without internal credential details. Unexpected errors now fall through to the existing API exception boundary and retain their 5xx classification instead of being masked as authentication failures.
Account.invalidate_api_token has no engine callers and was never exposed by the token resource; its only endpoint reference was a stub comment left in 2015. The method did not revoke or rotate the persisted token. Its reset_authentication_token! call resolves to Mongoid dirty tracking, which leaves an unchanged persisted attribute intact, and the method then returns the same token. Remove the misleading method and the stale endpoint stub. Token issuance and the supported header-based authentication contract are unchanged.
Constrain the published engine to the verified dependency lines: Rails ~> 8.1.3, rails-i18n ~> 8.1.0 and Mongoid ~> 9.1.0. Refresh the development and test bundle to Rack 3.2.6, selenium-webdriver 4.46, SimpleCov 1.0 and web-console 4.3.
Rack already decodes form parameters before they reach json_attribute. CGI.unescape decoded them again, converting "+" to spaces and expanding literal percent sequences such as "%20" inside JSON values. Parse JSON directly and add model and /current_site persistence regressions.
LoggerMiddleware bypassed Rails parameter filtering, exposing /tokens passwords and API keys and X-Locomotive-Account-Token headers in plaintext. Apply the configured filters to API params and X-* headers, register the engine's credential keys, and add regression coverage for both paths.
The catch-all API handler exposed exception messages in 500 responses. Return a generic error body while retaining full server-side logging. Add regression coverage for unexpected endpoint failures.
Mongoid 9.1 keeps update_attributes as an alias for update. Use the canonical update API across production code, specs, and mocks. Behavior is unchanged.
flash_cookie_session passes Rails sessions to legacy Flash/SWF uploaders. The engine no longer ships such an uploader and does not require the gem, so its Railtie and middleware are inactive.
Site SMTP settings previously mutated whichever delivery method the host app configured. Select Mail::SMTP explicitly so they cannot use a non-SMTP host adapter.
Site notifications without site SMTP currently use the host application's mail transport, consuming shared resources in multi-tenant deployments. Add a backward-compatible Locomotive setting to disable that fallback. Site SMTP delivery and account emails remain unaffected. Skipped deliveries are reported through Locomotive::Common::Logger with the site id and no recipient data.
config.mailer_sender is validated at boot via Mail::Address and must be a complete address (display-name form allowed, e.g. "Locomotive <support@example.com>"). The legacy completion that appended @config.domain to a bare local part is removed; an invalid value raises ArgumentError from Locomotive.configure.
Site notification senders were selected independently of their delivery transport. This allowed the application transport to send as synthesized site-domain addresses it might not be authorized for. Site SMTP now requires a valid `from`; otherwise delivery is skipped and reported through Locomotive::Common::Logger. Notifications sent through the application transport use `Locomotive.config.mailer_sender`. Domain-derived noreply addresses are no longer generated.
adomain no longer has any callers after site notifications stopped deriving sender addresses from site domains. Remove it from the gemspec and lockfile.
The initializer globally forced MultiJson to use yajl-ruby for Jbuilder, but current Jbuilder no longer uses MultiJson. Remove the initializer and the engine's direct multi_json and yajl-ruby dependencies.
No specs use its matchers. The rspec meta-gem drops with it; multi_json remains transitive.
HighLine 3 no longer requires abbrev, leaving the explicit dependency unused.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checkbox styles were referencing Font Awesome 5, while the backoffice loads Font Awesome 6.
Updated the font-family to Font Awesome 6 to fix the icon rendering.