Refactor

Author: arkadiysudarikov

app/controllers/api/v1/upcoming_rides_controller.rb

@@ -4,13 +4,7 @@
 module Api
   # API::V1 module for version 1 of the API
   module V1
-    # UpcomingRidesController for the API
-    # This controller is responsible for returning upcoming rides for a driver
-    # It includes the UpcomingRides module
-    # It includes the Pagy::Backend module for pagination
-    # It inherits from ApplicationController
-    # It has an index method
-    # It has a private driver_params method
+    # This controller is responsible for returning the upcoming rides for a driver
     class UpcomingRidesController < ApplicationController
       include UpcomingRides
       include Pagy::Backend

app/models/address.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Adress model is used to store the address of the address.
+# Adress model is used to store valid formatted addresses.
 class Address < ApplicationRecord
   has_many :home_addresses, class_name: "Driver", foreign_key: "home_address_id"
   has_many :start_addresses, class_name: "Ride", foreign_key: "start_address_id"

app/models/driver.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Driver model is used to store the driver's name and address.
+# Driver model is used to store the driver's address.
 class Driver < ApplicationRecord
   belongs_to :home_address, class_name: "Address"
 end

app/models/ride.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 # A ride has a start address and a destination address.
+# The destination address must be different from the start address.
 class Ride < ApplicationRecord
   belongs_to :start_address, class_name: "Address"
   belongs_to :destination_address, class_name: "Address"

app/services/google_directions_api_client.rb

@@ -1,12 +1,18 @@
 # frozen_string_literal: true
 
+# DirectionsAPIError is a custom error class that is raised when there is an error fetching
+# directions from the Google Directions API
 class DirectionsAPIError < StandardError; end
 
+# GoogleDirectionsApiClient is a service class that fetches directions from the Google Directions
+# API
 class GoogleDirectionsApiClient
   BASE_URL = "https://maps.googleapis.com/maps/api/directions/json"
 
   METERS_IN_A_MILE = 1609.34
-  SECONDS_IN_A_MINUTE = 60
+  SECONDS_IN_A_MINUTE = 60.0
+
+  GOOGLE_DIRECTIONS_API_ERROR = "Google Directions API Error"
 
   def initialize(api_key)
     @api_key = api_key
@@ -17,7 +23,7 @@ def get_directions(origin, destination)
 
     response = send_request(url)
 
-    check_response(response)
+    check_response!(response)
 
     parse_response(response)
   end
@@ -34,9 +40,9 @@ def send_request(url)
     raise DirectionsAPIError, "Unable to fetch directions. Please try again."
   end
 
-  def check_response(response)
-    raise DirectionsAPIError, "Not OK" unless response["status"] == "OK"
-    raise DirectionsAPIError, "Google Directioons API error" if response["error_message"].present?
+  def check_response!(response)
+    raise DirectionsAPIError, GOOGLE_DIRECTIONS_API_ERROR unless response["status"] == "OK"
+    raise DirectionsAPIError, GOOGLE_DIRECTIONS_API_ERROR if response["error_message"].present?
   end
 
   def parse_response(response)

db/seeds.rb

@@ -18,6 +18,18 @@
 Driver.find_or_create_by!(id: "e76885d9-dc50-4616-830e-cd24beefd7d9",
                           home_address: Address.all.sample)
 
-100.times do
-  Ride.find_or_create_by(start_address: Address.all.sample, destination_address: Address.all.sample)
-end
+Ride.find_or_create_by(start_address: Address.first, destination_address: Address.second)
+Ride.find_or_create_by(start_address: Address.first, destination_address: Address.third)
+Ride.find_or_create_by(start_address: Address.first, destination_address: Address.fourth)
+
+Ride.find_or_create_by(start_address: Address.second, destination_address: Address.first)
+Ride.find_or_create_by(start_address: Address.second, destination_address: Address.third)
+Ride.find_or_create_by(start_address: Address.second, destination_address: Address.fourth)
+
+Ride.find_or_create_by(start_address: Address.third, destination_address: Address.first)
+Ride.find_or_create_by(start_address: Address.third, destination_address: Address.second)
+Ride.find_or_create_by(start_address: Address.third, destination_address: Address.fourth)
+
+Ride.find_or_create_by(start_address: Address.fourth, destination_address: Address.first)
+Ride.find_or_create_by(start_address: Address.fourth, destination_address: Address.second)
+Ride.find_or_create_by(start_address: Address.fourth, destination_address: Address.third)

