Skip to content

Commit

Permalink
Add rake task for migrating deleted adoptions
Browse files Browse the repository at this point in the history
Occasionally adoptions are deleted when invalid data is corrected within
the source and reimported. This task will automatically move adoptions
that were close to other, unadopted, things as it is likely that the
unadopted thing is actually the one that the user adopted.
  • Loading branch information
jszwedko committed Nov 8, 2017
1 parent 6caf7cf commit 45b1156
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 0 deletions.
3 changes: 3 additions & 0 deletions .rubocop.yml
Expand Up @@ -24,10 +24,12 @@ Metrics/AbcSize:
- 'app/controllers/sessions_controller.rb'
- 'app/controllers/users_controller.rb'
- 'lib/thing_importer.rb'
- 'lib/adoption_mover.rb'

Metrics/BlockLength:
Exclude:
- 'config/initializers/*'
- 'lib/adoption_mover.rb' # removing heredocs would decrease readibility
ExcludedMethods: ['test']

Metrics/BlockNesting:
Expand All @@ -43,6 +45,7 @@ Metrics/MethodLength:
Exclude:
- 'db/migrate/*.rb'
- 'lib/thing_importer.rb' # removing heredocs would decrease readibility
- 'lib/adoption_mover.rb' # removing heredocs would decrease readibility

Metrics/ParameterLists:
Max: 4
Expand Down
69 changes: 69 additions & 0 deletions lib/adoption_mover.rb
@@ -0,0 +1,69 @@
# class for rectifying adoptions of invalid data

class AdoptionMover
class << self
# Move adoptions deleted later than `from` to close by unadopted things
# within `maximum_movement_in_feet` away
#
# Returns a hash of {to_id: from_id}
def move_close_deleted_adoptions(from, maximum_movement_in_feet)
moved_adoptions = {}

