diff --git a/.rubocop.yml b/.rubocop.yml index a96bb10..65750c4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -47,6 +47,11 @@ Naming/VariableNumber: EnforcedStyle: snake_case Rails/InverseOf: Enabled: false +Rails/UniqueValidationWithoutIndex: + Description: "We will enforce this via PR review. This gives false positives at the moment." + Details: "https://github.com/rubocop/rubocop-rails/issues/221" + Enabled: true + Severity: warning Style/AccessModifierDeclarations: EnforcedStyle: inline Style/Documentation: diff --git a/app/models/address.rb b/app/models/address.rb index ee165c4..7bfc420 100644 --- a/app/models/address.rb +++ b/app/models/address.rb @@ -6,6 +6,8 @@ class Address < ApplicationRecord has_many :ride_destinations, class_name: "Ride", foreign_key: "to_address_id", dependent: nil, inverse_of: :to_address validates :line_1, :city, :state, :zip_code, :place_id, :latitude, :longitude, presence: true + validates :place_id, uniqueness: true + validates :zip_code, uniqueness: { scope: %i[line_1 line_2] } geocoded_by :full_address do |obj, results| if (geo = results.first) @@ -17,7 +19,7 @@ class Address < ApplicationRecord before_validation :geocode, if: ->(obj) { - obj.full_address.present? && %i[line_1 city state zip_code place_id latitude + obj.full_address.present? && %i[line_1 line_2 city state zip_code place_id latitude longitude].any? do obj.send("#{_1}_changed?") end diff --git a/app/models/driver.rb b/app/models/driver.rb index 9c00472..145f47a 100644 --- a/app/models/driver.rb +++ b/app/models/driver.rb @@ -7,7 +7,11 @@ class Driver < ApplicationRecord class_name: "DriverAddress", dependent: :destroy, inverse_of: :driver - has_one :current_address, through: :current_driver_address, source: :driver, dependent: :destroy + has_one :current_address, through: :current_driver_address, source: :address, dependent: :destroy validates :first_name, :last_name, presence: true + + def origin_place_id + current_address.place_id + end end diff --git a/app/models/ride.rb b/app/models/ride.rb index eea732c..fda9e66 100644 --- a/app/models/ride.rb +++ b/app/models/ride.rb @@ -5,7 +5,7 @@ class Ride < ApplicationRecord belongs_to :from_address, class_name: "Address" belongs_to :to_address, class_name: "Address" - validates :duration, :distance, :commute_duration, :amount_cents, presence: true + validates :duration, :distance, :commute_duration, :amount_cents, presence: true, on: :update, if: :should_validate? monetize :amount_cents, as: :amount, allow_nil: false, @@ -16,4 +16,21 @@ class Ride < ApplicationRecord scope :by_address, ->(address_id) { where(from_address_id: address_id).or(where(to_address_id: address_id)) } + scope :selectable, -> { + includes(:from_address, :to_address) + .select(:id, :from_address_id, :to_address_id) + .where(driver_id: nil, duration: nil, distance: nil, commute_duration: nil, amount_cents: 0) + } + + def origin_place_id + from_address.place_id + end + + def destination_place_id + to_address.place_id + end + + private def should_validate? + duration.present? && distance.present? && commute_duration.present? && amount_cents.present? + end end diff --git a/db/migrate/20240702182238_create_addresses.rb b/db/migrate/20240702182238_create_addresses.rb index c0c110a..9366954 100644 --- a/db/migrate/20240702182238_create_addresses.rb +++ b/db/migrate/20240702182238_create_addresses.rb @@ -10,10 +10,12 @@ def change t.string :zip_code, index: true, null: false t.float :latitude, null: false t.float :longitude, null: false - t.string :place_id, null: true, index: true + t.string :place_id, null: false t.timestamps end add_index :addresses, %i[city state] + add_index :addresses, %i[line_1 line_2 zip_code], unique: true + add_index :addresses, :place_id, unique: true end end diff --git a/db/migrate/20240702193305_create_rides.rb b/db/migrate/20240702193305_create_rides.rb index 78a8251..7c68f1d 100644 --- a/db/migrate/20240702193305_create_rides.rb +++ b/db/migrate/20240702193305_create_rides.rb @@ -1,9 +1,9 @@ class CreateRides < ActiveRecord::Migration[7.1] def change create_table :rides do |t| - t.float :duration, index: true, null: false - t.float :distance, index: true, null: false - t.float :commute_duration, index: true, null: false + t.float :duration, index: true + t.float :distance, index: true + t.float :commute_duration, index: true t.monetize :amount t.references :driver, null: true, index: true t.references :from_address, foreign_key: {to_table: :addresses}, index: true, null: false diff --git a/db/schema.rb b/db/schema.rb index c9ad6d7..4b02b60 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -22,11 +22,12 @@ t.string "zip_code", null: false t.float "latitude", null: false t.float "longitude", null: false - t.string "place_id" + t.string "place_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["city", "state"], name: "index_addresses_on_city_and_state" - t.index ["place_id"], name: "index_addresses_on_place_id" + t.index ["line_1", "line_2", "zip_code"], name: "index_addresses_on_line_1_and_line_2_and_zip_code", unique: true + t.index ["place_id"], name: "index_addresses_on_place_id", unique: true t.index ["state"], name: "index_addresses_on_state" t.index ["zip_code"], name: "index_addresses_on_zip_code" end @@ -50,9 +51,9 @@ end create_table "rides", force: :cascade do |t| - t.float "duration", null: false - t.float "distance", null: false - t.float "commute_duration", null: false + t.float "duration" + t.float "distance" + t.float "commute_duration" t.integer "amount_cents", default: 0, null: false t.string "amount_currency", default: "USD", null: false t.bigint "driver_id" diff --git a/lib/base_command.rb b/lib/base_command.rb new file mode 100644 index 0000000..7842910 --- /dev/null +++ b/lib/base_command.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class BaseCommand + def self.call(**args) + new.call(**args) + end + + def call(**args) + raise NotImplementedError, "Must define #call method" + end +end diff --git a/lib/client/request.rb b/lib/client/request.rb new file mode 100644 index 0000000..cb4cb72 --- /dev/null +++ b/lib/client/request.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Client + class Request + CONNECTION = Faraday + private_constant :CONNECTION + + def self.connection(url:, params: {}, headers: {}) + new(url, params, headers) + end + + def post(url, body, headers = nil) + connection.post(url, body.to_json, headers) + end + + attr_reader :connection + private :connection + + private def initialize(url, params, headers) + @connection = CONNECTION.new(url, params:, headers:) + end + end +end diff --git a/lib/rides/commands/compute_amount.rb b/lib/rides/commands/compute_amount.rb new file mode 100644 index 0000000..19cf067 --- /dev/null +++ b/lib/rides/commands/compute_amount.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module Rides + module Commands + class ComputeAmount < BaseCommand + BASE_PAY_AMOUNT = Money.new(12_00) + MILEAGE_BONUS_AMOUNT = Money.new(1_50) + MILEAGE_BONUS_CLIFF = 5.0 + DURATION_BONUS_AMOUNT = Money.new(0.7) + DURATION_BONUS_CLIFF = 0.25 + + def call(ride:) + distance_bonus_amount = compute_distance_bonus(ride.distance_meters) + duration_bonus_amount = compute_duration_bonus(ride.duration) + + BASE_PAY_AMOUNT + distance_bonus_amount + duration_bonus_amount + end + + private def compute_distance_bonus(distance_meters) + distance_in_miles = convert_distance_to_miles(distance_meters) + + amount = distance_in_miles > MILEAGE_BONUS_CLIFF ? MILEAGE_BONUS_AMOUNT * distance_in_miles : 0 + Money.new(amount) + end + + private def compute_duration_bonus(duration) + duration_in_hours = convert_duration_to_hours(duration) + + amount = duration_in_hours > DURATION_BONUS_CLIFF ? DURATION_BONUS_AMOUNT * duration_in_hours : 0 + Money.new(amount) + end + + private def convert_distance_to_miles(distance_meters) + # 1 mile = 1609.34 meters + distance_meters / 1609.34 + end + + private def convert_duration_to_hours(duration) + # Since there are 3,600 seconds in one hour, that's the conversion ratio used in the formula + duration.to_f / 3600 + end + end + end +end diff --git a/lib/rides/commands/get_commute_duration.rb b/lib/rides/commands/get_commute_duration.rb new file mode 100644 index 0000000..f37ea78 --- /dev/null +++ b/lib/rides/commands/get_commute_duration.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Rides + module Commands + # Computes the duration of the commute for the ride. + # This is used in the ranking for the rides that the driver will + # choose from. + # Returns a list of objects where the origin is the driver's current address(home) + class GetCommuteDuration < BaseCommand + def call(rides:, driver:) + commute_rides = convert_rides(rides, driver) + GetRoutesData.call(rides: commute_rides) + end + + # Converts the Driver's current home address and + # the Ride#from_address into structs that can be used to + # obtain the route information + private def convert_rides(rides, driver) + rides.each_with_object([]) do |ride, acc| + acc << OpenStruct.new(origin_place_id: driver.origin_place_id, destination_place_id: ride.origin_place_id) + end + end + end + end +end diff --git a/lib/rides/commands/get_routes_data.rb b/lib/rides/commands/get_routes_data.rb new file mode 100644 index 0000000..5058151 --- /dev/null +++ b/lib/rides/commands/get_routes_data.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module Rides + module Commands + # Makes a request to the Google API to obtain the route information + class GetRoutesData < BaseCommand + DIRECTIONS_API_URL = "https://routes.googleapis.com/distanceMatrix/v2:computeRouteMatrix" + DEFAULT_HEADERS = { + "X-Goog-FieldMask" => "originIndex,destinationIndex,status,condition,distanceMeters,duration", + "X-goog-api-key" => ENV["GOOGLE_API_KEY"], + "Content-Type" => "application/json" + }.freeze + DEFAULT_REQUEST_PARAMS = { routingPreference: "TRAFFIC_AWARE", travelMode: "DRIVE" }.freeze + + def call(rides:) + data = get_direction_data_for_ride(rides) + results(data, rides) + end + + # Returns a list of objects, with attributes of + # @param[:distance_meters] = Integer + # @param[:duration] = String, e.g., "577s" + # Duration is in seconds + private def results(data, rides) + # The response keeps the array positioning on the return. Since we're getting a matrix + # of routes back, we only want the ones where we explicitly have a 'Ride'. This means that + # we want the computations where the indicies match. + data = data.select { _1[:originIndex] == _1[:destinationIndex] } + data = transform_keys!(data) + + data.map.with_index { OpenStruct.new(ride: rides[_2], **_1) } + end + + private def connection + @connection ||= Client::Request.connection( + url: DIRECTIONS_API_URL, + headers: DEFAULT_HEADERS + ) + end + + private def get_direction_data_for_ride(rides) + body = build_request_body(rides) + + response = connection.post( + DIRECTIONS_API_URL, + body.merge(routingPreference: "TRAFFIC_AWARE", travelMode: "DRIVE") + ) + + JSON.parse(response.body, symbolize_names: true) + end + + private def build_request_body(rides) + rides.each_with_object({}) do |ride, acc| + acc[:origins] ||= [] + acc[:destinations] ||= [] + + acc[:origins] << { waypoint: { placeId: ride.origin_place_id } } + acc[:destinations] << { waypoint: { placeId: ride.destination_place_id } } + end + end + + private def transform_keys!(data) + data.map { |d| d.transform_keys { |k| k.to_s.underscore.to_sym } } + end + end + end +end diff --git a/spec/factories/drivers.rb b/spec/factories/drivers.rb index 34d7220..3cb805c 100644 --- a/spec/factories/drivers.rb +++ b/spec/factories/drivers.rb @@ -4,5 +4,7 @@ factory :driver do first_name { Faker::Name.first_name } last_name { Faker::Name.last_name } + + association :current_address, factory: :address end end diff --git a/spec/factories/rides.rb b/spec/factories/rides.rb index 554a575..93c5835 100644 --- a/spec/factories/rides.rb +++ b/spec/factories/rides.rb @@ -2,10 +2,10 @@ FactoryBot.define do factory :ride do - duration { 2.3 } - commute_duration { 1.0 } - distance { 30.1 } - amount_cents { 1200 } + duration { nil } + commute_duration { nil } + distance { nil } + amount_cents { 0 } driver { nil } diff --git a/spec/lib/rides/commands/compute_amount_spec.rb b/spec/lib/rides/commands/compute_amount_spec.rb new file mode 100644 index 0000000..f406488 --- /dev/null +++ b/spec/lib/rides/commands/compute_amount_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +RSpec.describe Rides::Commands::ComputeAmount do + let(:duration) { "577s" } + let(:distance_meters) { 3105 } + subject { described_class.call(ride:) } + + describe "CSU to The Still" do + let(:ride) { OpenStruct.new(duration:, distance_meters:) } + it "computes the amount" do + result = subject + expect(result.format).to eq("$12.00") + end + end + + describe "Fort Collins to Denver" do + let(:distance_meters) { 100_262.1 } + let(:duration) { "3600s" } + let(:ride) { OpenStruct.new(duration:, distance_meters:) } + it "computes the amount" do + result = subject + expect(result.format).to eq("$105.46") + end + end +end diff --git a/spec/lib/rides/commands/get_commute_duration_spec.rb b/spec/lib/rides/commands/get_commute_duration_spec.rb new file mode 100644 index 0000000..5e26dfb --- /dev/null +++ b/spec/lib/rides/commands/get_commute_duration_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +RSpec.describe Rides::Commands::GetCommuteDuration do + it "gets data for an address" do + VCR.use_cassette("first_commute") do + from_address = create( + :address, :with_out_place_id, line_1: "4705 Weitzel Street", city: "Timnath", state: "CO", + zip_code: "80547" + ) + to_address = create( + :address, :with_out_place_id, line_1: "151 N College Ave", city: "Fort Collins", state: "CO", + zip_code: "80524" + ) + create_list(:ride, 2, from_address:, to_address:) + driver = create(:driver, current_address: from_address) + rides = Ride.selectable + data = described_class.call(rides:, driver:) + + expect(data.length).to eq(2) + expect(data.all? { _1.duration == "577s" }) + end + end +end diff --git a/spec/lib/rides/commands/get_routes_data_spec.rb b/spec/lib/rides/commands/get_routes_data_spec.rb new file mode 100644 index 0000000..aba66cf --- /dev/null +++ b/spec/lib/rides/commands/get_routes_data_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +RSpec.describe Rides::Commands::GetRoutesData do + it "gets data for an address" do + VCR.use_cassette("first_ride_directions") do + from_address = create( + :address, :with_out_place_id, line_1: "711 Oval Drive", city: "Fort Collins", state: "CO", + zip_code: "80521" + ) + to_address = create( + :address, :with_out_place_id, line_1: "151 N College Ave", city: "Fort Collins", state: "CO", + zip_code: "80524" + ) + create_list(:ride, 2, from_address:, to_address:) + rides = Ride.selectable + data = described_class.call(rides:) + + expect(data.length).to eq(2) + expect(data.all? { _1.distance_in_meters == 3105 && _1.duration == "577s" }) + end + end +end diff --git a/spec/models/ride_spec.rb b/spec/models/ride_spec.rb index 50a7dd6..7d42a82 100644 --- a/spec/models/ride_spec.rb +++ b/spec/models/ride_spec.rb @@ -11,10 +11,6 @@ end describe "attributes" do - it { is_expected.to validate_presence_of(:duration) } - it { is_expected.to validate_presence_of(:distance) } - it { is_expected.to validate_presence_of(:commute_duration) } - it { is_expected.to validate_presence_of(:amount_cents) } it { is_expected.to monetize(:amount_cents).as(:amount) } it { is_expected.to validate_numericality_of(:amount_cents) } end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 31cf0eb..0a46939 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -13,9 +13,10 @@ VCR.configure do |config| config.cassette_library_dir = "spec/cassettes" config.hook_into :webmock - config.default_cassette_options = { - match_requests_on: %i[method uri] - } + config.debug_logger = File.open("log/vcr_debug.log", "w") + # config.default_cassette_options = { + # match_requests_on: %i[method uri] + # } end # Add additional requires below this line. Rails is not loaded until this point!