diff --git a/AGENTS.md b/AGENTS.md index c73df4c..ccded6f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -141,140 +141,72 @@ npm run preview - Ruby の multi-line hash literal / keyword-like argument hash では、opening `{` を最初の pair と同じ行に置き、closing `}` を最後の pair と同じ行に 置く。Prettier 的な縦開き・縦閉じをしない。 -- Ruby の `if` / `unless` / `case` 条件で、複数行に分けるだけで安易に - `if ... end` へ展開しない。局所の既存コードが modifier 形式ならそれに - そろえる。 -- Ruby の guard 条件は、1 行で収まるなら modifier 形式を優先する。2 行以上に - なるなら通常の block 形式へ切り替へてよい。 -- Ruby の method chain や call argument を折り返す際、call-site の `)` - 直前で行を空けたり、closing delimiter を block のやうに独立させない。 +- Ruby の guard 条件は、1 行で収まるなら modifier 形式を優先する。 + 99 文字を超えるなら block 形式へ切り替へるか、message 定数化などで縮める。 +- Ruby の method chain や call argument を折り返す際、call-site の `)` を + block close のやうに独立させない。 Bad: ```rb -source = - if params[:format] == 'google_sheets' - PostImportGoogleSheetsFetcher.fetch!(params[:source], - rate_key: current_user.id) - else - params[:source] - end -parsed = PostImportSourceParser.new( - source:, - format:, - has_header: params[:has_header], - json_path: params[:json_path], -).parse -``` - -Good: - -```rb -source = - if params[:format] == 'google_sheets' - PostImportGoogleSheetsFetcher.fetch!(params[:source], - rate_key: current_user.id) - else - params[:source] - end -parsed = PostImportSourceParser.new( - source:, - format:, - has_header: params[:has_header], - json_path: params[:json_path]).parse -``` - -Bad: - -```rb -result[field] = { - 'kind' => kind, - 'value' => value['value'].to_s, - 'columns' => Array(columns).map { Integer(_1) }, -} -``` - -Good: - -```rb -result[field] = { - 'kind' => kind, - 'value' => value['value'].to_s, - 'columns' => Array(columns).map { Integer(_1) } } -``` - -Bad: - -```rb -unless rows.all? { |row| - values = Array(row['values']) - values.length <= columns.length && - values.all? { |value| - value.is_a?(String) && - value.bytesize <= PostImportSourceParser::MAX_CELL_BYTES - } - } - raise ArgumentError, '解析結果のセルが不正です．' -end -``` - -Good: - -```rb -raise ArgumentError, '解析結果のセルが不正です．' unless rows.all? { |row| - values = Array(row['values']) - values.length <= columns.length && values.all? { |value| - value.is_a?(String) && value.bytesize <= PostImportSourceParser::MAX_CELL_BYTES - } -} -``` - -Bad: - -```rb -if row['url'].to_s.bytesize > PostImportSourceParser::MAX_CELL_BYTES - raise ArgumentError, 'URL が長すぎます．' -end -``` - -Good: - -```rb -raise ArgumentError, 'URL が長すぎます．' if row['url'].to_s.bytesize > PostImportSourceParser::MAX_CELL_BYTES -``` - -Bad: - -```rb -parameters = row.is_a?(ActionController::Parameters) ? row : - ActionController::Parameters.new(row) -``` - -Good: - -```rb -parameters = - row.is_a?(ActionController::Parameters) ? row : ActionController::Parameters.new(row) -``` - -Bad: - -```rb -PostTagSection.create!( - post_id: post.id, - tag_id:, - begin_ms:, - end_ms:, +response = Example.fetch( + value, + option: option, ) ``` Good: ```rb -PostTagSection.create!(post_id: post.id, - tag_id:, - begin_ms:, - end_ms:) +response = Example.fetch( + value, + option: option) +``` + +Bad: + +```rb +payload = { + title: title, + url: url, +} +``` + +Good: + +```rb +payload = { + title: title, + url: url } +``` + +Bad: + +```rb +raise ArgumentError, 'URL が長すぎます．' if url.bytesize > MAX_URL_BYTES && flag.present? +``` + +Good: + +```rb +if url.bytesize > MAX_URL_BYTES && flag.present? + raise ArgumentError, 'URL が長すぎます．' +end +``` + +Bad: + +```rb +records.each { + do_work(_1) } +``` + +Good: + +```rb +records.each { + do_work(_1) +} ``` - TypeScript and Python: use GNU-style spacing before parentheses where syntactically valid. diff --git a/backend/app/controllers/post_imports_controller.rb b/backend/app/controllers/post_imports_controller.rb index eb1487f..5aefd2b 100644 --- a/backend/app/controllers/post_imports_controller.rb +++ b/backend/app/controllers/post_imports_controller.rb @@ -6,11 +6,11 @@ class PostImportsController < ApplicationController rows: PostImportUrlListParser.parse(params[:source])) render json: { rows: } rescue ArgumentError => e - render_bad_request(e.message) + render_bad_request e.message end def validate - rows = normalised_import_rows(allow_warning_fields: true) + rows = normalised_import_rows allow_warning_fields: true changed_row = Integer(params[:changed_row], exception: false) result = PostImportPreviewer.new.preview_rows(rows:, @@ -18,7 +18,7 @@ class PostImportsController < ApplicationController metadata_cache: { }) render json: { rows: result } rescue ArgumentError => e - render_bad_request(e.message) + render_bad_request e.message end def create @@ -26,7 +26,7 @@ class PostImportsController < ApplicationController rows: normalised_import_rows).run render json: result, status: result[:created].positive? ? :created : :ok rescue ArgumentError => e - render_bad_request(e.message) + render_bad_request e.message end private diff --git a/backend/app/models/post.rb b/backend/app/models/post.rb index 4394603..0697ee2 100644 --- a/backend/app/models/post.rb +++ b/backend/app/models/post.rb @@ -174,14 +174,5 @@ class Post < ApplicationRecord return if url.blank? self.url = PostUrlNormaliser.normalise(url) || url.strip - - u = URI.parse(url) - return unless u in URI::HTTP - - u.host = u.host.downcase if u.host - u.path = u.path.sub(/\/\Z/, '') if u.path.present? - self.url = PostUrlSanitisationRule.sanitise(u.to_s) - rescue URI::InvalidURIError - ; end end diff --git a/backend/app/services/post_import_previewer.rb b/backend/app/services/post_import_previewer.rb index cc0d89b..ea319b9 100644 --- a/backend/app/services/post_import_previewer.rb +++ b/backend/app/services/post_import_previewer.rb @@ -38,7 +38,7 @@ class PostImportPreviewer provenance['url'] = 'manual' normal_url = normalised_url(url) url_for_metadata = normal_url || url - existing_post = normal_url.present? && Post.exists?(url: normal_url) + existing_post = normal_url.present? ? Post.find_by(url: normal_url) : nil validation_errors = {} validation_errors[:url] = ['URL が不正です．'] if normal_url.blank? @@ -61,6 +61,8 @@ class PostImportPreviewer provenance:, tag_sources:, metadata_url: url_for_metadata, + skip_reason: 'existing', + existing_post_id: existing_post.id, field_warnings:, base_warnings:, validation_errors:, @@ -93,6 +95,8 @@ class PostImportPreviewer provenance:, tag_sources:, metadata_url: url_for_metadata, + skip_reason: nil, + existing_post_id: nil, field_warnings:, base_warnings:, validation_errors:, @@ -129,7 +133,9 @@ class PostImportPreviewer end def initial_field_warnings row - (row[:field_warnings] || { }).stringify_keys.transform_values { |value| Array(value).map(&:to_s) } + (row[:field_warnings] || { }) + .stringify_keys + .transform_values { |value| Array(value).map(&:to_s) } end def initial_base_warnings row @@ -161,7 +167,9 @@ class PostImportPreviewer data = PostMetadataFetcher.fetch(url).stringify_keys.compact warnings = { } add_field_warning!(warnings, 'title', TITLE_FETCH_WARNING) if data['title'].blank? - add_field_warning!(warnings, 'thumbnail_base', THUMBNAIL_FETCH_WARNING) if data['thumbnail_base'].blank? + if data['thumbnail_base'].blank? + add_field_warning!(warnings, 'thumbnail_base', THUMBNAIL_FETCH_WARNING) + end { data:, warnings: } rescue Preview::UrlSafety::UnsafeUrl, Preview::HttpFetcher::FetchFailed, diff --git a/backend/app/services/post_import_row_normaliser.rb b/backend/app/services/post_import_row_normaliser.rb index 9172cb1..fa2b7be 100644 --- a/backend/app/services/post_import_row_normaliser.rb +++ b/backend/app/services/post_import_row_normaliser.rb @@ -161,7 +161,9 @@ class PostImportRowNormaliser unless base_warnings.is_a?(Array) && base_warnings.all? { _1.is_a?(String) } raise ArgumentError, '警告の形式が不正です．' end - raise ArgumentError, '警告が大きすぎます．' if base_warnings.any? { _1.bytesize > PostImportUrlListParser::MAX_URL_BYTES } + if base_warnings.any? { _1.bytesize > PostImportUrlListParser::MAX_URL_BYTES } + raise ArgumentError, '警告が大きすぎます．' + end end private_class_method :normalise_warning_values! diff --git a/backend/app/services/post_import_runner.rb b/backend/app/services/post_import_runner.rb index 0fe4ef8..7ba824d 100644 --- a/backend/app/services/post_import_runner.rb +++ b/backend/app/services/post_import_runner.rb @@ -6,40 +6,29 @@ class PostImportRunner def run normalised_rows = PostImportRowNormaliser.normalise!(@rows) + previews = PostImportPreviewer.new.preview_rows(rows: normalised_rows, + fetch_metadata: false) + preview_map = previews.index_by { _1[:source_row] } - results = normalised_rows.map { |row| run_row(row) } - { - created: results.count { _1[:status] == 'created' }, + results = normalised_rows.map do |row| + run_row(row, preview_map.fetch(row['source_row'])) + end + + { created: results.count { _1[:status] == 'created' }, skipped: results.count { _1[:status] == 'skipped' }, failed: results.count { _1[:status] == 'failed' }, - rows: results, - } + rows: results } end private - def run_row row + def run_row row, preview attributes = row.fetch('attributes', { }).transform_keys { _1.to_s.underscore } - preview = PostImportPreviewer.new.preview_rows( - rows: [{ - source_row: row['source_row'], - url: row['url'], - attributes:, - provenance: row['provenance'], - tag_sources: row['tag_sources'], - }], - fetch_metadata: false).first - if Array(preview.dig(:field_warnings, 'url')).include?(PostImportPreviewer::EXISTING_SKIP_WARNING) - return row.slice('source_row').merge(status: 'skipped') - end + return row.slice('source_row').merge(status: 'skipped') if preview[:skip_reason] == 'existing' - if preview[:validation_errors].present? - return { - source_row: row['source_row'], - status: 'failed', - errors: preview[:validation_errors], - } - end + return { source_row: row['source_row'], + status: 'failed', + errors: preview[:validation_errors] } if preview[:validation_errors].present? attributes['tags'] = preview[:attributes]['tags'] attributes['url'] = row['url'] @@ -48,15 +37,22 @@ class PostImportRunner rescue ActiveRecord::RecordInvalid => e { source_row: row['source_row'], status: 'failed', errors: e.record.errors.to_hash } rescue Tag::NicoTagNormalisationError - { source_row: row['source_row'], status: 'failed', errors: { tags: ['ニコニコ・タグは直接指定できません．'] } } + { source_row: row['source_row'], + status: 'failed', + errors: { tags: ['ニコニコ・タグは直接指定できません．'] } } rescue Tag::DeprecatedTagNormalisationError - { source_row: row['source_row'], status: 'failed', errors: { tags: ['廃止済みタグは付与できません．'] } } + { source_row: row['source_row'], + status: 'failed', + errors: { tags: ['廃止済みタグは付与できません．'] } } rescue ArgumentError, PostCreator::VideoMsParseError - { source_row: row['source_row'], status: 'failed', errors: { base: ['入力値が不正です．'] } } + { source_row: row['source_row'], + status: 'failed', + errors: { base: ['入力値が不正です．'] } } rescue StandardError => e - Rails.logger.error( - "post_import_runner_failure #{ { error: e.class.name, message: e.message }.to_json }", - ) - { source_row: row['source_row'], status: 'failed', errors: { base: ['登録中にエラーが発生しました．'] } } + Rails.logger.error("post_import_runner_failure #{ { error: e.class.name, + message: e.message }.to_json }") + { source_row: row['source_row'], + status: 'failed', + errors: { base: ['登録中にエラーが発生しました．'] } } end end diff --git a/frontend/src/components/posts/import/PostImportRowDialog.tsx b/frontend/src/components/posts/import/PostImportRowDialog.tsx index f0541b6..7472e5b 100644 --- a/frontend/src/components/posts/import/PostImportRowDialog.tsx +++ b/frontend/src/components/posts/import/PostImportRowDialog.tsx @@ -43,11 +43,27 @@ const originOf = ( ): PostImportOrigin => row.provenance[field] ?? 'automatic' -const originalCreatedOrigin = (row: PostImportRow): PostImportOrigin => - originOf (row, 'originalCreatedFrom') === 'manual' - || originOf (row, 'originalCreatedBefore') === 'manual' +const changedOrigin = ( + changed: boolean, + row: PostImportRow, + field: string, +): PostImportOrigin => + changed ? 'manual' : originOf (row, field) + +const originalCreatedOrigin = ( + row: PostImportRow, + originalDraft: Draft, + draft: Draft, +): PostImportOrigin => + originalDraft.originalCreatedFrom !== draft.originalCreatedFrom + || originalDraft.originalCreatedBefore !== draft.originalCreatedBefore ? 'manual' - : 'automatic' + : ( + originOf (row, 'originalCreatedFrom') === 'manual' + || originOf (row, 'originalCreatedBefore') === 'manual' + ? 'manual' + : 'automatic' + ) const buildDraft = (row: PostImportRow): Draft => ({ url: row.url, @@ -78,6 +94,10 @@ const PostImportRowDialog: FC = ( if (!(row) || !(draft)) return null + const originalDraft = buildDraft (row) + const fieldOrigin = (field: keyof Draft): PostImportOrigin => + changedOrigin (draft[field] !== originalDraft[field], row, field) + const update = ( key: Key, value: Draft[Key], @@ -117,21 +137,21 @@ const PostImportRowDialog: FC = ( update ('url', value)}/> update ('title', value)}/> = ( )} onChange={value => update ('thumbnailBase', value)}/> } + labelAddon={ + + } originalCreatedFrom={draft.originalCreatedFrom || null} setOriginalCreatedFrom={value => update ('originalCreatedFrom', value ?? '')} originalCreatedBefore={draft.originalCreatedBefore || null} @@ -155,7 +178,7 @@ const PostImportRowDialog: FC = ( = ( update ('tags', value)}/> = ({ row, onEdit }) => {
#{row.sourceRow}
@@ -96,13 +94,6 @@ const PostImportRowSummary: FC = ({ row, onEdit }) => { {summaryDate (row) || '日時未取得'} {row.attributes.duration ? ` / ${ row.attributes.duration }` : ''}
- {row.createdPostId && ( - - 投稿 #{row.createdPostId} - )} {warning && (
{warning} @@ -145,13 +136,6 @@ const PostImportRowSummary: FC = ({ row, onEdit }) => {
- {row.createdPostId && ( - - 投稿 #{row.createdPostId} - )} {error && (
{error} diff --git a/frontend/src/components/posts/import/PostImportStatusBadge.tsx b/frontend/src/components/posts/import/PostImportStatusBadge.tsx index c8adb79..7556077 100644 --- a/frontend/src/components/posts/import/PostImportStatusBadge.tsx +++ b/frontend/src/components/posts/import/PostImportStatusBadge.tsx @@ -2,22 +2,12 @@ import { cn } from '@/lib/utils' import type { FC } from 'react' -import type { PostImportOrigin } from '@/lib/postImportSession' - -type BadgeValue = - 'ready' - | 'warning' - | 'error' - | 'pending' - | 'created' - | 'skipped' - | 'failed' - | PostImportOrigin +import type { PostImportBadgeValue } from '@/components/posts/import/postImportRowStatus' type Props = { - value: BadgeValue } + value: PostImportBadgeValue } -const LABELS: Record = { +const LABELS: Record = { ready: '登録可能', warning: '警告', error: '要修正', @@ -28,7 +18,7 @@ const LABELS: Record = { automatic: '自動取得', manual: '手修正' } -const STYLES: Record = { +const STYLES: Record = { ready: [ 'border-emerald-300 bg-emerald-50 text-emerald-700', 'dark:border-emerald-900 dark:bg-emerald-950 dark:text-emerald-200'], diff --git a/frontend/src/components/posts/import/postImportRowStatus.ts b/frontend/src/components/posts/import/postImportRowStatus.ts index 35afa2b..dae7bfa 100644 --- a/frontend/src/components/posts/import/postImportRowStatus.ts +++ b/frontend/src/components/posts/import/postImportRowStatus.ts @@ -1,4 +1,5 @@ -import type { PostImportRow } from '@/lib/postImportSession' +import type { PostImportOrigin, + PostImportRow } from '@/lib/postImportSession' export type PostImportEffectiveStatus = 'created' @@ -8,6 +9,11 @@ export type PostImportEffectiveStatus = | 'warning' | 'ready' +export type PostImportBadgeValue = + PostImportEffectiveStatus + | 'pending' + | PostImportOrigin + export const effectivePostImportStatus = ( row: PostImportRow, diff --git a/frontend/src/lib/postImportSession.ts b/frontend/src/lib/postImportSession.ts index a479e7d..763ad40 100644 --- a/frontend/src/lib/postImportSession.ts +++ b/frontend/src/lib/postImportSession.ts @@ -5,6 +5,7 @@ export type PostImportStatus = | 'created' | 'skipped' | 'failed' +export type PostImportSkipReason = 'existing' export type PostImportResultStatus = 'created' | 'skipped' | 'failed' export type PostImportAttributeValue = string | number @@ -19,6 +20,8 @@ export type PostImportRow = { provenance: Record tagSources?: Record status: 'ready' | 'warning' | 'error' + skipReason?: PostImportSkipReason + existingPostId?: number metadataUrl?: string createdPostId?: number importStatus?: PostImportStatus } @@ -185,6 +188,12 @@ const isValidOrigin = ( value === 'automatic' || value === 'manual' +const isValidSkipReason = ( + value: unknown, +): value is PostImportSkipReason => + value === 'existing' + + const sanitiseRow = (value: unknown): PostImportRow | null => { if (!(isPlainObject (value))) return null @@ -200,6 +209,8 @@ const sanitiseRow = (value: unknown): PostImportRow | null => { return null if (value.importStatus != null && !(isValidImportStatus (value.importStatus))) return null + if (value.skipReason != null && !(isValidSkipReason (value.skipReason))) + return null const validationErrors = ensureStringListRecord (value.validationErrors) const fieldWarnings = ensureStringListRecord (value.fieldWarnings) @@ -239,6 +250,9 @@ const sanitiseRow = (value: unknown): PostImportRow | null => { provenance: value.provenance as Record, tagSources: value.tagSources as Record | undefined, status: value.status, + skipReason: value.skipReason, + existingPostId: + Number.isInteger (value.existingPostId) ? Number (value.existingPostId) : undefined, metadataUrl: typeof value.metadataUrl === 'string' ? value.metadataUrl : undefined, createdPostId: Number.isInteger (value.createdPostId) ? Number (value.createdPostId) : undefined, @@ -422,6 +436,8 @@ export const mergeValidatedImportRows = ( attributes: row.attributes, provenance: row.provenance, tagSources: row.tagSources, + skipReason: row.skipReason, + existingPostId: row.existingPostId, fieldWarnings, baseWarnings: row.baseWarnings.length > 0 || row.metadataUrl !== previous.metadataUrl @@ -469,6 +485,8 @@ export const mergePreviewImportRows = ( attributes: mergedAttributes, provenance: mergedProvenance, tagSources: mergedTagSources, + skipReason: row.skipReason, + existingPostId: row.existingPostId, createdPostId: previous.createdPostId, importStatus: previous.importStatus, importErrors: previous.importErrors } @@ -584,8 +602,8 @@ export const validateImportSource = ( } -const hasSkipWarning = (row: PostImportRow): boolean => - (row.fieldWarnings.url ?? []).includes ('既存投稿のためスキップします．') +const hasSkipReason = (row: PostImportRow): boolean => + row.skipReason === 'existing' export const submittableImportRows = (rows: PostImportRow[]): PostImportRow[] => @@ -594,7 +612,7 @@ export const submittableImportRows = (rows: PostImportRow[]): PostImportRow[] => return false if (row.importStatus === 'skipped') return false - if (hasSkipWarning (row)) + if (hasSkipReason (row)) return false if (row.importStatus === 'failed') return false @@ -609,6 +627,6 @@ export const reviewSummaryCounts = (rows: PostImportRow[]) => ({ && Object.keys (row.validationErrors ?? { }).length === 0).length, invalid: rows.filter (row => Object.keys (row.validationErrors ?? { }).length > 0).length, skipPlanned: rows.filter (row => - hasSkipWarning (row) && row.importStatus !== 'created').length, + hasSkipReason (row) && row.importStatus !== 'created').length, created: rows.filter (row => row.importStatus === 'created').length, failed: rows.filter (row => row.importStatus === 'failed').length }) diff --git a/frontend/src/pages/posts/PostImportResultPage.tsx b/frontend/src/pages/posts/PostImportResultPage.tsx index 07331b3..1eb823d 100644 --- a/frontend/src/pages/posts/PostImportResultPage.tsx +++ b/frontend/src/pages/posts/PostImportResultPage.tsx @@ -192,13 +192,6 @@ const PostImportResultPage: FC = ({ user }) => {
{row.url}
- {row.createdPostId && ( - - 投稿 #{row.createdPostId} - )}
diff --git a/frontend/src/pages/posts/PostImportSourcePage.tsx b/frontend/src/pages/posts/PostImportSourcePage.tsx index 16749bd..0cba73a 100644 --- a/frontend/src/pages/posts/PostImportSourcePage.tsx +++ b/frontend/src/pages/posts/PostImportSourcePage.tsx @@ -29,6 +29,8 @@ import type { User } from '@/types' type Props = { user: User | null } const MAX_ROWS = 100 +const SOURCE_ERROR_ID = 'post-import-source-error' +const SOURCE_ISSUES_ID = 'post-import-source-issues' const PostImportSourcePage: FC = ({ user }) => { @@ -45,6 +47,11 @@ const PostImportSourcePage: FC = ({ user }) => { const lineCount = countImportSourceLines (source) const messages = sourceError ? [sourceError] : [] + const sourceDescribedBy = [ + sourceError ? SOURCE_ERROR_ID : null, + sourceIssues.length > 0 ? SOURCE_ISSUES_ID : null] + .filter (_1 => _1 != null) + .join (' ') useEffect (() => { cleanupExpiredPostImportSessions (message => @@ -134,6 +141,8 @@ const PostImportSourcePage: FC = ({ user }) => {