Skip to content

Conversation

@americac
Copy link
Collaborator

  • Fixed 29 previously skipped admin_v2 controller tests that were
    disabled after a Rails upgrade
  • Fixed bugs in form builder, controllers, and models that were causing
    test failures
  • All 687 tests now pass with 0 failures and 0 errors

Changes

Bug Fixes

Form Builder (app/form_builders/admin_v2/form_builder.rb)

  • Fixed input_class and error_message methods to handle nil objects
    using safe navigation, preventing crashes when forms are created
    without a model
  • Enhanced association_select to support callable label_method
    (lambdas/procs) in addition to symbols

Teachers Controller (app/controllers/admin_v2/teachers_controller.rb)

  • Fixed teacher_params to properly permit classroom_ids as an array
    parameter
  • Changed destroy action to use discard for soft delete consistency
    with other user types

SchoolYear Model (app/models/school_year.rb)

  • Added uniqueness validation for year_id scoped to school_id to catch
    duplicates at the model level
  • Changed dependent: :restrict_with_error to dependent:
    :restrict_with_exception so the controller's rescue block properly
    handles deletion restrictions

Portfolio Transactions View
(app/views/admin_v2/portfolio_transactions/show.html.erb)

  • Fixed invalid route helper admin_v2_portfolio_path that doesn't exist

Test Fixes

  • grades_controller_test.rb: Removed unnecessary skips (tests were
    passing)
  • students_controller_test.rb: Fixed assertions to match actual view
    content and enum values
  • teachers_controller_test.rb: Updated for soft delete behavior and
    classroom associations
  • school_years_controller_test.rb: Fixed assertion syntax and test
    expectations
  • portfolio_transactions_controller_test.rb: Removed skips (working
    after form builder fix)
    - base_controller_test.rb: Rewrote sorting tests to use proper HTTP
    integration tests
  • school_year_test.rb: Updated to expect DeleteRestrictionError
    exception

Notes

@AbuMareBear
Copy link
Collaborator

Looks like we worked on the same problem. #1009

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.

3 participants