From ef95b20a7e6f3aa05eb3b5a2560ab426212982cd Mon Sep 17 00:00:00 2001 From: miteruzo Date: Thu, 16 Jul 2026 18:33:45 +0900 Subject: [PATCH] #399 --- backend/app/models/post.rb | 15 +----- backend/app/services/post_metadata_fetcher.rb | 22 ++++++++- backend/spec/models/post_spec.rb | 47 +++++++++++++++++++ .../services/post_metadata_fetcher_spec.rb | 7 +++ backend/spec/services/youtube/sync_spec.rb | 18 ++++++- .../posts/import/PostImportRowSummary.tsx | 7 +++ frontend/src/lib/postImportRows.test.ts | 18 +++++++ frontend/src/lib/postImportRows.ts | 17 +++++++ .../src/pages/posts/PostImportResultPage.tsx | 18 +++---- 9 files changed, 145 insertions(+), 24 deletions(-) diff --git a/backend/app/models/post.rb b/backend/app/models/post.rb index 8a18ce9..c0cc34a 100644 --- a/backend/app/models/post.rb +++ b/backend/app/models/post.rb @@ -158,10 +158,8 @@ class Post < ApplicationRecord def attach_thumbnail_from_url! raw_url response = Preview::ThumbnailFetcher.fetch_image_response(raw_url) thumbnail.attach( - io: StringIO.new(response.body), - filename: remote_thumbnail_filename(response.url), - content_type: response.content_type) - resized_thumbnail! + self.class.resized_thumbnail_attachment( + StringIO.new(response.body))) rescue Preview::UrlSafety::UnsafeUrl, Preview::ThumbnailFetcher::GenerationFailed, Preview::HttpFetcher::FetchFailed, @@ -173,15 +171,6 @@ class Post < ApplicationRecord private - def remote_thumbnail_filename url - filename = File.basename(URI.parse(url).path.to_s) - return filename if filename.present? && filename != '/' - - 'thumbnail' - rescue URI::InvalidURIError - 'thumbnail' - end - def validate_original_created_range f = parse_original_created_value(:original_created_from) b = parse_original_created_value(:original_created_before) diff --git a/backend/app/services/post_metadata_fetcher.rb b/backend/app/services/post_metadata_fetcher.rb index e1ac96a..8dc5f33 100644 --- a/backend/app/services/post_metadata_fetcher.rb +++ b/backend/app/services/post_metadata_fetcher.rb @@ -81,12 +81,22 @@ class PostMetadataFetcher day = match[3].to_i hour = match[4].to_i minute = match[5]&.to_i || 0 + second = match[6]&.to_i || 0 + fraction = match[7] offset = match[8] + nanoseconds = parse_nanoseconds(fraction) timestamp = if offset.present? - Time.new(year, month, day, hour, minute, 0, parse_offset(offset)).in_time_zone + Time.new( + year, + month, + day, + hour, + minute, + second + Rational(nanoseconds, 1_000_000_000), + parse_offset(offset)).in_time_zone else - Time.zone.local(year, month, day, hour, minute) + Time.zone.local(year, month, day, hour, minute, second).change(nsec: nanoseconds) end from = timestamp.change(sec: 0, nsec: 0) @@ -94,6 +104,13 @@ class PostMetadataFetcher [from, before] end + def self.parse_nanoseconds value + return 0 if value.blank? + + digits = value[0, 9].ljust(9, '0') + Integer(digits, 10) + end + def self.parse_offset value return '+00:00' if value == 'Z' @@ -109,6 +126,7 @@ class PostMetadataFetcher private_class_method :platform_tags, :original_created_range, :parse_timestamp_range, + :parse_nanoseconds, :parse_offset, :serialise_time end diff --git a/backend/spec/models/post_spec.rb b/backend/spec/models/post_spec.rb index 7ef84b4..f1d1175 100644 --- a/backend/spec/models/post_spec.rb +++ b/backend/spec/models/post_spec.rb @@ -187,12 +187,59 @@ RSpec.describe Post, type: :model do post = described_class.create!(title: 'title', url: 'https://example.com/post') + expect(post.thumbnail).to receive(:attach).once.and_call_original post.attach_thumbnail_from_url!('https://example.com/thumb.png') expect(post.thumbnail).to be_attached image = read_image(post.thumbnail) expect(image.dimensions).to eq([180, 180]) end + + it 'does not attach anything when thumbnail conversion fails' do + response = Preview::HttpFetcher::Response.new( + 'not-an-image', + 'image/png', + 'https://example.com/thumb.png') + allow(Preview::ThumbnailFetcher).to receive(:fetch_image_response) + .with('https://example.com/thumb.png') + .and_return(response) + allow(described_class).to receive(:resized_thumbnail_attachment) + .and_raise(MiniMagick::Error, 'convert failed') + + post = described_class.create!(title: 'title', url: 'https://example.com/post') + + expect { + post.attach_thumbnail_from_url!('https://example.com/thumb.png') + }.to raise_error(Post::RemoteThumbnailFetchFailed, 'convert failed') + + expect(post.thumbnail).not_to be_attached + end + + it 'keeps an existing thumbnail when remote conversion fails' do + existing = described_class.create!(title: 'title', url: 'https://example.com/post') + existing.thumbnail.attach( + io: StringIO.new('existing'), + filename: 'existing.jpg', + content_type: 'image/jpeg') + blob_id = existing.thumbnail.blob.id + response = Preview::HttpFetcher::Response.new( + 'not-an-image', + 'image/png', + 'https://example.com/thumb.png') + allow(Preview::ThumbnailFetcher).to receive(:fetch_image_response) + .with('https://example.com/thumb.png') + .and_return(response) + allow(described_class).to receive(:resized_thumbnail_attachment) + .and_raise(MiniMagick::Error, 'convert failed') + + expect { + existing.attach_thumbnail_from_url!('https://example.com/thumb.png') + }.to raise_error(Post::RemoteThumbnailFetchFailed, 'convert failed') + + existing.reload + expect(existing.thumbnail).to be_attached + expect(existing.thumbnail.blob.id).to eq(blob_id) + end end end end diff --git a/backend/spec/services/post_metadata_fetcher_spec.rb b/backend/spec/services/post_metadata_fetcher_spec.rb index 25d851e..2b2a468 100644 --- a/backend/spec/services/post_metadata_fetcher_spec.rb +++ b/backend/spec/services/post_metadata_fetcher_spec.rb @@ -61,4 +61,11 @@ RSpec.describe PostMetadataFetcher do expect(result.fetch(:original_created_from)).to be_nil expect(result.fetch(:original_created_before)).to be_nil end + + it 'returns nil dates for an invalid timestamp second value' do + result = fetch_with_published_time('2024-02-03T12:34:99Z') + + expect(result.fetch(:original_created_from)).to be_nil + expect(result.fetch(:original_created_before)).to be_nil + end end diff --git a/backend/spec/services/youtube/sync_spec.rb b/backend/spec/services/youtube/sync_spec.rb index 20de82d..9e8f2c0 100644 --- a/backend/spec/services/youtube/sync_spec.rb +++ b/backend/spec/services/youtube/sync_spec.rb @@ -23,6 +23,22 @@ RSpec.describe Youtube::Sync do post, 'https://example.com/thumb.jpg') end + + it 'retries on a later sync when the previous remote attach failed' do + post = create(:post, thumbnail_base: nil) + allow(sync).to receive(:attach_thumbnail_if_needed!).and_call_original + expect(post).to receive(:attach_thumbnail_from_url!) + .with('https://example.com/thumb.jpg') + .twice + .and_raise(Post::RemoteThumbnailFetchFailed, 'failed') + + 2.times do + sync.send( + :attach_thumbnail_if_needed!, + post, + 'https://example.com/thumb.jpg') + end + end end describe '#sync!' do @@ -91,7 +107,7 @@ RSpec.describe Youtube::Sync do sync.sync! end - it 'creates a YouTube post with default tags and no_deerjikist when no deerjikist mapping exists' do + it 'creates a YouTube post with default tags when no deerjikist mapping exists' do Tag.tagme Tag.bot Tag.youtube diff --git a/frontend/src/components/posts/import/PostImportRowSummary.tsx b/frontend/src/components/posts/import/PostImportRowSummary.tsx index 4c7a0d0..b68e270 100644 --- a/frontend/src/components/posts/import/PostImportRowSummary.tsx +++ b/frontend/src/components/posts/import/PostImportRowSummary.tsx @@ -92,6 +92,13 @@ const PostImportRowSummary: FC = ({ row, onEdit }) => {
{displayStatus != null && }
+
+ {String (row.attributes.tags ?? '') || 'タグなし'} +
+
+ {summaryDate (row)} + {row.attributes.duration ? ` / ${ row.attributes.duration }` : ''} +
{warning && (
{warning} diff --git a/frontend/src/lib/postImportRows.test.ts b/frontend/src/lib/postImportRows.test.ts index b1aeb01..aad4b67 100644 --- a/frontend/src/lib/postImportRows.test.ts +++ b/frontend/src/lib/postImportRows.test.ts @@ -5,6 +5,8 @@ import { creatableImportRows, mergeImportResults, mergeValidatedImportRows, processableImportRows, + resultRepairMode, + resultRowMessages, resultSummaryCounts, retryImportRow, reviewSummaryCounts } from '@/lib/postImportSession' @@ -131,6 +133,22 @@ describe ('post import row state', () => { importErrors: undefined }) }) + it ('deduplicates messages and keeps repair mode only for repairable rows', () => { + const repairable = buildPostImportRow ({ + sourceRow: 1, + importStatus: 'pending', + validationErrors: { title: ['invalid'], base: ['duplicate'] }, + importErrors: { base: ['duplicate'], url: ['network'] } }) + const complete = buildPostImportRow ({ + sourceRow: 2, + importStatus: 'created', + createdPostId: 2 }) + + expect (resultRowMessages (repairable)).toEqual (['invalid', 'duplicate', 'network']) + expect (resultRepairMode ([repairable, complete])).toBe ('failed') + expect (resultRepairMode ([complete])).toBe ('all') + }) + it ('copies reset snapshot values instead of sharing mutable records', () => { const row = buildPostImportRow ({ fieldWarnings: { title: ['warning'] } }) const initialised = initialisePreviewRows ([row])[0] diff --git a/frontend/src/lib/postImportRows.ts b/frontend/src/lib/postImportRows.ts index d66583f..17e9ec8 100644 --- a/frontend/src/lib/postImportRows.ts +++ b/frontend/src/lib/postImportRows.ts @@ -7,6 +7,9 @@ const hasSkipReason = (row: PostImportRow): boolean => const hasValidationErrors = (row: PostImportRow): boolean => Object.keys (row.validationErrors ?? { }).length > 0 +const isRepairableImportStatus = (row: PostImportRow): boolean => + row.importStatus === 'failed' || row.importStatus === 'pending' + const buildResetSnapshot = (row: PostImportRow) => ({ url: row.url, attributes: { ...row.attributes }, @@ -67,6 +70,20 @@ export const resultSummaryCounts = (rows: PostImportRow[]) => { created: 0, skipped: 0, failed: 0 }) +export const resultRepairMode = ( + rows: PostImportRow[], +): 'all' | 'failed' => + rows.some (row => hasValidationErrors (row) || isRepairableImportStatus (row)) + ? 'failed' + : 'all' + + +export const resultRowMessages = (row: PostImportRow): string[] => + [...new Set ([ + ...Object.values (row.validationErrors ?? { }).flat (), + ...Object.values (row.importErrors ?? { }).flat ()])] + + export const mergeValidatedImportRows = ( current: PostImportRow[], validated: PostImportRow[], diff --git a/frontend/src/pages/posts/PostImportResultPage.tsx b/frontend/src/pages/posts/PostImportResultPage.tsx index c53d6de..e491eba 100644 --- a/frontend/src/pages/posts/PostImportResultPage.tsx +++ b/frontend/src/pages/posts/PostImportResultPage.tsx @@ -20,6 +20,8 @@ import { clearPostImportSourceDraft, loadPostImportSession, mergeImportResults, mergeValidatedImportRows, + resultRepairMode, + resultRowMessages, resultSummaryCounts, retryImportRow, savePostImportSession } from '@/lib/postImportSession' @@ -34,9 +36,6 @@ import type { User } from '@/types' type Props = { user: User | null } -const rowMessages = (row: PostImportRow): string[] => - Object.values (row.importErrors ?? { }).flat () - const buildNextEditedRow = ( editingRow: PostImportRow, draft: PostImportRowDraft, @@ -293,7 +292,7 @@ const PostImportResultPage: FC = ({ user }) => { const resultSession = { ...nextSession, rows: nextRows, - repairMode: recoverableTarget == null ? 'all' as const : 'failed' as const } + repairMode: resultRepairMode (nextRows) } sessionRef.current = resultSession setSession (resultSession) if (recoverableTarget != null @@ -367,10 +366,13 @@ const PostImportResultPage: FC = ({ user }) => {
{session.rows.map (row => { const displayStatus = displayPostImportStatus (row) - const canEdit = row.importStatus === 'failed' - const canRetry = + const hasValidationErrors = Object.keys (row.validationErrors).length > 0 + const canEdit = row.importStatus === 'failed' - && Object.keys (row.validationErrors).length === 0 + || (row.importStatus === 'pending' && hasValidationErrors) + const canRetry = + (row.importStatus === 'failed' || row.importStatus === 'pending') + && !(hasValidationErrors) return (
= ({ user }) => {
{row.url}
- +