Move photos fetching for trips to a separate service

.app_version

@@ -1 +1 @@
-0.19.3
+0.19.4

CHANGELOG.md

@@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+# 0.19.4 - 2024-12-10
+
+### Fixed
+
+- Fixed a bug where the Photoprism photos were not being shown on the trip page.
+- Fixed a bug where the Immich photos were not being shown on the trip page.
+
+### Added
+
+- A link to the Photoprism photos on the trip page if there are any.
+- A `orientation` field in the Api::PhotoSerializer, hence the `GET /api/v1/photos` endpoint now includes the orientation of the photo. Valid values are `portrait` and `landscape`.
+
 # 0.19.3 - 2024-12-06
 
 ### Changed

app/controllers/trips_controller.rb

@@ -15,9 +15,10 @@ class TripsController < ApplicationController
       :country
     ).map { [_1.to_f, _2.to_f, _3.to_s, _4.to_s, _5.to_s, _6.to_s, _7.to_s, _8.to_s] }
 
-    @photos = Rails.cache.fetch("trip_photos_#{@trip.id}", expires_in: 1.day) do
+    @photo_previews = Rails.cache.fetch("trip_photos_#{@trip.id}", expires_in: 1.day) do
-      @trip.photos
+      @trip.photo_previews
     end
+    @photo_sources = @trip.photo_sources
   end
 
   def new

app/helpers/application_helper.rb

@@ -110,27 +110,4 @@ module ApplicationHelper
   def human_date(date)
     date.strftime('%e %B %Y')
   end
-
-  def immich_search_url(base_url, start_date, end_date)
-    query = {
-      takenAfter: "#{start_date.to_date}T00:00:00.000Z",
-      takenBefore: "#{end_date.to_date}T23:59:59.999Z"
-    }
-
-    encoded_query = URI.encode_www_form_component(query.to_json)
-    "#{base_url}/search?query=#{encoded_query}"
-  end
-
-  def photoprism_search_url(base_url, start_date, _end_date)
-    "#{base_url}/library/browse?view=cards&year=#{start_date.year}&month=#{start_date.month}&order=newest&public=true&quality=3"
-  end
-
-  def photo_search_url(source, settings, start_date, end_date)
-    case source
-    when 'immich'
-      immich_search_url(settings['immich_url'], start_date, end_date)
-    when 'photoprism'
-      photoprism_search_url(settings['photoprism_url'], start_date, end_date)
-    end
-  end
 end

app/helpers/trips_helper.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module TripsHelper
+  def immich_search_url(base_url, start_date, end_date)
+    query = {
+      takenAfter: "#{start_date.to_date}T00:00:00.000Z",
+      takenBefore: "#{end_date.to_date}T23:59:59.999Z"
+    }
+
+    encoded_query = URI.encode_www_form_component(query.to_json)
+    "#{base_url}/search?query=#{encoded_query}"
+  end
+
+  def photoprism_search_url(base_url, start_date, _end_date)
+    "#{base_url}/library/browse?view=cards&year=#{start_date.year}&month=#{start_date.month}&order=newest&public=true&quality=3"
+  end
+
+  def photo_search_url(source, settings, start_date, end_date)
+    case source
+    when 'immich'
+      immich_search_url(settings['immich_url'], start_date, end_date)
+    when 'photoprism'
+      photoprism_search_url(settings['photoprism_url'], start_date, end_date)
+    end
+  end
+end

app/models/trip.rb

@@ -17,20 +17,27 @@ class Trip < ApplicationRecord
     points.pluck(:country).uniq.compact
   end
 
-  def photos
+  def photo_previews
-    return [] unless can_fetch_photos?
+    @photo_previews ||= select_dominant_orientation(photos).sample(12)
-
-    filtered_photos.sample(12)
-                         .sort_by { |photo| photo['localDateTime'] }
-                         .map { |asset| photo_thumbnail(asset) }
   end
 
-  def photos_sources
+  def photo_sources
-    filtered_photos.map { _1[:source] }.uniq
+    @photo_sources ||= photos.map { _1[:source] }.uniq
   end
 
   private
 
+  def photos
+    @photos ||= Trips::Photos.new(self, user).call
+  end
+
+  def select_dominant_orientation(photos)
+    vertical_photos = photos.select { |photo| photo[:orientation] == 'portrait' }
+    horizontal_photos = photos.select { |photo| photo[:orientation] == 'landscape' }
+
+    vertical_photos.count > horizontal_photos.count ? vertical_photos : horizontal_photos
+  end
+
   def calculate_distance
     distance = 0
 
@@ -44,32 +51,4 @@ class Trip < ApplicationRecord
 
     self.distance = distance.round
   end
