Skip to content

Commit

Permalink
City Merge was Broken (#735)
Browse files Browse the repository at this point in the history
* keyword argument defaults need to be keyword-ized

* Regression test for city merge bug
  • Loading branch information
Nick Barnwell authored Apr 12, 2018
1 parent 13a8e8d commit e75ff26
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 2 deletions.
2 changes: 1 addition & 1 deletion app/controllers/cities_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def create
# PATCH/PUT /cities/1.json
def update
if params[:approval_action]
approver = CityApprover.new(@city)
approver = CityApprover.new(@city.id)
case params[:approval_action]
when "merge"
approver.merge!(params[:merge_city_id])
Expand Down
2 changes: 1 addition & 1 deletion app/services/city_approver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def merge!(existing_city_id)
existing = City.find(existing_city_id)
approval_gate do
User.where(home_city: @city.id).update_all(home_city_id: existing.id)
self.reject!(false)
self.reject!(mail_user: false)
UserMailer.delay.notify_city_suggestor(@city, :merged)
end
end
Expand Down
38 changes: 38 additions & 0 deletions spec/controllers/cities_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
require 'spec_helper'

describe CitiesController do
before do
[:away_ye_waitlisted, :authenticate_user!, :authorized?].each do |s|
allow(controller).to receive(s).and_return true
end
end

describe '#update' do
let(:unapproved_city) {
city = create(:city, :unapproved)
(1..10).each { |i| create(:user, home_city: city) }
city
}

let(:merge_request) {
{
id: unapproved_city.city_code,
city: {id: unapproved_city.id},
merge_city_id: create(:city).id,
approval_action: "merge"
}
}

it 'should move all users from City A to City B upon merge' do
city_a = unapproved_city
city_a_users = city_a.users
city_b_id = merge_request[:merge_city_id]

patch :update, merge_request
expect(response).to redirect_to(city_path(city_a))
expect(city_a_users.map {
|u| u.reload.home_city_id == city_b_id
}.all?).to eq(true)
end
end
end

0 comments on commit e75ff26

Please sign in to comment.