Skip to content

Address any legacy patterns #1103

Description

@lortza

Plan: Modernize Legacy Code Patterns to Rails 8 Idioms

Objective

Identify and replace legacy code patterns that are not idiomatic Rails 8, improving code consistency and preparing for future framework upgrades (Bootstrap 5, removal of
jQuery, etc.).

Summary of Findings

The application is well-modernized overall with minimal legacy patterns. The scan identified 7 pattern categories with varying priorities:

  • ✅ JavaScript: Already modernized (Stimulus controllers, no legacy custom JS)
  • ✅ Forms: Mostly modern (form_with used throughout)
  • ✅ Models: Modern callbacks and associations
  • ⚠️ Controllers: Some unnecessary respond_to blocks
  • ⚠️ Views: Bootstrap 4 data attributes, verbose render syntax
  • ⚠️ Devise: Framework-generated form_for usage

Priority 1: Medium Priority (Recommended Next)

1.1 Remove Unnecessary respond_to Blocks in Destroy Actions

Files Affected: 3

  • app/controllers/body_parts_controller.rb:38-40
  • app/controllers/exercises_controller.rb:45-47
  • app/controllers/pains_controller.rb:38-40

Current Pattern:
def destroy
@body_part.destroy
respond_to do |format|
format.html { redirect_to body_parts_url, notice: "#{@body_part.name} was successfully deleted." }
end
end

Rails 8 Idiomatic:
def destroy
@body_part.destroy
redirect_to body_parts_url, notice: "#{@body_part.name} was successfully deleted."
end

Rationale: When only responding to HTML format, respond_to is unnecessary boilerplate.

Implementation:

  1. Edit each controller's destroy action
  2. Remove respond_to block wrapper
  3. Keep redirect and notice message
  4. Run controller specs to verify

Time Estimate: 10 minutes


1.2 Update Bootstrap 4 Data Attributes for Future Bootstrap 5 Migration

Files Affected: 8 view files, ~20 instances

  • app/views/exercise_logs/show.html.erb:34
  • app/views/exercise_logs/_rep_counter_modal.html.erb:16
  • app/views/shared/_admin_nav.html.erb:3
  • app/views/shared/_session_nav.html.erb (multiple lines)
  • app/views/shared/_flashes.html.erb:3
  • app/views/shared/_navbar.html.erb:4

Current Pattern (Bootstrap 4):


Rails 8 + Bootstrap 5 Pattern:



Rationale:

  • Bootstrap 5 changed data- prefix to data-bs- to avoid conflicts
  • Prepares for eventual Bootstrap 4 → 5 upgrade
  • No functional change now (Bootstrap 4 still works)

Implementation Approach:

Option A: Do Nothing Now (Recommended)

  • Wait until Bootstrap 5 upgrade
  • Change all attributes at once during that migration

Option B: Proactive Replacement

  • Use find/replace: data-toggle → data-bs-toggle
  • Use find/replace: data-target → data-bs-target
  • Use find/replace: data-dismiss → data-bs-dismiss
  • Test all modals, dropdowns, and alerts
  • Note: Requires Bootstrap 5 for these to work

Recommendation: Option A - defer until Bootstrap 5 upgrade for efficiency

Time Estimate (if Option B): 20 minutes


Priority 2: Low Priority (Future Cleanup)

2.1 Simplify render partial: Syntax

Files Affected: 45+ instances across many view files

Current Pattern:
<%= render partial: 'shared/filter_form', locals: { destination_path: charts_path } %>

Rails 8 Simplified:
<%= render 'shared/filter_form', destination_path: charts_path %>

Rationale:

  • More concise
  • partial: keyword is redundant when path starts with directory name
  • locals: hash can be passed directly as keyword arguments

Implementation:

  • Low priority due to high file count and working code
  • Consider during broader view refactoring
  • Can be done incrementally per feature area

Time Estimate: 2-3 hours for all instances


2.2 Modernize Devise Views (Optional)

Files Affected: 7 Devise-generated views

  • All views in app/views/devise/ directory

Current Pattern:
<%= form_for(resource, as: resource_name, url: password_path(resource_name)) do |f| %>

Modern Pattern:
<%= form_with(model: resource, url: password_path(resource_name), local: true) do |f| %>

Rationale:

  • form_for is soft-deprecated in favor of form_with
  • However, these are Devise gem-generated views
  • Customizing requires overriding Devise defaults

Recommendation: Skip - Devise maintains these templates, and customization adds maintenance burden. Only customize if needed for design reasons.

Time Estimate (if pursued): 30 minutes


2.3 Remove Legacy jQuery and Bootstrap 4 CDN Scripts

File: app/views/layouts/_scripts.html.erb

Current Pattern:

<script src="https://code.jquery.com/jquery-3.3.1.slim.min.js" ...></script> <script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.14.3/umd/popper.min.js" ...></script> <script src="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/js/bootstrap.min.js" ...></script>

Context:

  • jQuery is required by Bootstrap 4
  • These scripts are loading from CDN
  • Bootstrap 5 does not require jQuery

Recommendation: Defer until Bootstrap 5 upgrade (separate project outlined in CLAUDE.md)