Thing.transaction do
records = ActiveRecord::Base.connection.execute <<-SQL.strip_heredoc
WITH
deleted_adopted_things AS (
SELECT *
FROM things
WHERE
user_id IS NOT NULL
-- only recently deleted things
AND (deleted_at > #{ActiveRecord::Base.sanitize(from)})
)
SELECT
deleted_adopted_things.id AS deleted_adopted_thing_id,
closest_unadopted_thing.id AS closest_unadopted_thing_id
FROM
deleted_adopted_things
LEFT JOIN LATERAL (
SELECT *,
-- Haversine formula
-- https://developers.google.com/maps/solutions/store-locator/clothing-store-locator#finding-locations-with-mysql
(
3959
* ACOS(
COS(RADIANS(deleted_adopted_things.lat))
* COS(RADIANS(unadopted_things.lat))
* COS(RADIANS(unadopted_things.lng) - RADIANS(deleted_adopted_things.lng))
+ SIN(RADIANS(deleted_adopted_things.lat))
* SIN(RADIANS(unadopted_things.lat))
)
) * 5280 AS distance_in_feet
FROM things AS unadopted_things
WHERE deleted_at IS NULL AND user_id IS NULL
ORDER BY distance_in_feet
LIMIT 1
) AS closest_unadopted_thing ON 1=1
WHERE distance_in_feet < #{ActiveRecord::Base.sanitize(maximum_movement_in_feet)}
ORDER BY distance_in_feet
;
SQL

records.each do |record|
deleted_adopted_thing = Thing.unscoped.find(record['deleted_adopted_thing_id'])
closeby_unadopted_thing = Thing.find(record['closest_unadopted_thing_id'])

closeby_unadopted_thing.update_attributes!(
user_id: deleted_adopted_thing.user_id,
adopted_name: deleted_adopted_thing.adopted_name,
)
deleted_adopted_thing.update_attributes!(user_id: nil)

moved_adoptions[deleted_adopted_thing.id] = closeby_unadopted_thing.id
end
end

moved_adoptions
end
end
end
21 changes: 21 additions & 0 deletions lib/tasks/data.rake
Expand Up @@ -6,4 +6,25 @@ namespace :data do

ThingImporter.load('https://data.sfgov.org/api/views/jtgq-b7c5/rows.csv?accessType=DOWNLOAD')
end

# move adoptions to closeby things
# useful for rectifying adoptions of inconsistencies in the dataset (things
# that are removed during scheduled import)
task move_close_deleted_adoptions: :environment do
require 'adoption_mover'

ENV['ADOPTION_DELETION_FROM'] || raise('$ADOPTION_DELETION_FROM required')
ENV['MAXIMUM_MOVEMENT_IN_FEET'] || raise('$MAXIMUM_MOVEMENT_IN_FEET required')

adoption_deletion_from = Time.zone.parse(ENV['ADOPTION_DELETION_FROM'])

moved_adoptions = AdoptionMover.move_close_deleted_adoptions(adoption_deletion_from, ENV['MAXIMUM_MOVEMENT_IN_FEET'])

CSV($stdout) do |csv|
csv << %w[from to]
moved_adoptions.each do |from, to|
csv << [from, to]
end
end
end
end
71 changes: 71 additions & 0 deletions test/lib/adopted_mover_test.rb
@@ -0,0 +1,71 @@
require 'test_helper'

require 'adoption_mover'

class AdoptionMoverTest < ActiveSupport::TestCase
test 'moves deleted adoptions to close by unadopted things' do
deleted_adoption_to_be_moved = things(:thing_1).tap do |thing|
thing.update!(user_id: users(:erik).id, adopted_name: 'hello', lat: '37.74092857302200', lng: '-122.422757295129000')
thing.destroy!
end
unadopted_thing_to_be_moved_to = things(:thing_10).tap do |thing|
thing.update!(user_id: nil, lat: '37.74093794334370', lng: '-122.42275720139800')
end

moved_adoptions = AdoptionMover.move_close_deleted_adoptions(Time.zone.parse('1 hour ago'), 5)

assert_equal moved_adoptions[deleted_adoption_to_be_moved.id], unadopted_thing_to_be_moved_to.id

# assert close deleted adoption moved
unadopted_thing_to_be_moved_to.reload
assert_equal unadopted_thing_to_be_moved_to.user_id, deleted_adoption_to_be_moved.user_id
assert_equal unadopted_thing_to_be_moved_to.adopted_name, deleted_adoption_to_be_moved.adopted_name

# original adoption removed
deleted_adoption_to_be_moved.reload
assert_nil deleted_adoption_to_be_moved.user_id
end

test 'does not consider adopted drains when moving deleted adoptions to close by unadopted things' do
deleted_adoption_to_be_moved = things(:thing_1).tap do |thing|
thing.update!(user_id: users(:erik).id, adopted_name: 'hello', lat: '37.74092857302200', lng: '-122.422757295129000')
thing.destroy!
end
adopted_thing_to_ignore = things(:thing_10).tap do |thing|
thing.update!(user_id: users(:dan).id, adopted_name: 'world', lat: '37.74093794334370', lng: '-122.42275720139800')
end

assert_equal AdoptionMover.move_close_deleted_adoptions(Time.zone.parse('1 hour ago'), 5), {}

# original adoption unchanged
deleted_adoption_to_be_moved.reload
assert_equal deleted_adoption_to_be_moved.user_id, users(:erik).id
assert_equal deleted_adoption_to_be_moved.adopted_name, 'hello'

# assert close adoption unchanged
adopted_thing_to_ignore.reload
assert_equal adopted_thing_to_ignore.user_id, users(:dan).id
assert_equal adopted_thing_to_ignore.adopted_name, 'world'
end

test 'does not consider unadopted, far away, drains when moving deleted adoptions to close by unadopted things' do
deleted_adoption_to_be_moved = things(:thing_1).tap do |thing|
thing.update!(user_id: users(:erik).id, adopted_name: 'hello', lat: '37.74092857302200', lng: '-122.422757295129000')
thing.destroy!
end
thing_to_ignore = things(:thing_10).tap do |thing|
thing.update!(user_id: nil, lat: '38.74093794334370', lng: '-122.42275720139800')
end

assert_equal AdoptionMover.move_close_deleted_adoptions(Time.zone.parse('1 hour ago'), 5), {}

# original adoption unchanged
deleted_adoption_to_be_moved.reload
assert_equal deleted_adoption_to_be_moved.user_id, users(:erik).id
assert_equal deleted_adoption_to_be_moved.adopted_name, 'hello'

# assert far unadopted thing unchanged
thing_to_ignore.reload
assert_nil thing_to_ignore.user_id
end
end

0 comments on commit 45b1156

Please sign in to comment.