Skip to content

[WIP] Replace Comfortable Mexican Sofa with Comfortable Media Surfer Gem; address #737#885

Open
jefferya wants to merge 10 commits intomainfrom
jefferya/737_comfy_gem
Open

[WIP] Replace Comfortable Mexican Sofa with Comfortable Media Surfer Gem; address #737#885
jefferya wants to merge 10 commits intomainfrom
jefferya/737_comfy_gem

Conversation

@jefferya
Copy link
Contributor

@jefferya jefferya commented Nov 6, 2025

Address #737

Depends on PR #884, Rails 8.1 PR to be merged first.

⚠️ Requires DB migration and should be tested with a production DB dump
⚠️ May require changes to the RPM build pipeline: new Gem requires an installation of npm which is used by bundle exec rails comfy:compile_assets. comfy:compile_assets installs NPM modules and bundles JS/CSS for the CMS Engine Gem.

Comfortable Mexican Sofa is no longer being supported. Comfortable Media Surfer Gem is a fork that is being actively updated. This PR is the replacement.

How to test in staging and prod; these pass in dev:

  • public pages visible and align with production (need to update seed data in GitHub and staging site)
  • login to the admin interface, if application log errors regarding missing JavaScript/CSS then check if comfy:compile_assets was successful
  • check pages with cms:snippet like "ask-us" work; if errors like markdown not found then the DB migration might be missing

Todo:

  • Add a how to test section? Highlight need to test pages with cms:snippet like "ask-us" due to errors like markdown not found (see DB migration for the reason). There might be more.
  • Update seeds with production data Update Seeds #803
  • Update staging with production data
  • what does comfy_bootrap_form do? is it required? - YES
  • dump schema: Rails 8.1 will sort column names in the schema - is this a problem? No but test
  • GitHub Action tests fail but tests pass locally. Local test passed due to preexisting javascript/CSS bundles (./tmp/sprockets and ./tmp/cache). Resting local generates similar errors as GitHub action. The error:
Error:
CmsAdminTest#test_admin_site_pages_index:
ActionView::Template::Error: Error: File to import not found or unreadable: codemirror/lib/codemirror.
        on line 3:1 of vendor/bundle/ruby/3.3.0/gems/comfortable_media_surfer-3.1.4/app/assets/stylesheets/comfy/admin/cms/application.sass
>> @import "codemirror/lib/codemirror";
   ^
    test/integration/cms_admin_test.rb:27:in `block in <class:CmsAdminTest>'
bin/rails test test/integration/cms_admin_test.rb:26

Resolution:

  • Assets not built in the new CMS Gem / rails engine triggering the "not found error"
  • The new CMS Gem / Rails Engine doesn't compile assets whereas the previous Gem included the vendored assets in the Gem within the directory app/assets/javascript/comfy/vendor and assets:precompile used the path in lib/comfortable_mexican_sofa/engine.rb to obtain the assets.
  • Resolution option (1): hook lib/tasks/comfy.rake CMS task onto the assets:precompile task Rake::Task["assets:precompile"].enhance(["comfy:compile_assets"])
  • The order of operations when sprocket-rails controls the assets:precompile means that adding the .enhance(["comfy:compile_assets"]) to Rake::Task["assets:precompile"] yields missing assets or a esbulid version collision because the CMS Gem includes a JS version lock file (not best practice).
  • Solution: for now, run comfy:compile_assets as a separate task after codebase is available but before assets:precompile

* Markdown not found error when cms:snippet interpreted in "ask-us" and "" page tests and when accessing web UI
Process:
* bin/rails railties:install:migrations (to get migration)
* bin/rails db:migrate:status
* bin/rails db:migrate
@jefferya jefferya force-pushed the jefferya/737_comfy_gem branch from 964b1d1 to 1b68816 Compare November 7, 2025 15:57
Comment on lines -44 to -55

###
# Disable automatic column serialization into YAML.
# To keep the historic behavior, you can set it to `YAML`, however it is
# recommended to explicitly define the serialization method for each column
# rather than to rely on a global default.
###
# Rails.application.config.active_record.default_column_serializer = nil
# comfortable_mexican_sofa v3.5 https://github.com/restarone/comfortable-mexican-sofa.git
# not updated for Rails 7.1 to specify serialization method for the
# Comfy::Cms::Fragment and Comfy::Cms::Revision models
Rails.application.config.active_record.default_column_serializer = YAML
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the change to Comfortable Media Surfer means this workaround is no longer required.

@jefferya jefferya mentioned this pull request Nov 7, 2025
@jefferya jefferya force-pushed the jefferya/737_comfy_gem branch 3 times, most recently from e5d5484 to aecfd4b Compare December 4, 2025 00:05
@jefferya jefferya force-pushed the jefferya/737_comfy_gem branch from 50f005b to 12d6542 Compare December 4, 2025 00:39
Comment on lines +49 to +53
- name: Compile CMS Engine assets
run: bundle exec rails comfy:compile_assets
env:
RAILS_ENV: test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Required to install and build the CMS Engine's javascript and CSS. The new Gem doesn't include the vendor javascript libraries and bundles.

Comment on lines +77 to +81
- name: Test - precompile Rails assets
run: bundle exec rails assets:precompile
env:
RAILS_ENV: test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add a test to see if assets:precompile works.

Comment on lines +1 to +9
# frozen_string_literal: true

# This migration comes from comfortable_media_surfer_engine (originally 2)
# null: false added by rubocop, not in original migration
class AddMarkdownToSnippets < ActiveRecord::Migration[7.1]
def change
add_column :comfy_cms_snippets, :markdown, :boolean, default: false, null: false
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DB migration for the new CSM Gem. PR includes a warning to highlight the need.

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