Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes the “Reject School” capability from the Admin Schools UI by deleting the reject button, its controller action, and related admin feature specs, aligning the admin console behavior with the linked issue.
Changes:
- Removed the “Reject School” button from the admin school show page.
- Removed
Admin::SchoolsController#rejectand the associated admin feature specs. - Removed “reject school” i18n keys from the admin English locale file.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| spec/features/admin/schools_spec.rb | Removes feature expectations and request specs tied to the reject action/link. |
| config/locales/admin/en.yml | Removes i18n strings for reject action labels/flash messages. |
| app/views/admin/schools/show.html.erb | Removes the “Reject School” action link from the admin UI. |
| app/controllers/admin/schools_controller.rb | Removes the reject controller action implementation. |
Comments suppressed due to low confidence (1)
app/controllers/admin/schools_controller.rb:20
- The admin schools routes still define a
patch :rejectmember route (config/routes.rb:18), but this PR removesAdmin::SchoolsController#reject. Hitting that endpoint will now raiseAbstractController::ActionNotFound. Either remove the reject route (and any remaining references/specs) or restore the controller action (even if it just returns 404) to keep routing consistent.
redirect_to admin_school_path(requested_resource)
end
def reopen
service = SchoolVerificationService.new(requested_resource)
if service.reopen
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expect(response.body).to include(I18n.t('administrate.actions.verify_school')) | ||
| end | ||
|
|
||
| it 'includes link to reject school' do | ||
| expect(response.body).to include(I18n.t('administrate.actions.reject_school')) | ||
| end | ||
|
|
||
| it 'does not include a link to search for this school by its ZIP code in the NCES public schools database' do | ||
| expect(response.body).not_to include('Search for this school in the NCES database') | ||
| end |
There was a problem hiding this comment.
PR description/title says the reject button/function/tests were removed, but there are still specs covering SchoolVerificationService#reject (spec/services/school_verification_service_spec.rb) and the service still delegates :reject. If the intent is to remove reject functionality entirely, those should be removed/updated; otherwise consider updating the PR description to clarify this is only removing the admin UI/controller reject behavior.
Test coverage89.57% line coverage reported by SimpleCov. |
Status
What's changed?