Stop asking for a postal code on every check-in

backend/app/services/weather_retriever.rb

@@ -1,73 +1,109 @@
 class WeatherRetriever
   class << self
     def get(date, postal_code)
+      date = date.to_date
       position = Position.find_or_create_by(postal_code: postal_code)
 
-      weather = Weather.find_by(date: date, position_id: position&.id)
-
-      return weather if weather.present?
-
       if position&.latitude.blank? || position&.longitude.blank?
         Rails.logger.warn "No coordinates found for postal_code #{postal_code}: #{position.inspect}"
 
         return
       end
 
-      forecast = get_forecast(date, position)
+      weather = Weather.find_by(date: date, position_id: position.id)
+
+      return weather if weather.present?
+
+      forecast = get_forecast(position)
 
       if forecast.status != 200
         Rails.logger.warn "No forecast found for position #{position.inspect}: response code was #{forecast.status}, headers were #{forecast.headers}, body contained #{forecast.body}"
 
         return
       end
 
-      create_weather(forecast, position.id)
+      day = daily_forecast_on(forecast, date, position)
+
+      if day.blank?
+        Rails.logger.warn "No forecast for #{date} at position #{position.inspect}: the forecast endpoint only covers today onwards"
+
+        return
+      end
+
+      create_weather(day, date, position.id)
     end
 
     private
 
-    def get_forecast(date, position)
+    def get_forecast(position)
       Tomorrowiorb.forecast(
         "#{position.latitude},#{position.longitude}",
         ["1d"],
         "imperial"
       )
     end
 