Time Estimate (with Bootstrap 5 upgrade): Part of larger Bootstrap migration


Priority 3: Already Completed ✅

3.1 JavaScript Modernization

✅ Status: Complete

  • SLIT timer converted to Stimulus
  • Rep counter modal converted to Stimulus
  • No legacy custom JavaScript in app/javascript/custom/ (only placeholder cable.js)
  • No jQuery usage in custom code
  • No inline <script> tags in views

3.2 Form Patterns

✅ Status: Modern

  • All custom forms use form_with
  • No remote: true usage
  • No Rails UJS patterns (data-remote, data-method)
  • Turbo properly integrated

3.3 Model Patterns

✅ Status: Modern

  • No string callback names
  • Modern association syntax
  • No deprecated patterns found

Implementation Plan

Phase 1: Quick Wins (30 minutes)

Step 1: Remove Unnecessary respond_to Blocks

File: app/controllers/body_parts_controller.rb (line 38-40)

def destroy
@body_part.destroy

  • respond_to do |format|
  • format.html { redirect_to body_parts_url, notice: "#{@body_part.name} was successfully deleted." }
  • end
  • redirect_to body_parts_url, notice: "#{@body_part.name} was successfully deleted."
    end

Repeat for:

  • app/controllers/exercises_controller.rb:45-47
  • app/controllers/pains_controller.rb:38-40

Step 2: Run Tests
bundle exec rspec spec/controllers/body_parts_controller_spec.rb
bundle exec rspec spec/controllers/exercises_controller_spec.rb
bundle exec rspec spec/controllers/pains_controller_spec.rb

Step 3: Manual Testing

  • Test delete functionality for body parts
  • Test delete functionality for exercises
  • Test delete functionality for pains
  • Verify flash messages appear

Phase 2: Future Cleanup (Defer to Later)

Bootstrap 5 Migration (Separate Epic)

  • Update Bootstrap 4.1.3 → Bootstrap 5.x
  • Replace data-toggle/data-target/data-dismiss → data-bs-*
  • Remove jQuery dependency
  • Remove CDN scripts from _scripts.html.erb
  • Test all modals, dropdowns, alerts

View Syntax Cleanup (Low Priority)

  • Simplify render partial: to render (45+ instances)
  • Can be done incrementally during feature work

Verification Checklist

After Phase 1 (Removing respond_to blocks):

  • All RSpec controller tests pass
  • Delete actions work correctly in browser
  • Flash messages display correctly
  • No regressions in redirect behavior

Code Quality Checks:

  • Run StandardRB: bundle exec standardrb
  • No new linting errors introduced

Files to Modify (Phase 1 Only)

  1. app/controllers/body_parts_controller.rb - Remove respond_to in destroy action
  2. app/controllers/exercises_controller.rb - Remove respond_to in destroy action
  3. app/controllers/pains_controller.rb - Remove respond_to in destroy action

Total: 3 files, 3 changes


Risk Assessment

Overall Risk: VERY LOW

Phase 1 Changes:

  • Simple syntax cleanup
  • No functional changes
  • Well-covered by existing tests
  • Easy to revert if needed

Why Low Risk:

  • Only removing boilerplate wrapper code
  • Same redirect logic preserved
  • Test coverage exists
  • Small change scope (3 files)

Timeline

Phase 1 (Recommended Now):

  • Implementation: 10 minutes
  • Testing: 10 minutes
  • Code review: 5 minutes
  • Total: 25 minutes

Phase 2 (Future):

  • Bootstrap 5 migration: 4-6 hours (separate epic)
  • View syntax cleanup: 2-3 hours (low priority)

Success Metrics

Phase 1:

  • ✅ 3 unnecessary respond_to blocks removed
  • ✅ All tests pass
  • ✅ No functional regressions
  • ✅ Code is more concise and idiomatic

Long-term:

  • ✅ Application follows Rails 8 conventions
  • ✅ Ready for Bootstrap 5 migration
  • ✅ Minimal legacy code patterns

Notes

  • Good News: The application is already very modern! Most Rails 8 patterns are in place.
  • Legacy JS Cleanup: Already completed (converted to Stimulus)
  • Devise Views: Framework-generated, low value to customize
  • Bootstrap Attributes: Should be addressed during Bootstrap 5 upgrade, not separately
  • Render Syntax: Works fine, cleanup is cosmetic

Related Projects (from CLAUDE.md)

This plan addresses item #4 from CLAUDE.md:
"Scan application for other legacy code that is not idiomatic Rails 8 and create a plan for replacement."

Other ongoing projects:

  • ✅ Convert legacy async JavaScript to Stimulus (COMPLETED)
  • ⏳ Remove local: true from forms (Medium Priority)
  • ⏳ Delete unused JavaScript files (Completed - cable.js is placeholder only)
  • 🔜 Bootstrap 4 → Tailwind/Bootstrap 5 migration (Future)

Recommendation

Proceed with Phase 1 immediately - it's a quick win that improves code quality with minimal risk.

Defer Phase 2 - Bootstrap data attributes should be updated during the planned Bootstrap upgrade, not as a separate effort.
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions