forked from artofhuman/activeadmin_settings_cached
-
Notifications
You must be signed in to change notification settings - Fork 0
Migrate to ActiveAdmin 4, Rails 8, and rails-settings-cached 2.x #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
glebtv
wants to merge
10
commits into
master
Choose a base branch
from
refactor
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
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
This commit completes the migration to modern dependencies: - ActiveAdmin 4.0.0-beta16 (Tailwind CSS + esbuild) - Rails 8.0 - rails-settings-cached 2.9.6 ## Major Changes ### 1. ActiveAdmin 4 Setup with Tailwind + esbuild - Added complete test app infrastructure in spec/internal/ - Configured Tailwind CSS build with @activeadmin/activeadmin plugin - Set up esbuild for JavaScript bundling (IIFE format) - Created Rake tasks for CSS/JS builds - Added GitHub Actions CI workflow ### 2. Modernized for rails-settings-cached 2.x API - Removed legacy scoped settings (dotted field names) - Updated Setting model to use modern field definitions with scope - Migrated from `Setting['key']` to `Setting.key` API - Updated all test fixtures to use new API ### 3. Fixed Test Suite - Fixed Coercions spec (removed extra display parameter) - Completely rewrote settings_spec.rb for modern API - Updated model_spec.rb field names - Installed Playwright for feature tests - All 27 tests now passing ### 4. Build Configuration - Added package.json with Tailwind, esbuild, @activeadmin/activeadmin - Created esbuild.config.js for JavaScript bundling - Created tailwind.config.js with ActiveAdmin plugin - Added Rake tasks for CSS/JS compilation ### 5. Updated Dependencies - Bump to ActiveAdmin 4.0.0-beta16 - Require rails-settings-cached >= 2.0.0 - Added Rails 7.2 and 8.0 gemfiles for CI - Removed Rails 5.1 gemfile 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add rubocop to development dependencies (was missing from gemspec) - Use consistent hyphenated naming in Appraisals (rails-7.x-active-admin-4.x) - Regenerate all appraisal gemfiles with correct naming (underscores) - Remove old gemfiles with incorrect naming pattern This ensures CI can find the correct gemfiles and lint job can run rubocop. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
## Code Quality Improvements - Add .rubocop.yml configuration with NewCops enabled - Add RuboCop extension gems (capybara, rake, rspec, rspec_rails) - Auto-fix 162+ style violations across codebase - Configure reasonable cop limits for project patterns - All code now passes RuboCop linting (20 files, 0 offenses) ## Documentation - Create comprehensive upgrade guide: docs/upgrade-to-v3.md - Step-by-step upgrade instructions from v1.x/v2.x - rails-settings-cached 0.x → 2.x migration guide - ActiveAdmin 4 setup instructions with Tailwind CSS - Complete asset pipeline configuration examples - Troubleshooting section for common issues - Migration checklist and rollback plan - Update README.md: - Add v3.0 announcement with key features - Add compatibility matrix table - Update CI badge to GitHub Actions - Add ActiveAdmin 4 setup section - Update examples to rails-settings-cached 2.x syntax - Add development instructions for contributors - Link to upgrade guide - Update CHANGELOG.md: - Set release date: 2025-10-10 - Add link to upgrade guide ## Key Changes All code is now: - ✅ RuboCop compliant with modern cops - ✅ Consistently styled with frozen string literals - ✅ Using Ruby 1.9+ hash syntax - ✅ Following RSpec best practices - ✅ Well-documented for v3.0 release This prepares the codebase for v3.0 release with: - Clean, linted code - Professional documentation - Clear upgrade path for users - ActiveAdmin 4 and Rails 8 support 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
## Problem CI was failing with: "Error: Some specified paths were not resolved, unable to cache dependencies" The setup-node action was trying to cache npm dependencies using spec/internal/package-lock.json, but this file is gitignored and not committed to the repository. ## Root Cause - spec/internal/package-lock.json is in .gitignore (line 24) - File exists locally but not in git repository - setup-node@v4 with cache-dependency-path fails when file doesn't exist - This is documented in: actions/setup-node#694 ## Solution Removed Setup Node.js step from CI workflow since: 1. Ubuntu-latest runners include Node.js by default 2. We don't need npm caching for test infrastructure files 3. The working reference (activeadmin-searchable_select) doesn't use setup-node in test jobs 4. Changed npm ci to npm install (more forgiving without package-lock.json) ## Verification Compared with working setup at: /data/activeadmin-searchable_select/.github/workflows/ci.yml Their test job doesn't use setup-node either - just runs npm install directly. This matches the pattern used by other successful CI configurations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The db:create and db:schema:load rake tasks are only available within a Rails application context. Since this is a Rails engine gem, these tasks need to be run from the spec/internal directory (the Combustion test app) rather than the gem root. Changed: - Run database setup from spec/internal directory - Use ./bin/rails instead of bundle exec rake - Match pattern from working activeadmin-searchable_select setup
Created bin/rails and bin/rake scripts needed for running database tasks in CI. These scripts are required for the Combustion test app to function properly. Files added: - spec/internal/bin/rails - Rails command runner - spec/internal/bin/rake - Rake task runner Both scripts are executable and follow the standard Rails binstub pattern.
Added missing preloaded_module_packages method to ImportmapStub and an importmap helper method to return the stub instance. This fixes test failures on Rails 8 where the real importmap helper was calling preloaded_module_packages on our stub. The preloaded_module_packages method was added in importmap-rails 2.2.2 for module preloading optimization. Our stub now returns an empty array, which is sufficient for test purposes. Fixes all 8 failing tests in Rails 8.0 gemfile.
The spec/internal directory is a full Rails application used for testing, not a Combustion setup. It needs its own Gemfile to work properly. This Gemfile: - Defines all dependencies for the test Rails app - Loads the gem under test via path: '../..' - Supports both Rails 7.2 and 8.0 via RAILS_VERSION env var - Enables running 'bundle exec rackup' for development - Matches the pattern used in activeadmin-searchable_select Benefits: - Cleaner and simpler than Combustion - Full Rails app is easier to maintain and debug - Works with standard Rails commands (rails, rackup, etc) - No hidden magic or framework abstractions The parent Appraisals gemfiles still control which Rails version is used in CI via BUNDLE_GEMFILE environment variable.
ActiveAdmin 4 requires importmap-rails for JavaScript management. Instead of using hacky stubs that don't reflect real-world usage, we now properly include importmap-rails as a dependency. Changes: - Removed ImportmapStub and related helper shims - Added importmap-rails (>= 2.0) to both root and spec/internal Gemfiles - Created spec/internal/config/importmap.rb with proper configuration - Updated boot.rb comment to clarify Appraisals behavior - Updated README.md to list importmap-rails as required dependency - Updated upgrade guide to emphasize importmap-rails requirement Benefits: - No hacks or stubs - uses real importmap-rails - Matches what actual users will do in their apps - Cleaner, more maintainable code - Better documentation for users All 27 tests pass with real importmap-rails.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This commit completes the migration to modern dependencies:
Major Changes
1. ActiveAdmin 4 Setup with Tailwind + esbuild
2. Modernized for rails-settings-cached 2.x API
Setting['key']toSetting.keyAPI3. Fixed Test Suite
4. Build Configuration
5. Updated Dependencies
🤖 Generated with Claude Code