-    def create_weather(forecast, position_id)
-      today = JSON.parse(forecast.body, symbolize_names: true).dig(:timelines, :daily, 0)
-      the_time = today.dig(:time)
-      today = today.dig(:values)
-      rain_intensity = today.dig(:rainIntensityAvg)
-      sleet_intensity = today.dig(:sleetIntensityAvg)
-      snow_intensity = today.dig(:snowIntensityAvg)
-      icon = get_icon_legacy(today)
-      summary = "General conditions are #{icon}, with an average temperature of #{today[:temperatureAvg]}."
-      Weather.create(
-        date: Date.strptime(the_time, "%Y-%m-%d"),
-        humidity: today.dig(:humidityAvg).round,
+    # The forecast endpoint takes no date: it always answers with a daily timeline
+    # starting at the position's today. Pick out the day that was actually asked
+    # for -- comparing dates in the position's own time zone, since the timeline
+    # stamps each day in UTC -- so that the record we store is keyed by the date
+    # the caller wanted and the cache above can find it again. Days outside the
+    # window (a back-filled check-in, say) have no forecast to store.
+    def daily_forecast_on(forecast, date, position)
+      daily = JSON.parse(forecast.body, symbolize_names: true).dig(:timelines, :daily) || []
+      time_zone = NearestTimeZone.to(position.latitude.to_f, position.longitude.to_f).presence || "UTC"
+
+      daily.find { |day| local_date(day[:time], time_zone) == date }
+    end
+
+    def local_date(time, time_zone)
+      Time.parse(time.to_s).in_time_zone(time_zone).to_date
+    rescue ArgumentError
+      nil
+    end
+
+    def create_weather(day, date, position_id)
+      values = day.dig(:values)
+      rain_intensity = values.dig(:rainIntensityAvg)
+      sleet_intensity = values.dig(:sleetIntensityAvg)
+      snow_intensity = values.dig(:snowIntensityAvg)
+      icon = get_icon_legacy(values)
+      summary = "General conditions are #{icon}, with an average temperature of #{values[:temperatureAvg]}."
+      weather = Weather.new(
+        date: date,
+        humidity: values.dig(:humidityAvg).round,
         icon: icon,
         position_id: position_id,
         precip_intensity: rain_intensity + sleet_intensity + snow_intensity,
-        pressure: today.dig(:pressureSurfaceLevelAvg),
+        pressure: values.dig(:pressureSurfaceLevelAvg),
         summary: summary,
-        temperature_max: today.dig(:temperatureMax),
-        temperature_min: today.dig(:temperatureMin)
+        temperature_max: values.dig(:temperatureMax),
+        temperature_min: values.dig(:temperatureMin)
       )
+
+      return weather if weather.save
+
+      Rails.logger.warn "Could not store weather for #{date} at position #{position_id}: #{weather.errors.full_messages.to_sentence}"
+
+      # Another request cached this day while we were fetching it. Hand back the
+      # record that won rather than an unsaved one, whose nil id callers would
+      # store as "this check-in has no weather".
+      Weather.find_by(date: date, position_id: position_id)
     end
 
-    def get_icon_legacy(today)
+    def get_icon_legacy(values)
       # Our icons do not coverage their full range of weather codes. We could pull in their icons (linked below) on the frontend to expand options
       # This method adapts their weather codes to our existing icons as best as possible
       # Icons and codes found here: https://docs.tomorrow.io/reference/data-layers-weather-codes
       # Icon files here: https://github.com/Tomorrow-IO-API/tomorrow-weather-codes
       # Daily forecast is always daytime weather codes / icons regardless of actual time
-      code = if today["weatherCodeMin"]
-        today["weatherCodeMin"]
-      elsif today["weatherCodeFullDay"]
-        today["weatherCodeFullDay"]
+      # The forecast body is parsed with symbolized names, so these keys are symbols
+      code = if values[:weatherCodeMin]
+        values[:weatherCodeMin]
+      elsif values[:weatherCodeFullDay]
+        values[:weatherCodeFullDay]
       else
-        today["weatherCode"]
+        values[:weatherCode]
       end
 
       case code

backend/spec/controllers/api/v1/weathers_controller_spec.rb

@@ -39,6 +39,17 @@
       it { is_expected.to include(*expected_keys) }
       it { is_expected.not_to include(*not_expected_keys) }
       it { expect(response).to have_http_status(:ok) }
+      it { expect(json_response[:weather][:id]).to eq(weather.id) }
+    end
+
+    describe "index when no weather is available" do
+      let(:json_response) { JSON.parse(response.body, symbolize_names: true) }
+
+      before { expect(WeatherRetriever).to receive(:get).and_return(nil) }
+      before { index_action }
+
+      it { expect(response).to have_http_status(:ok) }
+      it { expect(json_response).to eq(weathers: []) }
     end
   end
 end

backend/spec/services/checkin/creator_spec.rb

@@ -109,18 +109,37 @@
       end
     end
 
-    context "when postal code is set on previous checkin" do
-      let(:weather) { create :weather }
+    context "when a location is set on the previous checkin", :vcr do
+      # The recorded forecast is Minneapolis from 2023-12-05 onwards, so pin "today"
+      # inside that window: the trackings above are only active from today.
+      let!(:date) { Date.parse("2023-12-05") }
+      let(:cassete) { "WeatherRetriever/#{postal_code}" }
       let(:postal_code) { "55403" }
-      let(:position) { Position.create(postal_code: postal_code) }
+      let(:position) { VCR.use_cassette(cassete) { Position.create(postal_code: postal_code) } }
 
-      let!(:previous_checkin) { create :checkin, user_id: user.id, position_id: position.id }
+      let!(:previous_checkin) do
+        create :checkin, user_id: user.id, date: date - 1.day, position_id: position.id
+      end
+
+      subject { VCR.use_cassette(cassete) { Checkin::Creator.new(user.id, date).create! } }
+
+      around { |example| travel_to(date) { example.run } }
 
-      before { expect(WeatherRetriever).to receive(:get).and_return(weather) }
+      before { allow(Tomorrowiorb).to receive(:api_key).and_return("MY_MEGA_TOMORROW_IO_KEY") }
 
-      it "should ask for weather" do
+      it "carries the location over and asks for that day's weather" do
         expect(subject.position.postal_code).to eq(postal_code)
-        expect(subject.weather_id).to eq(weather.id)
+        expect(subject.weather).to be_present
+        expect(subject.weather.date).to eq(date)
+      end
+
+      context "when the weather for that date cannot be retrieved" do
+        before { allow(WeatherRetriever).to receive(:get).and_return(nil) }
+
+        it "still carries the location over" do
+          expect(subject.position.postal_code).to eq(postal_code)
+          expect(subject.weather_id).to be_nil
+        end
       end
     end
   end

backend/spec/services/weather_retriever_spec.rb

@@ -6,7 +6,9 @@
   # stub it here rather than depending on whatever TOMORROW_IO_KEY happens to hold.
   before { allow(Tomorrowiorb).to receive(:api_key).and_return("MY_MEGA_TOMORROW_IO_KEY") }
 
-  let(:date) { Date.parse "2016-01-06" }
+  # The cassette was recorded for Minneapolis (America/Chicago) and carries the
+  # six daily forecasts from 2023-12-05 to 2023-12-10.
+  let(:date) { Date.parse "2023-12-05" }
   let(:cassete) { "#{described_class.name}/#{postal_code}" }
   let(:postal_code) { "55403" }
 
@@ -18,11 +20,53 @@
 
   context "no weather cached" do
     it { expect(perform).to be_a(Weather) }
+    it { expect(perform).to be_persisted }
     it { expect { perform }.to change { Weather.count }.by(1) }
+
+    it "stores the weather under the date that was asked for" do
+      expect(perform.date).to eq(date)
+    end
+
+    it "stores the forecast for that date" do
+      weather = perform
+
+      expect(weather.icon).to eq("cloudy")
+      expect(weather.summary).to eq("General conditions are cloudy, with an average temperature of -0.76.")
+      expect(weather.temperature_min).to eq(-1.83)
+      expect(weather.temperature_max).to eq(0.64)
+      expect(weather.humidity).to eq(81)
+      expect(weather.pressure).to eq(991.51)
+      expect(weather.precip_intensity).to eq(0)
+    end
+
+    it "stores it against the position for the postal code" do
+      expect(perform.position.postal_code).to eq(postal_code)
+    end
+
+    context "when the caller passes a time rather than a date" do
+      let(:date) { DateTime.new(2023, 12, 5, 17, 43, 12) }
+
+      it "stores the weather under that day" do
+        expect(perform.date).to eq(Date.parse("2023-12-05"))
+      end
+    end
+
+    context "for a later day in the forecast window" do
+      let(:date) { Date.parse "2023-12-07" }
+
+      it "stores that day's forecast, not the first day's" do
+        weather = perform
+
+        expect(weather.date).to eq(date)
+        expect(weather.icon).to eq("clear-day")
+        expect(weather.temperature_min).to eq(-1.87)
+        expect(weather.temperature_max).to eq(8.17)
+      end
+    end
   end
 
   context "the weather is already cached" do
-    let(:position) { Position.create(postal_code: postal_code) }
+    let(:position) { VCR.use_cassette(cassete) { Position.create(postal_code: postal_code) } }
     let!(:weather) { create :weather, date: date, position_id: position.id }
 
     before { expect(Weather).not_to receive(:create) }
@@ -32,4 +76,113 @@
     it { expect(perform).to eq(weather) }
     it { expect { perform }.not_to change { Weather.count } }
   end
+
+  # The pre-fix retriever cached every forecast under the date the API answered
+  # with instead of the date it was asked for, so the lookup above never hit, and
+  # the second call tripped the date/position uniqueness validation and returned
+  # an unsaved record whose nil id callers stored as "no weather".
+  context "asking twice for the same date" do
+    it "serves the second call from the cache" do
+      first = perform
+      second = perform
+
+      expect(second).to be_persisted
+      expect(second.id).to eq(first.id)
+      expect(Weather.count).to eq(1)
+    end
+  end
+
+  context "another date is already cached for the position" do
+    let(:other_date) { Date.parse "2023-12-06" }
+
+    it "stores and returns a persisted record for the new date" do
+      cached = VCR.use_cassette(cassete) { described_class.get(other_date, postal_code) }
+      weather = perform
+
+      expect(weather).to be_persisted
+      expect(weather.id).not_to eq(cached.id)
+      expect(weather.date).to eq(date)
+      expect(cached.reload.date).to eq(other_date)
+    end
+  end
+
+  # The forecast endpoint only covers today onwards, so a back-filled check-in has
+  # no forecast to store. Storing whatever the API did answer with would file
+  # another day's weather under this date.
+  context "the requested date is outside the forecast window" do
+    let(:date) { Date.parse "2023-12-01" }
+
+    it { expect(perform).to be_nil }
+    it { expect { perform }.not_to change { Weather.count } }
+  end
+
+  context "the position's day differs from the UTC day" do
+    # Sydney is UTC+11 in December: the daily forecast stamped 2023-12-05T20:00:00Z
+    # is 2023-12-06 there, and that is the date the check-in was filed under.
+    let(:date) { Date.parse "2023-12-06" }
+    let(:postal_code) { "2000" }
+
+    let(:forecast_body) do
+      {
+        timelines: {
+          daily: [
+            {time: "2023-12-05T20:00:00Z", values: {
+              weatherCodeMin: 4000, humidityAvg: 62.4, pressureSurfaceLevelAvg: 1011.2,
+              rainIntensityAvg: 0.5, sleetIntensityAvg: 0, snowIntensityAvg: 0,
+              temperatureAvg: 22.1, temperatureMin: 18.3, temperatureMax: 26.7
+            }}
+          ]
+        }
+      }.to_json
+    end
+
+    before do
+      allow(Geocoder).to receive(:search).with(postal_code).and_return(
+        [double(city: "Sydney", state: "New South Wales", province: nil, country: "Australia",
+          latitude: -33.8688, longitude: 151.2093)]
+      )
+      allow(Tomorrowiorb).to receive(:forecast).and_return(
+        Tomorrowiorb::TomorrowioResponse.new(200, {}, forecast_body)
+      )
+    end
+
+    subject { described_class.get(date, postal_code) }
+
+    it "stores the forecast under the local date" do
+      expect(subject.date).to eq(date)
+      expect(subject.icon).to eq("rain")
+      expect(subject.temperature_max).to eq(26.7)
+    end
+
+    it "serves the next call for that date from the cache" do
+      first = subject
+
+      expect(described_class.get(date, postal_code).id).to eq(first.id)
+    end
+  end
+
+  context "the postal code cannot be geocoded" do
+    let(:postal_code) { "not a place" }
+
+    before { allow(Geocoder).to receive(:search).with(postal_code).and_return([]) }
+    before { expect(Tomorrowiorb).not_to receive(:forecast) }
+
+    it { expect(described_class.get(date, postal_code)).to be_nil }
+  end
+
+  context "the forecast API fails" do
+    before do
+      allow(Tomorrowiorb).to receive(:forecast).and_return(
+        Tomorrowiorb::TomorrowioResponse.new(429, {}, "rate limit exceeded")
+      )
+    end
+
+    it "returns nothing rather than an unsaved record" do
+      position = VCR.use_cassette(cassete) { Position.create(postal_code: postal_code) }
+
+      expect(position).to be_persisted
+      expect(described_class.get(date, postal_code)).to be_nil
+      expect(Weather.count).to eq(0)
+    end
+  end
 end