lib/upcoming_rides.rb

@@ -2,6 +2,10 @@
 
 # Description: This module is used to get the upcoming rides from the database.
 module UpcomingRides
+  # The CACHE_EXPIRATION constant is used to set the cache expiration time for the ride's
+  # directions.
+  CACHE_EXPIRATION = 5.0.minutes
+
   # The upcoming_rides method takes in a driver and returns the upcoming rides from the database.
   # It uses the Ride.where method to get the upcoming rides from the database.
   # It uses the map method to map the rides to a new array with the ride's attributes and additional
@@ -36,7 +40,8 @@ def sort_upcoming_rides(upcoming_rides)
   def get_directions(start_address, destination_address)
     cache_key = "ride/#{start_address.id}-#{destination_address.id}/directions"
 
-    Rails.cache.fetch(cache_key, expires_in: 5.minutes) do # Look to see if can call fetch_all
+    Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRATION) do
+      # Consider dependency injection
       google_directions_api_client = GoogleDirectionsApiClient.new(
         Rails.application.credentials.google_api_key
       )
@@ -50,44 +55,41 @@ def get_directions(start_address, destination_address)
   # ride's attributes and additional attributes such as the driver's home address, the commute
   # distance and duration, the ride distance and duration, the ride earnings, and the score.
   def add_ride_attributes(driver, ride)
-    # Rename data (to ride_data?) - testable
-    data = data(driver.home_address, ride.start_address, ride.destination_address)
+    data = get_data(driver.home_address, ride.start_address, ride.destination_address)
 
     ride.attributes.merge(data)
   end
 
-  # Add unit tests
   def ride_earnings(ride_distance, ride_duration)
     # The ride earnings is how much the driver earns by driving the ride. It takes into account
     # both the amount of time the ride is expected to take and the distance. For the purposes of
     # this exercise, it is calculated as:
     # $12 + $1.50 per mile beyond 5 miles + (ride duration) * $0.70 per minute beyond 15 minutes
 
-    12 + ([ride_distance - 5, 0].max * 1.5) + ([ride_duration - 15, 0].max * 0.7)
+    12.0 + ([ride_distance - 5.0, 0.0].max * 1.5) + ([ride_duration - 15.0, 0.0].max * 0.7)
   end
 
   # The score is a metric that takes into account the ride earnings, the commute duration, and the
   # ride duration. It is calculated as the ride earnings divided by the sum of the commute duration
   # and the ride duration.
-  # Add unit test
   def score(ride_earnings, commute_duration, ride_duration)
     ride_earnings / (commute_duration + ride_duration)
   end
 
-  # The data method takes in the driver's home address, the start address, and the destination
+  # The get_data method takes in the driver's home address, the start address, and the destination
   # address, and returns a hash with the commute distance and duration, the ride distance and
   # duration, the ride earnings, and the score.
   # It uses the get_directions method to get the commute distance and duration and the ride distance
   # and duration, and the ride_earnings method to get the ride earnings.
   # It uses the score method to get the score.
   # It returns the hash with the data.
-  # The data method is used by the add_ride_attributes method to add the ride's attributes and
+  # The get_data method is used by the add_ride_attributes method to add the ride's attributes and
   # additional attributes to the ride.
-  # The data method is also used by the upcoming_rides method to get the upcoming rides from the
+  # The get_data method is also used by the upcoming_rides method to get the upcoming rides from the
   # database.
