From d6b88ae9cb5f873fa44bb39904e322d040b088ea Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Tue, 10 Dec 2024 19:31:52 +0100 Subject: [PATCH] Move photos fetching for trips to a separate service --- .app_version | 2 +- CHANGELOG.md | 12 ++++ app/controllers/trips_controller.rb | 5 +- app/helpers/application_helper.rb | 23 ------ app/helpers/trips_helper.rb | 26 +++++++ app/models/trip.rb | 51 ++++--------- app/services/immich/request_photos.rb | 10 +-- app/services/trips/photos.rb | 43 +++++++++++ app/views/trips/show.html.erb | 8 +-- spec/models/trip_spec.rb | 20 ++++-- spec/requests/trips_spec.rb | 2 +- spec/serializers/api/photo_serializer_spec.rb | 2 + spec/services/photos/search_spec.rb | 6 +- spec/services/trips/photos_spec.rb | 72 +++++++++++++++++++ spec/swagger/api/v1/photos_controller_spec.rb | 4 +- swagger/v1/swagger.yaml | 7 ++ 16 files changed, 209 insertions(+), 84 deletions(-) create mode 100644 app/helpers/trips_helper.rb create mode 100644 app/services/trips/photos.rb create mode 100644 spec/services/trips/photos_spec.rb diff --git a/.app_version b/.app_version index b72b05ed..c0b8d590 100644 --- a/.app_version +++ b/.app_version @@ -1 +1 @@ -0.19.3 +0.19.4 diff --git a/CHANGELOG.md b/CHANGELOG.md index 16b6f86d..19dcf086 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb index 97492e74..2a9a26d2 100644 --- a/app/controllers/trips_controller.rb +++ b/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 - @trip.photos + @photo_previews = Rails.cache.fetch("trip_photos_#{@trip.id}", expires_in: 1.day) do + @trip.photo_previews end + @photo_sources = @trip.photo_sources end def new diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 8cc09c1c..3fe89204 100644 --- a/app/helpers/application_helper.rb +++ b/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 diff --git a/app/helpers/trips_helper.rb b/app/helpers/trips_helper.rb new file mode 100644 index 00000000..fa0b77ae --- /dev/null +++ b/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 diff --git a/app/models/trip.rb b/app/models/trip.rb index 7a0fdaba..e75103c2 100644 --- a/app/models/trip.rb +++ b/app/models/trip.rb @@ -17,20 +17,27 @@ class Trip < ApplicationRecord points.pluck(:country).uniq.compact end - def photos - return [] unless can_fetch_photos? - - filtered_photos.sample(12) - .sort_by { |photo| photo['localDateTime'] } - .map { |asset| photo_thumbnail(asset) } + def photo_previews + @photo_previews ||= select_dominant_orientation(photos).sample(12) end - def photos_sources - filtered_photos.map { _1[:source] }.uniq + def photo_sources + @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 - diff --git a/app/services/immich/request_photos.rb b/app/services/immich/request_photos.rb index 034a6452..59baa496 100644 --- a/app/services/immich/request_photos.rb +++ b/app/services/immich/request_photos.rb @@ -37,15 +37,7 @@ class Immich::RequestPhotos items = response.dig('assets', 'items') - 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 + break if items.blank? data << items diff --git a/app/services/trips/photos.rb b/app/services/trips/photos.rb new file mode 100644 index 00000000..33442833 --- /dev/null +++ b/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 diff --git a/app/views/trips/show.html.erb b/app/views/trips/show.html.erb index d0b265fc..f399eb3f 100644 --- a/app/views/trips/show.html.erb +++ b/app/views/trips/show.html.erb @@ -36,8 +36,8 @@ - <% if @photos.any? %> - <% @photos.each_slice(4) do |slice| %> + <% if @photo_previews.any? %> + <% @photo_previews.each_slice(4) do |slice| %>
<% slice.each do |photo| %>
@@ -52,9 +52,9 @@ <% end %> <% end %> - <% if @trip.photos_sources.any? %> + <% if @photo_sources.any? %>
- <% @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 %>
diff --git a/spec/models/trip_spec.rb b/spec/models/trip_spec.rb index b19f348c..0638d781 100644 --- a/spec/models/trip_spec.rb +++ b/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 diff --git a/spec/requests/trips_spec.rb b/spec/requests/trips_spec.rb index 20daa652..d0e1e794 100644 --- a/spec/requests/trips_spec.rb +++ b/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 diff --git a/spec/serializers/api/photo_serializer_spec.rb b/spec/serializers/api/photo_serializer_spec.rb index 3dad077a..7d354d16 100644 --- a/spec/serializers/api/photo_serializer_spec.rb +++ b/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 diff --git a/spec/services/photos/search_spec.rb b/spec/services/photos/search_spec.rb index 0ce34613..ee451597 100644 --- a/spec/services/photos/search_spec.rb +++ b/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 diff --git a/spec/services/trips/photos_spec.rb b/spec/services/trips/photos_spec.rb new file mode 100644 index 00000000..abe9f52b --- /dev/null +++ b/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 diff --git a/spec/swagger/api/v1/photos_controller_spec.rb b/spec/swagger/api/v1/photos_controller_spec.rb index eef5d9a5..2c375ef1 100644 --- a/spec/swagger/api/v1/photos_controller_spec.rb +++ b/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' } diff --git a/swagger/v1/swagger.yaml b/swagger/v1/swagger.yaml index 6657aebf..49d8ef24 100644 --- a/swagger/v1/swagger.yaml +++ b/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":