-
-  def can_fetch_photos?
-    user.immich_integration_configured? || user.photoprism_integration_configured?
-  end
-
-  def filtered_photos
-    return @filtered_photos if defined?(@filtered_photos)
-
-    photos = Photos::Search.new(
-      user,
-      start_date: started_at.to_date.to_s,
-      end_date: ended_at.to_date.to_s
-    ).call
-
-    @filtered_photos = select_dominant_orientation(photos)
-  end
-
-  def select_dominant_orientation(photos)
-    vertical_photos = photos.select { |photo| photo[:orientation] == 'portrait' }
-    horizontal_photos = photos.select { |photo| photo[:orientation] == 'landscape' }
-
-    vertical_photos.count > horizontal_photos.count ? vertical_photos : horizontal_photos
-  end
-
-  def photo_thumbnail(asset)
-    { url: "/api/v1/photos/#{asset[:id]}/thumbnail.jpg?api_key=#{user.api_key}&source=#{asset[:source]}" }
-  end
 end
-

app/services/immich/request_photos.rb

@@ -37,15 +37,7 @@ class Immich::RequestPhotos
 
       items = response.dig('assets', 'items')
 
-      if items.blank?
+      break if items.blank?
-        Rails.logger.debug('==== IMMICH RESPONSE WITH NO ITEMS ====')
-        Rails.logger.debug("START_DATE: #{start_date}")
-        Rails.logger.debug("END_DATE: #{end_date}")
-        Rails.logger.debug(response)
-        Rails.logger.debug('==== IMMICH RESPONSE WITH NO ITEMS ====')
-
-        break
-      end
 
       data << items
 

app/services/trips/photos.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+class Trips::Photos
+  def initialize(trip, user)
+    @trip = trip
+    @user = user
+  end
+
+  def call
+    return [] unless can_fetch_photos?
+
+    photos
+  end
+
+  private
+
+  attr_reader :trip, :user
+
+  def can_fetch_photos?
+    user.immich_integration_configured? || user.photoprism_integration_configured?
+  end
+
+  def photos
+    return @photos if defined?(@photos)
+
+    photos = Photos::Search.new(
+      user,
+      start_date: trip.started_at.to_date.to_s,
+      end_date: trip.ended_at.to_date.to_s
+    ).call
+
+    @photos = photos.map { |photo| photo_thumbnail(photo) }
+  end
+
+  def photo_thumbnail(asset)
+    {
+      id: asset[:id],
+      url: "/api/v1/photos/#{asset[:id]}/thumbnail.jpg?api_key=#{user.api_key}&source=#{asset[:source]}",
+      source: asset[:source],
+      orientation: asset[:orientation]
+    }
+  end
+end

app/views/trips/show.html.erb

@@ -36,8 +36,8 @@
         </div>
 
         <!-- Photos Grid Section -->
-        <% if @photos.any? %>
+        <% if @photo_previews.any? %>
-          <% @photos.each_slice(4) do |slice| %>
+          <% @photo_previews.each_slice(4) do |slice| %>
             <div class="h-32 flex gap-4 mt-4 justify-center">
               <% slice.each do |photo| %>
                 <div class="flex-1 h-full overflow-hidden rounded-lg transition-transform duration-300 hover:scale-105 hover:shadow-lg">
@@ -52,9 +52,9 @@
           <% end %>
         <% end %>
 
-        <% if @trip.photos_sources.any? %>
+        <% if @photo_sources.any? %>
           <div class="text-center mt-6">
-            <% @trip.photos_sources.each do |source| %>
+            <% @photo_sources.each do |source| %>
               <%= link_to "More photos on #{source}", photo_search_url(source, current_user.settings, @trip.started_at, @trip.ended_at), class: "btn btn-primary mt-2", target: '_blank' %>
             <% end %>
           </div>

spec/models/trip_spec.rb

@@ -41,7 +41,7 @@ RSpec.describe Trip, type: :model do
     end
   end
 
-  describe '#photos' do
+  describe '#photo_previews' do
     let(:photo_data) do
       [
         {
@@ -80,8 +80,18 @@ RSpec.describe Trip, type: :model do
     let(:trip) { create(:trip, user:) }
     let(:expected_photos) do
       [
-        { url: "/api/v1/photos/456/thumbnail.jpg?api_key=#{user.api_key}" },
+        {
-        { url: "/api/v1/photos/789/thumbnail.jpg?api_key=#{user.api_key}" }
+          id: '456',
+          url: "/api/v1/photos/456/thumbnail.jpg?api_key=#{user.api_key}&source=immich",
+          source: 'immich',
+          orientation: 'portrait'
+        },
+        {
+          id: '789',
+          url: "/api/v1/photos/789/thumbnail.jpg?api_key=#{user.api_key}&source=immich",
+          source: 'immich',
+          orientation: 'portrait'
+        }
       ]
     end
 
@@ -93,7 +103,7 @@ RSpec.describe Trip, type: :model do
       let(:settings) { {} }
 
       it 'returns an empty array' do
-        expect(trip.photos).to eq([])
+        expect(trip.photo_previews).to eq([])
       end
     end
 
@@ -106,7 +116,7 @@ RSpec.describe Trip, type: :model do
       end
 
       it 'returns the photos' do
-        expect(trip.photos).to eq(expected_photos)
+        expect(trip.photo_previews).to eq(expected_photos)
       end
     end
   end

spec/requests/trips_spec.rb

@@ -25,7 +25,7 @@ RSpec.describe '/trips', type: :request do
     stub_request(:any, 'https://api.github.com/repos/Freika/dawarich/tags')
       .to_return(status: 200, body: '[{"name": "1.0.0"}]', headers: {})
 
-    allow_any_instance_of(Trip).to receive(:photos).and_return([])
+    allow_any_instance_of(Trip).to receive(:photo_previews).and_return([])
 
     sign_in user
   end

spec/serializers/api/photo_serializer_spec.rb

@@ -73,6 +73,7 @@ RSpec.describe Api::PhotoSerializer do
           state: 'Berlin',
           country: 'Germany',
           type: 'image',
+          orientation: 'portrait',
           source: 'immich'
         )
       end
@@ -152,6 +153,7 @@ RSpec.describe Api::PhotoSerializer do
           state: 'Unknown',
           country: 'zz',
           type: 'image',
+          orientation: 'landscape',
           source: 'photoprism'
         )
       end