-  # The data method is also used by the add_ride_attributes method to add the ride's attributes and
-  # additional attributes to the ride.
-  def data(home_address, start_address, destination_address)
+  # The get_data method is also used by the add_ride_attributes method to add the ride's attributes
+  # and additional attributes to the ride.
+  def get_data(home_address, start_address, destination_address)
     commute_distance, commute_duration = get_directions(home_address, start_address)
     ride_distance, ride_duration = get_directions(start_address, destination_address)
     ride_earnings = ride_earnings(ride_distance, ride_duration)

spec/lib/upcoming_rides_spec.rb

@@ -1,12 +1,6 @@
 # frozen_string_literal: true
 
 require "rails_helper"
-require_relative "../../lib/upcoming_rides"
-
-# Create a class to include the UpcomingRides module for testing
-class UpcomingRidesForTesting
-  include UpcomingRides
-end
 
 RSpec.describe UpcomingRides do
   include described_class
@@ -16,6 +10,7 @@ class UpcomingRidesForTesting
     instance_double(Ride, start_address: Faker::Address.street_address,
                           destination_address: Faker::Address.street_address)
   end
+
   let(:headers) do
     {
       "Accept" => "*/*",
@@ -40,11 +35,11 @@ class UpcomingRidesForTesting
   end
 
   describe '#sort_upcoming_rides' do
-    xit 'sorts the upcoming rides by the score in descending order' do
+    it 'sorts the upcoming rides by the score in descending order' do
       # Create some sample rides with scores
-      ride1 = instance_double(Ride, score: 5)
-      ride2 = instance_double(Ride, score: 7)
-      ride3 = instance_double(Ride, score: 3)
+      ride1 = { score: 5 }
+      ride2 = { score: 7 }
+      ride3 = { score: 3 }
 
       # Expect the sort_upcoming_rides method to return the rides sorted by score
       expect(sort_upcoming_rides([ride1, ride2, ride3])).to eq([ride2, ride1, ride3])
@@ -108,18 +103,18 @@ class UpcomingRidesForTesting
 
   describe '#add_ride_attributes' do
     it 'returns a hash with ride attributes and additional attributes' do
-      # Stub the data method to return sample data
-      allow(self).to receive(:data).and_return({
-                                                 home_address: '123 Main St',
-                                                 start_address: '456 Elm St',
-                                                 commute_distance: 5.0,
-                                                 commute_duration: 10.0,
-                                                 destination_address: '789 Oak St',
-                                                 ride_distance: 8.0,
-                                                 ride_duration: 15.0,
-                                                 ride_earnings: 20.0,
-                                                 score: 0.5
-                                               })
+      # Stub the get_data method to return sample data
+      allow(self).to receive(:get_data).and_return({
+                                                     home_address: '123 Main St',
+                                                     start_address: '456 Elm St',
+                                                     commute_distance: 5.0,
+                                                     commute_duration: 10.0,
+                                                     destination_address: '789 Oak St',
+                                                     ride_distance: 8.0,
+                                                     ride_duration: 15.0,
+                                                     ride_earnings: 20.0,
+                                                     score: 0.5
+                                                   })
 
       # Stub the ride attributes to be merged
       allow(ride).to receive(:attributes).and_return({ id: 1, name: 'Ride 1' })
@@ -163,7 +158,7 @@ class UpcomingRidesForTesting
     end
   end
 
-  describe '#data' do
+  describe '#get_data' do
     it 'returns a hash with data including commute distance, duration, ride earnings, and score' do
       home_address = instance_double(Address, id: Faker::Internet.uuid,
                                               address: "1588 E Thompson Blvd")
@@ -172,6 +167,9 @@ class UpcomingRidesForTesting
       destination_address = instance_double(Address, id: Faker::Internet.uuid,
                                                      address: "2112 E Thompson Blvd")
 
+      # expect_any_instance_of(GoogleDirectionsApiClient).to receive(:get_directions).with(home_address.address, start_address.address).and_return([5.0, 10.0])
+      # expect_any_instance_of(GoogleDirectionsApiClient).to receive(:get_directions).with(start_address.address, destination_address.address).and_return([5.0, 10.0])
+
       stub_request(:get, "https://maps.googleapis.com/maps/api/directions/json?origin=1588 E Thompson Blvd&destination=2112 E Thompson Blvd&key=#{Rails.application.credentials.google_api_key}")
         .with(
           headers: {
@@ -232,20 +230,18 @@ class UpcomingRidesForTesting
         "status" : "OK"
         }', headers: {})
 
-      puts data(home_address, start_address, destination_address).inspect
-
-      # Expect the data method to return a hash with the correct data
-      expect(data(home_address, start_address, destination_address)).to eq({
-                                                                             home_address: home_address,
-                                                                             start_address: start_address,
-                                                                             commute_distance: 5.0,
-                                                                             commute_duration: 10.0,
-                                                                             destination_address: destination_address,
-                                                                             ride_distance: 5.0,
-                                                                             ride_duration: 10.0,
-                                                                             ride_earnings: 12.0,
-                                                                             score: 0.6
-                                                                           })
+      # Expect the get_data method to return a hash with the correct data
+      expect(get_data(home_address, start_address, destination_address)).to eq({
+                                                                                 home_address: home_address,
+                                                                                 start_address: start_address,
+                                                                                 commute_distance: 5.0,
+                                                                                 commute_duration: 10.0,
+                                                                                 destination_address: destination_address,
+                                                                                 ride_distance: 5.0,
+                                                                                 ride_duration: 10.0,
+                                                                                 ride_earnings: 12.0,
+                                                                                 score: 0.6
+                                                                               })
     end
   end
 end

spec/models/driver_spec.rb

@@ -9,4 +9,17 @@
       expect(association.macro).to eq(:belongs_to)
     end
   end
+
+  describe "validations" do
+    let(:driver) { described_class.new(home_address: build(:address)) }
+
+    it "is valid with valid attributes" do
+      expect(driver.valid?).to be true
+    end
+
+    it "is not valid without a home_address" do
+      driver.home_address = nil
+      expect(driver.valid?).not_to be true
+    end
+  end
 end

spec/models/ride_spec.rb

@@ -16,4 +16,29 @@
       expect(association.options[:class_name]).to eq("Address")
     end
   end
+
+  describe "validations" do
+    let(:ride) do
+      described_class.new(start_address: build(:address), destination_address: build(:address))
+    end
+
+    it "is valid with valid attributes" do
+      expect(ride.valid?).to be true
+    end
+
+    it "is not valid without a start_address" do
+      ride.start_address = nil
+      expect(ride.valid?).not_to be true
+    end
+
+    it "is not valid without a destination_address" do
+      ride.destination_address = nil
+      expect(ride.valid?).not_to be true
+    end
+
+    it "is not valid if the destination_address is the same as the start_address" do
+      ride.destination_address = ride.start_address
+      expect(ride.valid?).not_to be true
+    end
+  end
 end

spec/services/google_directions_api_client_spec.rb

@@ -4,7 +4,7 @@
 
 RSpec.describe GoogleDirectionsApiClient do
   subject(:google_directions_api_client) do
-    described_class.new(Rails.application.credentials.google_api_key)
+    described_class.new("GOOGLE_DIRECTIONS_API_KEY")
   end
 
   let(:headers) do
@@ -17,7 +17,7 @@
 
   describe '#get_directions' do
     it 'returns the distance and duration in miles and minutes' do
-      stub_request(:get, "https://maps.googleapis.com/maps/api/directions/json?origin=1588%20E%20Thompson%20Blvd&destination=2112%20E%20Thompson%20Blvd&key=#{Rails.application.credentials.google_api_key}")
+      stub_request(:get, "https://maps.googleapis.com/maps/api/directions/json?origin=1588%20E%20Thompson%20Blvd&destination=2112%20E%20Thompson%20Blvd&key=GOOGLE_DIRECTIONS_API_KEY")
         .with(
           headers: headers
         )