diff --git a/AGENTS.md b/AGENTS.md index acf1a18..1722ece 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -266,6 +266,44 @@ const value = value selection. Do not replace a clear ternary with `if` statements, and do not introduce immediately invoked functions just to avoid or reformat a ternary expression. +- In TypeScript and TSX, multi-stage ternary expressions must make branch + boundaries explicit. Wrap each condition group in parentheses instead of + relying on indentation alone to show which `?` matches which `:`. +- In TypeScript and TSX, wrap nested ternary expressions in parentheses. Do + not write flat vertical chains of `?` and `:` without explicit grouping. +- In TypeScript and TSX, when a ternary condition contains `&&` or `||`, wrap + the whole condition in parentheses before `?`. +- In TypeScript and TSX, if a ternary reaches three or more stages and still + reads poorly after explicit grouping, extract a helper function or use `if` + statements instead of keeping a flat multi-stage ternary. + +Bad: + +```ts +row.skipReason === 'existing' || row.importStatus === 'skipped' +? 'skipped' +: Object.keys (row.validationErrors ?? { }).length > 0 + || row.importStatus === 'failed' + || row.importStatus === 'created' +? null +: hasWarnings (row) || row.status === 'warning' +? 'warning' +: 'ready' +``` + +Good: + +```ts +(row.skipReason === 'existing' || row.importStatus === 'skipped') +? 'skipped' +: ((Object.keys (row.validationErrors ?? { }).length > 0 + || row.importStatus === 'failed' + || row.importStatus === 'created') + ? null + : ((hasWarnings (row) || row.status === 'warning') + ? 'warning' + : 'ready')) +``` - In TypeScript and TSX, do not write `let` followed by later `if` assignments when the value can be expressed as a single `const` initializer. Prefer `const` because it prevents accidental later reassignment. diff --git a/backend/app/services/post_import_previewer.rb b/backend/app/services/post_import_previewer.rb index 7fa55db..b572729 100644 --- a/backend/app/services/post_import_previewer.rb +++ b/backend/app/services/post_import_previewer.rb @@ -55,7 +55,7 @@ class PostImportPreviewer source = row.symbolize_keys url = source[:url].to_s.strip normal_url = normalised_url(url) - source.merge(url_text: url, normal_url:) + source.merge(url_text: url, normal_url:, url_error: validate_url_safety(normal_url)) end def preview_row row, @@ -78,6 +78,9 @@ class PostImportPreviewer validation_errors = {} validation_errors[:url] = ['URL が不正です．'] if normal_url.blank? + if row[:url_error].present? + validation_errors[:url] = [row[:url_error]] + end if normal_url.present? && url_counts[normal_url].to_i > 1 validation_errors[:url] = ['URL が重複しています．'] end @@ -206,8 +209,7 @@ class PostImportPreviewer add_field_warning!(warnings, 'thumbnail_base', THUMBNAIL_FETCH_WARNING) end { data:, warnings: } - rescue Preview::UrlSafety::UnsafeUrl, - Preview::HttpFetcher::FetchFailed, + rescue Preview::HttpFetcher::FetchFailed, Preview::HttpFetcher::ResponseTooLarge => e Rails.logger.info( "post_import_metadata_fetch_failure "\ @@ -252,6 +254,7 @@ class PostImportPreviewer def preload_metadata! prepared_rows, fetch_metadata, metadata_cache, existing_posts, url_counts urls = prepared_rows.filter_map { |row| next unless row[:normal_url].present? + next if row[:url_error].present? next unless should_fetch_metadata?(fetch_metadata, row[:source_row].to_i) next if url_counts[row[:normal_url]].to_i > 1 next if existing_posts.key?(row[:normal_url]) @@ -339,12 +342,22 @@ class PostImportPreviewer def should_apply_metadata_to_row? row, fetch_metadata, existing_posts, url_counts normal_url = row[:normal_url] return false if normal_url.blank? + return false if row[:url_error].present? return false if url_counts[normal_url].to_i > 1 return false if existing_posts.key?(normal_url) should_fetch_metadata?(fetch_metadata, row[:source_row].to_i) end + def validate_url_safety normal_url + return nil if normal_url.blank? + + Preview::UrlSafety.validate(normal_url) + nil + rescue Preview::UrlSafety::UnsafeUrl => e + e.message + end + def apply_metadata! attributes, provenance, tag_sources, metadata metadata.each do |field, value| if field == 'tags' diff --git a/backend/app/services/post_import_runner.rb b/backend/app/services/post_import_runner.rb index 44564dd..4646c1c 100644 --- a/backend/app/services/post_import_runner.rb +++ b/backend/app/services/post_import_runner.rb @@ -46,6 +46,13 @@ class PostImportRunner end { source_row: row['source_row'], status: 'failed', errors: e.record.errors.to_hash } + rescue ActiveRecord::RecordNotUnique + existing_post = existing_post_for_race(row) + raise unless existing_post + + { source_row: row['source_row'], + status: 'skipped', + existing_post_id: existing_post.id } rescue Tag::NicoTagNormalisationError { source_row: row['source_row'], status: 'failed', @@ -66,8 +73,10 @@ class PostImportRunner errors: { base: ['登録中にエラーが発生しました．'] } } end - def existing_post_for_race row, record - return nil unless record.errors.of_kind?(:url, :taken) + def existing_post_for_race row, record = nil + if record && !(record.errors.of_kind?(:url, :taken)) + return nil + end normal_url = PostUrlNormaliser.normalise(row['url']) return nil if normal_url.blank? diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index cddc89b..e61b8b7 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -129,6 +129,10 @@ pass or the remaining failure is clearly blocked. it is JSX- or React-specific. - Preserve compact TSX expression shapes such as inline ternary branches and closing `)` forms when nearby code uses them. +- Multi-stage ternary expressions must use explicit parentheses for each + condition group and nested ternary branch. Do not rely on indentation alone + to show `?` / `:` pairing, and extract a helper or `if` when a three-stage + ternary still reads poorly after grouping. - Treat TypeScript and TSX formatting rules as hard constraints, not preferences. Before finishing a TypeScript or TSX edit, inspect the edited hunks for closing `)`, `]`, and `}` placement and fix violations instead of diff --git a/frontend/src/components/PostOriginalCreatedTimeField.tsx b/frontend/src/components/PostOriginalCreatedTimeField.tsx index 4ed3d29..8fef4c9 100644 --- a/frontend/src/components/PostOriginalCreatedTimeField.tsx +++ b/frontend/src/components/PostOriginalCreatedTimeField.tsx @@ -2,11 +2,10 @@ import DateTimeField from '@/components/common/DateTimeField' import FormField from '@/components/common/FormField' import { Button } from '@/components/ui/button' -import type { FC, ReactNode } from 'react' +import type { FC } from 'react' type Props = { disabled?: boolean - labelAddon?: ReactNode originalCreatedFrom: string | null setOriginalCreatedFrom: (x: string | null) => void originalCreatedBefore: string | null @@ -16,19 +15,12 @@ type Props = { const PostOriginalCreatedTimeField: FC = ( { disabled, - labelAddon, originalCreatedFrom, setOriginalCreatedFrom, originalCreatedBefore, setOriginalCreatedBefore, errors }: Props) => ( - - オリジナルの作成日時 - {labelAddon} - } - messages={errors}> + {({ describedBy, invalid }) => ( <>
diff --git a/frontend/src/components/posts/import/PageActionFooter.tsx b/frontend/src/components/posts/import/PageActionFooter.tsx deleted file mode 100644 index 6cd149d..0000000 --- a/frontend/src/components/posts/import/PageActionFooter.tsx +++ /dev/null @@ -1,22 +0,0 @@ -import type { FC, ReactNode } from 'react' - -type Props = { - summary: ReactNode - actions: ReactNode } - -const PageActionFooter: FC = ({ summary, actions }) => ( -
-
-
- {summary} -
-
- {actions} -
-
-
) - -export default PageActionFooter diff --git a/frontend/src/components/posts/import/PostImportRowDialog.tsx b/frontend/src/components/posts/import/PostImportRowDialog.tsx index 41d915d..4678999 100644 --- a/frontend/src/components/posts/import/PostImportRowDialog.tsx +++ b/frontend/src/components/posts/import/PostImportRowDialog.tsx @@ -2,9 +2,7 @@ import PostOriginalCreatedTimeField from '@/components/PostOriginalCreatedTimeFi import FieldError from '@/components/common/FieldError' import FieldWarning from '@/components/common/FieldWarning' import FormField from '@/components/common/FormField' -import PostImportStatusBadge from '@/components/posts/import/PostImportStatusBadge' import ThumbnailPreview from '@/components/posts/import/ThumbnailPreview' -import { effectivePostImportStatus } from '@/components/posts/import/postImportRowStatus' import { Button } from '@/components/ui/button' import { Dialog, DialogContent, @@ -17,7 +15,7 @@ import { inputClass } from '@/lib/utils' import type { FC } from 'react' import { useEffect, useState } from 'react' -import type { PostImportOrigin, +import type { PostImportResetSnapshot, PostImportRow } from '@/lib/postImportSession' type Draft = { @@ -36,37 +34,11 @@ type Props = { messageRow?: PostImportRow | null saving: boolean onOpenChange: (open: boolean) => void - onSave: (draft: Draft) => Promise } - -const originOf = ( - row: PostImportRow, - field: string, -): PostImportOrigin => - row.provenance[field] ?? 'automatic' - -const changedOrigin = ( - changed: boolean, - row: PostImportRow, - field: string, -): PostImportOrigin => - changed ? 'manual' : originOf (row, field) - -const originalCreatedOrigin = ( - row: PostImportRow, - originalDraft: Draft, - draft: Draft, -): PostImportOrigin => { - const changed = - originalDraft.originalCreatedFrom !== draft.originalCreatedFrom - || originalDraft.originalCreatedBefore !== draft.originalCreatedBefore - if (changed) - return 'manual' - - const manualOrigin = - originOf (row, 'originalCreatedFrom') === 'manual' - || originOf (row, 'originalCreatedBefore') === 'manual' - return manualOrigin ? 'manual' : 'automatic' -} + onSave: ( + payload: { draft: Draft + resetRequested: boolean + resetSnapshot: PostImportResetSnapshot }, + ) => Promise } const buildDraft = (row: PostImportRow): Draft => ({ url: row.url, @@ -108,9 +80,6 @@ const PostImportRowDialog: FC = ( return null const displayRow = messageRow ?? row - const originalDraft = buildDraft (row) - const fieldOrigin = (field: keyof Draft): PostImportOrigin => - changedOrigin (draft[field] !== originalDraft[field], row, field) const update = ( key: Key, @@ -121,7 +90,12 @@ const PostImportRowDialog: FC = ( } const save = async () => { - if (await onSave (draft)) + if ( + await onSave ({ + draft, + resetRequested: false, + resetSnapshot: row.resetSnapshot }) + ) onOpenChange (false) } @@ -142,16 +116,12 @@ const PostImportRowDialog: FC = ( -
- -
= ( = ( = ( )} onChange={value => update ('thumbnailBase', value)}/> - } originalCreatedFrom={draft.originalCreatedFrom || null} setOriginalCreatedFrom={value => update ('originalCreatedFrom', value ?? '')} originalCreatedBefore={draft.originalCreatedBefore || null} @@ -194,7 +158,6 @@ const PostImportRowDialog: FC = ( = ( = ( = (