spec/services/photos/search_spec.rb

@@ -74,7 +74,8 @@ RSpec.describe Photos::Search do
           state: nil,
           country: nil,
           type: 'image',
-          source: 'immich'
+          source: 'immich',
+          orientation: 'landscape'
         }
       end
       let(:serialized_photoprism) do
@@ -88,7 +89,8 @@ RSpec.describe Photos::Search do
           state: nil,
           country: nil,
           type: 'image',
-          source: 'photoprism'
+          source: 'photoprism',
+          orientation: 'landscape'
         }
       end
 

spec/services/trips/photos_spec.rb

@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Trips::Photos do
+  let(:user) { instance_double('User') }
+  let(:trip) { instance_double('Trip', started_at: Date.new(2024, 1, 1), ended_at: Date.new(2024, 1, 7)) }
+  let(:service) { described_class.new(trip, user) }
+
+  describe '#call' do
+    context 'when user has no photo integrations configured' do
+      before do
+        allow(user).to receive(:immich_integration_configured?).and_return(false)
+        allow(user).to receive(:photoprism_integration_configured?).and_return(false)
+      end
+
+      it 'returns an empty array' do
+        expect(service.call).to eq([])
+      end
+    end
+
+    context 'when user has photo integrations configured' do
+      let(:photo_search) { instance_double('Photos::Search') }
+      let(:raw_photos) do
+        [
+          {
+            id: 1,
+            url: '/api/v1/photos/1/thumbnail.jpg?api_key=test-api-key&source=immich',
+            source: 'immich',
+            orientation: 'landscape'
+          },
+          {
+            id: 2,
+            url: '/api/v1/photos/2/thumbnail.jpg?api_key=test-api-key&source=photoprism',
+            source: 'photoprism',
+            orientation: 'portrait'
+          }
+        ]
+      end
+
+      before do
+        allow(user).to receive(:immich_integration_configured?).and_return(true)
+        allow(user).to receive(:photoprism_integration_configured?).and_return(false)
+        allow(user).to receive(:api_key).and_return('test-api-key')
+
+        allow(Photos::Search).to receive(:new)
+          .with(user, start_date: '2024-01-01', end_date: '2024-01-07')
+          .and_return(photo_search)
+        allow(photo_search).to receive(:call).and_return(raw_photos)
+      end
+
+      it 'returns formatted photo thumbnails' do
+        expected_result = [
+          {
+            id: 1,
+            url: '/api/v1/photos/1/thumbnail.jpg?api_key=test-api-key&source=immich',
+            source: 'immich',
+            orientation: 'landscape'
+          },
+          {
+            id: 2,
+            url: '/api/v1/photos/2/thumbnail.jpg?api_key=test-api-key&source=photoprism',
+            source: 'photoprism',
+            orientation: 'portrait'
+          }
+        ]
+
+        expect(service.call).to eq(expected_result)
+      end
+    end
+  end
+end

spec/swagger/api/v1/photos_controller_spec.rb

@@ -111,6 +111,7 @@ RSpec.describe 'Api::V1::PhotosController', type: :request do
                    state: { type: :string },
                    country: { type: :string },
                    type: { type: :string },
+                   orientation: { type: :string },
                    source: { type: :string }
                  },
                  required: %w[id latitude longitude localDateTime originalFileName city state country type source]
@@ -143,7 +144,8 @@ RSpec.describe 'Api::V1::PhotosController', type: :request do
                  state: { type: :string },
                  country: { type: :string },
                  type: { type: :string },
-                 source: { type: :string }
+                 source: { type: :string },
+                 orientation: { type: :string, enum: %w[portrait landscape] }
                }
 
         let(:id) { '7fe486e3-c3ba-4b54-bbf9-1281b39ed15c' }

swagger/v1/swagger.yaml

@@ -366,6 +366,8 @@ paths:
                       type: string
                     type:
                       type: string
+                    orientation:
+                      type: string
                     source:
                       type: string
                   required:
@@ -431,6 +433,11 @@ paths:
                     type: string
                   source:
                     type: string
+                  orientation:
+                    type: string
+                    enum:
+                    - portrait
+                    - landscape
         '404':
           description: photo not found
   "/api/v1/points":