From 638dccad6d03ab3907b5e3691b9e0b8523ef33f0 Mon Sep 17 00:00:00 2001 From: miteruzo Date: Tue, 26 May 2026 05:52:09 +0900 Subject: [PATCH 01/11] #90 --- .../app/controllers/application_controller.rb | 63 +++++++++++++++++++ .../app/controllers/deerjikists_controller.rb | 10 ++- .../app/controllers/materials_controller.rb | 10 +-- .../app/controllers/nico_tags_controller.rb | 6 +- backend/app/controllers/posts_controller.rb | 18 +++--- backend/app/controllers/preview_controller.rb | 9 +-- .../controllers/tag_children_controller.rb | 10 +-- backend/app/controllers/tags_controller.rb | 18 +++--- .../theatre_comments_controller.rb | 2 +- backend/app/controllers/users_controller.rb | 4 +- .../app/controllers/wiki_pages_controller.rb | 16 +++-- backend/spec/requests/error_responses_spec.rb | 47 ++++++++++++++ frontend/src/components/PostEditForm.tsx | 34 ++++++++-- frontend/src/components/PostFormTagsArea.tsx | 9 ++- .../PostOriginalCreatedTimeField.tsx | 10 ++- frontend/src/components/common/FieldError.tsx | 17 +++++ frontend/src/lib/apiErrors.ts | 30 +++++++++ frontend/src/pages/posts/PostDetailPage.tsx | 1 - 18 files changed, 259 insertions(+), 55 deletions(-) create mode 100644 backend/spec/requests/error_responses_spec.rb create mode 100644 frontend/src/components/common/FieldError.tsx create mode 100644 frontend/src/lib/apiErrors.ts diff --git a/backend/app/controllers/application_controller.rb b/backend/app/controllers/application_controller.rb index 4f3d6ed..c06c936 100644 --- a/backend/app/controllers/application_controller.rb +++ b/backend/app/controllers/application_controller.rb @@ -1,4 +1,7 @@ class ApplicationController < ActionController::API + rescue_from ActiveRecord::RecordInvalid, with: :render_record_invalid + rescue_from ActiveRecord::RecordNotUnique, with: :render_record_not_unique + before_action :reject_banned_ip_address! before_action :authenticate_user before_action :reject_banned_user! @@ -25,6 +28,42 @@ class ApplicationController < ActionController::API end end + def render_bad_request message = 'リクエストが不正です．', field: nil, code: :bad_request + render_error(:bad_request, message, field:, code:) + end + + def render_unprocessable_entity message = '入力を確認してください．', field: nil, code: :invalid + render_error(:unprocessable_entity, message, field:, code:) + end + + def render_error status, message, field: nil, code: status + error = { code: code.to_s, message: } + error[:field] = field.to_s if field.present? + + render json: { errors: [error] }, status: + end + + def render_model_errors record, status: :unprocessable_entity + errors = + record.errors.map do |error| + { code: error.type.to_s, + field: error.attribute.to_s, + message: error.full_message } + end + + errors = [{ code: 'invalid', message: '入力を確認してください．' }] if errors.empty? + + render json: { errors: }, status: + end + + def render_record_invalid error + render_model_errors(error.record) + end + + def render_record_not_unique _error = nil + render_unprocessable_entity('既に存在してゐます．', code: :taken) + end + def reject_banned_ip_address! ip_address = IpAddress.find_by(ip_address: IPAddr.new(request.remote_ip).hton) return unless ip_address&.banned? @@ -37,4 +76,28 @@ class ApplicationController < ActionController::API head :forbidden end + + def render_validation_error record = nil, fields: { }, base: [] + errors = { } + + if record + record.errors.messages.each do |attr, messages| + errors[attr] ||= [] + errors[attr].concat(messages) + end + end + + fields.each do |attr, messages| + errors[attr] ||= [] + errors[attr].concat(Array(messages)) + end + + base_errors = Array(base) - Array(errors.delete(:base)) + + render json: { type: 'validation_error', + message: '入力内容を確認してください．', + errors:, + base_errors: }, + status: :unprocessable_entity + end end diff --git a/backend/app/controllers/deerjikists_controller.rb b/backend/app/controllers/deerjikists_controller.rb index dff9d82..7a9cb75 100644 --- a/backend/app/controllers/deerjikists_controller.rb +++ b/backend/app/controllers/deerjikists_controller.rb @@ -2,7 +2,8 @@ class DeerjikistsController < ApplicationController def show platform = params[:platform].to_s.strip code = params[:code].to_s.strip - return head :bad_request if platform.blank? || code.blank? + return render_bad_request('platform は必須です．', field: :platform) if platform.blank? + return render_bad_request('code は必須です．', field: :code) if code.blank? deerjikist = Deerjikist .joins(:tag) @@ -22,7 +23,9 @@ class DeerjikistsController < ApplicationController platform = params[:platform].to_s.strip code = params[:code].to_s.strip tag_id = params[:tag_id].to_i - return head :bad_request if platform.blank? || code.blank? || tag_id <= 0 + return render_bad_request('platform は必須です．', field: :platform) if platform.blank? + return render_bad_request('code は必須です．', field: :code) if code.blank? + return render_bad_request('tag_id が不正です．', field: :tag_id) if tag_id <= 0 deerjikist = Deerjikist.find_or_initialize_by(platform:, code:).tap do |d| d.tag_id = tag_id @@ -38,7 +41,8 @@ class DeerjikistsController < ApplicationController platform = params[:platform].to_s.strip code = params[:code].to_s.strip - return head :bad_request if platform.blank? || code.blank? + return render_bad_request('platform は必須です．', field: :platform) if platform.blank? + return render_bad_request('code は必須です．', field: :code) if code.blank? Deerjikist.find([platform, code]).destroy! diff --git a/backend/app/controllers/materials_controller.rb b/backend/app/controllers/materials_controller.rb index d61a4b3..401fcc6 100644 --- a/backend/app/controllers/materials_controller.rb +++ b/backend/app/controllers/materials_controller.rb @@ -40,7 +40,8 @@ class MaterialsController < ApplicationController tag_name_raw = params[:tag].to_s.strip file = params[:file] url = params[:url].to_s.strip.presence - return head :bad_request if tag_name_raw.blank? || (file.blank? && url.blank?) + return render_bad_request('タグは必須です．', field: :tag) if tag_name_raw.blank? + return render_bad_request('ファイルまたは URL は必須です．') if file.blank? && url.blank? tag_name = TagName.find_undiscard_or_create_by!(name: tag_name_raw) tag = tag_name.tag @@ -54,7 +55,7 @@ class MaterialsController < ApplicationController if material.save render json: MaterialRepr.base(material, host: request.base_url), status: :created else - render json: { errors: material.errors.full_messages }, status: :unprocessable_entity + render_model_errors(material) end end @@ -68,7 +69,8 @@ class MaterialsController < ApplicationController tag_name_raw = params[:tag].to_s.strip file = params[:file] url = params[:url].to_s.strip.presence - return head :bad_request if tag_name_raw.blank? || (file.blank? && url.blank?) + return render_bad_request('タグは必須です．', field: :tag) if tag_name_raw.blank? + return render_bad_request('ファイルまたは URL は必須です．') if file.blank? && url.blank? tag_name = TagName.find_undiscard_or_create_by!(name: tag_name_raw) tag = tag_name.tag @@ -84,7 +86,7 @@ class MaterialsController < ApplicationController if material.save render json: MaterialRepr.base(material, host: request.base_url) else - render json: { errors: material.errors.full_messages }, status: :unprocessable_entity + render_model_errors(material) end end diff --git a/backend/app/controllers/nico_tags_controller.rb b/backend/app/controllers/nico_tags_controller.rb index 087c592..55c54a4 100644 --- a/backend/app/controllers/nico_tags_controller.rb +++ b/backend/app/controllers/nico_tags_controller.rb @@ -30,12 +30,14 @@ class NicoTagsController < ApplicationController id = params[:id].to_i tag = Tag.find(id) - return head :bad_request unless tag.nico? + return render_bad_request('ニコニコ・タグを指定してください．', field: :id) unless tag.nico? linked_tag_names = params[:tags].to_s.split linked_tags = Tag.normalise_tags!(linked_tag_names, with_tagme: false, with_no_deerjikist: false) - return head :bad_request if linked_tags.any? { |t| t.nico? } + if linked_tags.any? { |t| t.nico? } + return render_bad_request('ニコニコ・タグ同士は連携できません．', field: :tags) + end ApplicationRecord.transaction do TagVersioning.record_tag_snapshots!(linked_tags, created_by_user: current_user) diff --git a/backend/app/controllers/posts_controller.rb b/backend/app/controllers/posts_controller.rb index e8cae00..1a982bf 100644 --- a/backend/app/controllers/posts_controller.rb +++ b/backend/app/controllers/posts_controller.rb @@ -148,11 +148,11 @@ class PostsController < ApplicationController post.reload render json: PostRepr.base(post), status: :created rescue Tag::NicoTagNormalisationError - head :bad_request + render_bad_request('ニコニコ・タグは直接指定できません．', field: :tags) rescue ArgumentError => e - render json: { errors: [e.message] }, status: :unprocessable_entity + render_unprocessable_entity(e.message) rescue ActiveRecord::RecordInvalid => e - render json: { errors: e.record.errors.full_messages }, status: :unprocessable_entity + render_model_errors(e.record) end def viewed @@ -175,10 +175,10 @@ class PostsController < ApplicationController force = bool?(:force) merge = bool?(:merge) - return head :bad_request if force && merge + return render_bad_request('force と merge は同時に指定できません．') if force && merge base_version_no = parse_base_version_no - return head :bad_request if !(force) && !(base_version_no) + return render_bad_request('base_version_no は必須です．', field: :base_version_no) if !(force) && !(base_version_no) title = params[:title].presence tag_names = params[:tags].to_s.split @@ -238,11 +238,11 @@ class PostsController < ApplicationController json['tags'] = build_tag_tree_for(post.tags) render json:, status: :ok rescue Tag::NicoTagNormalisationError - head :bad_request + render_bad_request('ニコニコ・タグは直接指定できません．', field: :tags) rescue ArgumentError => e - render json: { errors: [e.message] }, status: :unprocessable_entity + render_validation_error(fields: { parent_post_ids: [e.message] }) rescue ActiveRecord::RecordInvalid => e - render json: { errors: e.record.errors.full_messages }, status: :unprocessable_entity + render_validation_error(e.record) end def changes @@ -416,7 +416,7 @@ class PostsController < ApplicationController def sync_parent_posts! post, parent_post_ids if parent_post_ids.include?(post.id) - post.errors.add(:base, '自分自身を親投稿にはできません．') + post.errors.add(:parent_post_ids, '自分自身を親投稿にはできません．') raise ActiveRecord::RecordInvalid, post end diff --git a/backend/app/controllers/preview_controller.rb b/backend/app/controllers/preview_controller.rb index 0590891..24d2a2d 100644 --- a/backend/app/controllers/preview_controller.rb +++ b/backend/app/controllers/preview_controller.rb @@ -4,7 +4,7 @@ class PreviewController < ApplicationController return head :unauthorized unless current_user url = params[:url] - return head :bad_request unless url.present? + return render_bad_request('URL は必須です．', field: :url) unless url.present? unless url.start_with?(/http(s)?:\/\//) url = 'http://' + url @@ -16,7 +16,7 @@ class PreviewController < ApplicationController render json: { title: title } rescue => e - render json: { error: e.message }, status: :bad_request + render_bad_request(e.message, field: :url) end def thumbnail @@ -25,7 +25,7 @@ class PreviewController < ApplicationController return head :unauthorized unless current_user url = params[:url] - return head :bad_request if url.blank? + return render_bad_request('URL は必須です．', field: :url) if url.blank? unless url.start_with?(/http(s)?:\/\//) url = 'http://' + url @@ -40,7 +40,8 @@ class PreviewController < ApplicationController File.delete(path) rescue nil send_file image.path, type: 'image/png', disposition: 'inline' else - render json: { error: 'Failed to generate thumbnail' }, status: :internal_server_error + render_error(:internal_server_error, 'サムネールを生成できませんでした．', + code: :thumbnail_generation_failed) end end end diff --git a/backend/app/controllers/tag_children_controller.rb b/backend/app/controllers/tag_children_controller.rb index 8eb972e..ee4300f 100644 --- a/backend/app/controllers/tag_children_controller.rb +++ b/backend/app/controllers/tag_children_controller.rb @@ -5,11 +5,12 @@ class TagChildrenController < ApplicationController parent_id = params[:parent_id] child_id = params[:child_id] - return head :bad_request if parent_id.blank? || child_id.blank? + return render_bad_request('parent_id は必須です．', field: :parent_id) if parent_id.blank? + return render_bad_request('child_id は必須です．', field: :child_id) if child_id.blank? parent = Tag.find(parent_id) child = Tag.find(child_id) - return head :bad_request if parent.nico? || child.nico? + return render_bad_request('ニコニコ・タグの階層は変更できません．') if parent.nico? || child.nico? ApplicationRecord.transaction do TagVersioning.ensure_snapshot!(child, created_by_user: current_user) @@ -27,11 +28,12 @@ class TagChildrenController < ApplicationController parent_id = params[:parent_id] child_id = params[:child_id] - return head :bad_request if parent_id.blank? || child_id.blank? + return render_bad_request('parent_id は必須です．', field: :parent_id) if parent_id.blank? + return render_bad_request('child_id は必須です．', field: :child_id) if child_id.blank? parent = Tag.find(parent_id) child = Tag.find(child_id) - return head :bad_request if parent.nico? || child.nico? + return render_bad_request('ニコニコ・タグの階層は変更できません．') if parent.nico? || child.nico? ApplicationRecord.transaction do TagVersioning.ensure_snapshot!(child, created_by_user: current_user) diff --git a/backend/app/controllers/tags_controller.rb b/backend/app/controllers/tags_controller.rb index aed029f..89911de 100644 --- a/backend/app/controllers/tags_controller.rb +++ b/backend/app/controllers/tags_controller.rb @@ -168,7 +168,7 @@ class TagsController < ApplicationController def show_by_name name = params[:name].to_s.strip - return head :bad_request if name.blank? + return render_bad_request('name は必須です．', field: :name) if name.blank? tag = Tag.joins(:tag_name) .includes(:tag_name, :materials, tag_name: :wiki_page) @@ -192,7 +192,7 @@ class TagsController < ApplicationController def deerjikists_by_name name = params[:name].to_s.strip - return head :bad_request if name.blank? + return render_bad_request('name は必須です．', field: :name) if name.blank? tag = Tag.joins(:tag_name) .includes(:tag_name, tag_name: :wiki_page) @@ -228,7 +228,7 @@ class TagsController < ApplicationController def materials_by_name name = params[:name].to_s.strip - return head :bad_request if name.blank? + return render_bad_request('name は必須です．', field: :name) if name.blank? tag = Tag.joins(:tag_name) .includes(:tag_name, :materials, tag_name: :wiki_page) @@ -247,17 +247,16 @@ class TagsController < ApplicationController name = params[:name].to_s.strip category = params[:category].to_s.strip - return head :unprocessable_entity if name.blank? || category.blank? + return render_unprocessable_entity('名前は必須です．', field: :name) if name.blank? + return render_unprocessable_entity('カテゴリは必須です．', field: :category) if category.blank? if name != tag.name && tag.in?([Tag.tagme, Tag.bot, Tag.no_deerjikist, Tag.video, Tag.niconico]) - return render json: { error: 'システム・タグの名称は変更できません．' }, - status: :unprocessable_entity + return render_unprocessable_entity('システム・タグの名称は変更できません．', field: :name) end if tag.nico? || category == 'nico' - return render json: { error: 'ニコタグは変更できません．' }, - status: :unprocessable_entity + return render_unprocessable_entity('ニコタグは変更できません．', field: :category) end alias_names = params[:aliases].to_s.split.uniq @@ -302,8 +301,7 @@ class TagsController < ApplicationController tag = Tag.find(params[:id]) if tag.nico? || (category.present? && category == 'nico') - return render json: { error: 'ニコタグは変更できません．' }, - status: :unprocessable_entity + return render_unprocessable_entity('ニコタグは変更できません．', field: :category) end ApplicationRecord.transaction do diff --git a/backend/app/controllers/theatre_comments_controller.rb b/backend/app/controllers/theatre_comments_controller.rb index 50ec9ef..fdc7ba1 100644 --- a/backend/app/controllers/theatre_comments_controller.rb +++ b/backend/app/controllers/theatre_comments_controller.rb @@ -15,7 +15,7 @@ class TheatreCommentsController < ApplicationController return head :unauthorized unless current_user content = params[:content] - return head :unprocessable_entity if content.blank? + return render_unprocessable_entity('本文は必須です．', field: :content) if content.blank? theatre = Theatre.find_by(id: params[:theatre_id]) return head :not_found unless theatre diff --git a/backend/app/controllers/users_controller.rb b/backend/app/controllers/users_controller.rb index a144593..b1d0299 100644 --- a/backend/app/controllers/users_controller.rb +++ b/backend/app/controllers/users_controller.rb @@ -42,12 +42,12 @@ class UsersController < ApplicationController return head :unauthorized if user&.id != params[:id].to_i name = params[:name] - return head :bad_request if name.blank? + return render_bad_request('名前は必須です．', field: :name) if name.blank? if user.update(name:) render json: user.slice(:id, :name, :inheritance_code, :role), status: :ok else - render json: user.errors, status: :unprocessable_entity + render_model_errors(user) end end diff --git a/backend/app/controllers/wiki_pages_controller.rb b/backend/app/controllers/wiki_pages_controller.rb index de42c52..ef2250a 100644 --- a/backend/app/controllers/wiki_pages_controller.rb +++ b/backend/app/controllers/wiki_pages_controller.rb @@ -46,7 +46,7 @@ class WikiPagesController < ApplicationController def diff id = params[:id] - return head :bad_request if id.blank? + return render_bad_request('id は必須です．', field: :id) if id.blank? from = params[:from].presence to = params[:to].presence @@ -56,7 +56,7 @@ class WikiPagesController < ApplicationController from_rev = from && page.wiki_revisions.find(from) to_rev = to ? page.wiki_revisions.find(to) : page.current_revision if ((from_rev && !(from_rev.content?)) || !(to_rev&.content?)) - return head :unprocessable_entity + return render_unprocessable_entity('差分を表示できない版です．') end diffs = Diff::LCS.sdiff(from_rev&.body&.lines || [], to_rev.body.lines) @@ -89,7 +89,8 @@ class WikiPagesController < ApplicationController body = params[:body].to_s message = params[:message].presence - return head :unprocessable_entity if title.blank? || body.blank? + return render_unprocessable_entity('タイトルは必須です．', field: :title) if title.blank? + return render_unprocessable_entity('本文は必須です．', field: :body) if body.blank? tag_name = TagName.find_undiscard_or_create_by!(name: title) @@ -101,8 +102,10 @@ class WikiPagesController < ApplicationController message:) render json: WikiPageRepr.base(page), status: :created - rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotUnique - head :unprocessable_entity + rescue ActiveRecord::RecordInvalid => e + render_model_errors(e.record) + rescue ActiveRecord::RecordNotUnique + render_record_not_unique end def update @@ -112,7 +115,8 @@ class WikiPagesController < ApplicationController title = params[:title]&.strip body = params[:body].to_s - return head :unprocessable_entity if title.blank? || body.blank? + return render_unprocessable_entity('タイトルは必須です．', field: :title) if title.blank? + return render_unprocessable_entity('本文は必須です．', field: :body) if body.blank? page = WikiPage.find(params[:id]) base_revision_id = params[:base_revision_id].presence diff --git a/backend/spec/requests/error_responses_spec.rb b/backend/spec/requests/error_responses_spec.rb new file mode 100644 index 0000000..4fdc4d6 --- /dev/null +++ b/backend/spec/requests/error_responses_spec.rb @@ -0,0 +1,47 @@ +require 'rails_helper' + +RSpec.describe 'error responses', type: :request do + describe 'manual input errors' do + it 'returns a stable errors array for bad requests' do + user = create(:user) + sign_in_as(user) + + put "/users/#{ user.id }", params: { name: ' ' } + + expect(response).to have_http_status(:bad_request) + expect(json.fetch('errors')).to contain_exactly( + include('code' => 'bad_request', + 'field' => 'name', + 'message' => be_present)) + end + + it 'returns a stable errors array for unprocessable requests' do + member = create(:user, :member) + tag = create(:tag, :general, name: 'error_response_tag') + sign_in_as(member) + + patch "/tags/#{ tag.id }", params: { category: 'nico' } + + expect(response).to have_http_status(:unprocessable_entity) + expect(json.fetch('errors')).to contain_exactly( + include('code' => 'invalid', + 'field' => 'category', + 'message' => be_present)) + end + end + + describe 'model validation errors' do + it 'returns field, code, and message for model errors' do + user = create(:user) + sign_in_as(user) + + put "/users/#{ user.id }", params: { name: 'a' * 256 } + + expect(response).to have_http_status(:unprocessable_entity) + expect(json.fetch('errors')).to include( + include('code' => 'too_long', + 'field' => 'name', + 'message' => be_present)) + end + end +end diff --git a/frontend/src/components/PostEditForm.tsx b/frontend/src/components/PostEditForm.tsx index bc1048e..0d8ff44 100644 --- a/frontend/src/components/PostEditForm.tsx +++ b/frontend/src/components/PostEditForm.tsx @@ -2,17 +2,23 @@ import { useEffect, useState } from 'react' import PostFormTagsArea from '@/components/PostFormTagsArea' import PostOriginalCreatedTimeField from '@/components/PostOriginalCreatedTimeField' +import FieldError from '@/components/common/FieldError' import Label from '@/components/common/Label' import { useDialogue } from '@/components/dialogues/DialogueProvider' import { Button } from '@/components/ui/button' import { toast } from '@/components/ui/use-toast' import { isApiError } from '@/lib/api' +import { extractValidationError } from '@/lib/apiErrors' import { updatePost } from '@/lib/posts' import type { FC, FormEvent } from 'react' +import type { FieldErrors } from '@/lib/apiErrors' import type { Post, Tag } from '@/types' +type PostFormField = + 'parentPostId' | 'tags' | 'originalCreatedFrom' | 'originalCreatedBefore' + const tagsToStr = (tags: Tag[]): string => { const result: Tag[] = [] @@ -34,7 +40,9 @@ type Props = { post: Post const PostEditForm: FC = ({ post, onSave }) => { + const [baseErrors, setBaseErrors] = useState ([]) const [disabled, setDisabled] = useState (false) + const [fieldErrors, setFieldErrors] = useState ({ }) const [originalCreatedBefore, setOriginalCreatedBefore] = useState (post.originalCreatedBefore) const [originalCreatedFrom, setOriginalCreatedFrom] = @@ -47,6 +55,9 @@ const PostEditForm: FC = ({ post, onSave }) => { const dialogue = useDialogue () const update = async (...args: Parameters) => { + setFieldErrors ({ }) + setBaseErrors ([]) + try { const data = await updatePost (...args) @@ -67,7 +78,18 @@ const PostEditForm: FC = ({ post, onSave }) => { if (response?.status !== 409) { - toast ({ description: '更新はできなかったよ……' }) + const validationError = extractValidationError (e) + + if (validationError) + { + setFieldErrors (validationError.fieldErrors) + setBaseErrors (validationError.baseErrors) + toast ({ description: '更新はできなかったよ……' }) + return + } + + toast ({ title: '失敗……', description: '入力を確認してください．' }) + return } @@ -142,13 +164,15 @@ const PostEditForm: FC = ({ post, onSave }) => { value={parentPostIds} onChange={e => setParentPostIds (e.target.value)} className="w-full border p-2 rounded"/> + {/* タグ */} + setTags={setTags} + errors={fieldErrors.tags}/> {/* オリジナルの作成日時 */} = ({ post, onSave }) => { originalCreatedFrom={originalCreatedFrom} setOriginalCreatedFrom={setOriginalCreatedFrom} originalCreatedBefore={originalCreatedBefore} - setOriginalCreatedBefore={setOriginalCreatedBefore}/> + setOriginalCreatedBefore={setOriginalCreatedBefore} + fromErrors={fieldErrors.originalCreatedFrom} + beforeErrors={fieldErrors.originalCreatedBefore}/> {/* 送信 */} + +
= ({ disabled,
+ ) -export default PostOriginalCreatedTimeField \ No newline at end of file +export default PostOriginalCreatedTimeField diff --git a/frontend/src/components/common/FieldError.tsx b/frontend/src/components/common/FieldError.tsx new file mode 100644 index 0000000..ce2ab68 --- /dev/null +++ b/frontend/src/components/common/FieldError.tsx @@ -0,0 +1,17 @@ +import type { FC } from 'react' + +type Props = { messages?: string[] } + + +export const FieldError: FC = ({ messages }: Props) => { + if (!(messages?.length)) + return null + + return ( +
    + {messages.map ((message, i) =>
  • {message}
    • )} +
) +} + + +export default FieldError diff --git a/frontend/src/lib/apiErrors.ts b/frontend/src/lib/apiErrors.ts new file mode 100644 index 0000000..c71b2fc --- /dev/null +++ b/frontend/src/lib/apiErrors.ts @@ -0,0 +1,30 @@ +import toCamel from 'camelcase-keys' + +import { isApiError } from '@/lib/api' + +export type FieldErrors = Partial> + +export type ValidationError = { message: string + fieldErrors: FieldErrors + baseErrors: string[] } + +type RawValidationError = { type?: string + message?: string + errors?: Record + baseErrors?: string[] } + + +export const extractValidationError = (err: unknown) => { + if (!(isApiError (err)) || err.response?.status !== 422) + return null + + const data = toCamel ((err.response.data ?? { }) as Record, + { deep: true }) as RawValidationError + + if (data.type !== 'validation_error' && !(data.errors)) + return null + + return { message: data.message ?? '入力内容を確認してください．', + fieldErrors: (data.errors ?? { }) as FieldErrors, + baseErrors: data.baseErrors ?? [] } +} diff --git a/frontend/src/pages/posts/PostDetailPage.tsx b/frontend/src/pages/posts/PostDetailPage.tsx index 906c5cb..9472ee1 100644 --- a/frontend/src/pages/posts/PostDetailPage.tsx +++ b/frontend/src/pages/posts/PostDetailPage.tsx @@ -3,7 +3,6 @@ import { motion } from 'framer-motion' import { useEffect, useRef, useState } from 'react' import { Helmet } from 'react-helmet-async' import { useParams } from 'react-router-dom' - import PostEditForm from '@/components/PostEditForm' import PostEmbed from '@/components/PostEmbed' import PostList from '@/components/PostList' -- 2.34.1 From ff5e8c4d49d69e19c6bf0a4fc846aebe4de4a8e8 Mon Sep 17 00:00:00 2001 From: miteruzo Date: Wed, 3 Jun 2026 07:25:24 +0900 Subject: [PATCH 02/11] #90 --- .../app/controllers/application_controller.rb | 33 +++------ backend/app/controllers/posts_controller.rb | 71 ++++++++++++++----- backend/app/models/post.rb | 2 +- backend/app/representations/post_repr.rb | 57 +++++++++++++-- backend/app/representations/tag_repr.rb | 4 ++ backend/spec/requests/posts_spec.rb | 70 ++++++++++++++++++ frontend/src/components/PostEditForm.tsx | 16 +++-- frontend/src/components/PostFormTagsArea.tsx | 3 +- .../PostOriginalCreatedTimeField.tsx | 25 ++++--- frontend/src/components/common/FieldError.tsx | 2 +- frontend/src/components/common/Label.tsx | 35 +++++---- frontend/src/components/common/TextArea.tsx | 22 +++++- frontend/src/lib/users.test.ts | 20 ++++++ frontend/src/lib/users.ts | 7 ++ frontend/src/lib/utils.ts | 12 ++++ frontend/src/pages/posts/PostDetailPage.tsx | 4 +- frontend/src/pages/posts/PostNewPage.tsx | 3 +- frontend/src/pages/tags/NicoTagListPage.tsx | 7 +- frontend/src/pages/wiki/WikiEditPage.tsx | 3 +- frontend/src/pages/wiki/WikiNewPage.tsx | 3 +- 20 files changed, 311 insertions(+), 88 deletions(-) create mode 100644 frontend/src/lib/users.test.ts create mode 100644 frontend/src/lib/users.ts diff --git a/backend/app/controllers/application_controller.rb b/backend/app/controllers/application_controller.rb index c06c936..39a4465 100644 --- a/backend/app/controllers/application_controller.rb +++ b/backend/app/controllers/application_controller.rb @@ -43,25 +43,12 @@ class ApplicationController < ActionController::API render json: { errors: [error] }, status: end - def render_model_errors record, status: :unprocessable_entity - errors = - record.errors.map do |error| - { code: error.type.to_s, - field: error.attribute.to_s, - message: error.full_message } - end - - errors = [{ code: 'invalid', message: '入力を確認してください．' }] if errors.empty? - - render json: { errors: }, status: - end - def render_record_invalid error - render_model_errors(error.record) + render_validation_error error.record end def render_record_not_unique _error = nil - render_unprocessable_entity('既に存在してゐます．', code: :taken) + render_validation_error base: ['すでに存在してゐます．'] end def reject_banned_ip_address! @@ -77,27 +64,27 @@ class ApplicationController < ActionController::API head :forbidden end - def render_validation_error record = nil, fields: { }, base: [] + def render_validation_error record = nil, fields: { }, base: [], status: :unprocessable_entity errors = { } if record - record.errors.messages.each do |attr, messages| - errors[attr] ||= [] - errors[attr].concat(messages) + record.errors.each do |error| + errors[error.attribute] ||= [] + errors[error.attribute] << error.message end end fields.each do |attr, messages| - errors[attr] ||= [] - errors[attr].concat(Array(messages)) + errors[attr.to_sym] ||= [] + errors[attr.to_sym].concat(Array(messages)) end - base_errors = Array(base) - Array(errors.delete(:base)) + base_errors = Array(base) + Array(errors.delete(:base)) render json: { type: 'validation_error', message: '入力内容を確認してください．', errors:, base_errors: }, - status: :unprocessable_entity + status: end end diff --git a/backend/app/controllers/posts_controller.rb b/backend/app/controllers/posts_controller.rb index 1a982bf..daecc0b 100644 --- a/backend/app/controllers/posts_controller.rb +++ b/backend/app/controllers/posts_controller.rb @@ -44,7 +44,7 @@ class PostsController < ApplicationController filtered_posts .joins("LEFT JOIN (#{ pt_max_sql }) pt_max ON pt_max.post_id = posts.id") .reselect('posts.*', Arel.sql("#{ updated_at_all_sql } AS updated_at_all")) - .preload(tags: [:deerjikists, :materials, { tag_name: :wiki_page }]) + .preload(:uploaded_user, tags: [:deerjikists, :materials, { tag_name: :wiki_page }]) .with_attached_thumbnail q = q.where('posts.url LIKE ?', "%#{ url }%") if url @@ -95,7 +95,9 @@ class PostsController < ApplicationController end def random - post = filtered_posts.preload(tags: [:deerjikists, :materials, { tag_name: :wiki_page }]) + post = filtered_posts.preload(:uploaded_user, + tags: [:deerjikists, :materials, { tag_name: :wiki_page }]) + .with_attached_thumbnail .order('RAND()') .first return head :not_found unless post @@ -104,12 +106,24 @@ class PostsController < ApplicationController end def show - post = Post.includes(tags: [:deerjikists, :materials, { tag_name: :wiki_page }]).find_by(id: params[:id]) + post = + Post + .includes(:uploaded_user, tags: [:deerjikists, :materials, { tag_name: :wiki_page }]) + .with_attached_thumbnail + .find_by(id: params[:id]) return head :not_found unless post - render json: PostRepr.base(post, current_user) - .merge(tags: build_tag_tree_for(post.tags), - related: PostRepr.many(post.related(limit: 20))) + parent_posts = post.parents.with_attached_thumbnail.order(:id).to_a + child_posts = post.children.with_attached_thumbnail.order(:id).to_a + sibling_posts = sibling_posts_by_parent(parent_posts.map(&:id)) + related = post.related(limit: 20).to_a + + render json: PostRepr.detail(post, current_user, + parent_posts:, + child_posts:, + sibling_posts:, + related:) + .merge(tags: build_tag_tree_for(post.tags)) end def create @@ -148,11 +162,11 @@ class PostsController < ApplicationController post.reload render json: PostRepr.base(post), status: :created rescue Tag::NicoTagNormalisationError - render_bad_request('ニコニコ・タグは直接指定できません．', field: :tags) + render_validation_error fields: { tags: 'ニコニコ・タグは直接指定できません．' } rescue ArgumentError => e - render_unprocessable_entity(e.message) + render_validation_error fields: { parent_post_ids: [e.message] } rescue ActiveRecord::RecordInvalid => e - render_model_errors(e.record) + render_post_form_record_invalid e.record end def viewed @@ -238,11 +252,11 @@ class PostsController < ApplicationController json['tags'] = build_tag_tree_for(post.tags) render json:, status: :ok rescue Tag::NicoTagNormalisationError - render_bad_request('ニコニコ・タグは直接指定できません．', field: :tags) + render_validation_error fields: { tags: ['ニコニコ・タグは直接指定できません．'] } rescue ArgumentError => e - render_validation_error(fields: { parent_post_ids: [e.message] }) + render_validation_error fields: { parent_post_ids: [e.message] } rescue ActiveRecord::RecordInvalid => e - render_validation_error(e.record) + render_post_form_record_invalid e.record end def changes @@ -385,7 +399,7 @@ class PostsController < ApplicationController return nil unless tag if path.include?(tag_id) - return TagRepr.base(tag).merge(children: []) + return TagRepr.inline(tag).merge(children: []) end if memo.key?(tag_id) @@ -397,12 +411,26 @@ class PostsController < ApplicationController children = child_ids.filter_map { |cid| build_node.(cid, new_path) } - memo[tag_id] = TagRepr.base(tag).merge(children:) + memo[tag_id] = TagRepr.inline(tag).merge(children:) end root_ids.filter_map { |id| build_node.call(id, []) } end + def sibling_posts_by_parent parent_post_ids + return { } if parent_post_ids.blank? + + implications = + PostImplication + .where(parent_post_id: parent_post_ids) + .includes(post: { thumbnail_attachment: :blob }) + .order(:parent_post_id, :post_id) + + implications.group_by(&:parent_post_id).transform_values { |items| + items.map(&:post) + } + end + def parse_parent_post_ids raise ArgumentError, 'parent_post_ids は必須です．' unless params.key?(:parent_post_ids) @@ -416,7 +444,7 @@ class PostsController < ApplicationController def sync_parent_posts! post, parent_post_ids if parent_post_ids.include?(post.id) - post.errors.add(:parent_post_ids, '自分自身を親投稿にはできません．') + post.errors.add :parent_post_ids, '自分自身を親投稿にはできません．' raise ActiveRecord::RecordInvalid, post end @@ -424,7 +452,8 @@ class PostsController < ApplicationController missing_ids = parent_post_ids - existing_ids if missing_ids.present? - post.errors.add(:base, "存在しない親投稿 ID があります: #{ missing_ids.join(' ') }") + post.errors.add :parent_post_ids, + "存在しない親投稿 ID があります: #{ missing_ids.join(' ') }" raise ActiveRecord::RecordInvalid, post end @@ -640,4 +669,14 @@ class PostsController < ApplicationController merged.uniq.sort end + + def render_post_form_record_invalid record + if e.record.is_a?(TagName) || e.record.is_a?(Tag) + render_validation_error(fields: { tags: e.record.errors.full_messages.map { |message| + "タグ名 “#{ e.record.name }”: #{ message }" + } }) + else + render_validation_error(record) + end + end end diff --git a/backend/app/models/post.rb b/backend/app/models/post.rb index 4c3ddb1..2da8b02 100644 --- a/backend/app/models/post.rb +++ b/backend/app/models/post.rb @@ -94,7 +94,7 @@ class Post < ApplicationRecord return if !(f) || !(b) if f >= b - errors.add :original_created_before, 'オリジナルの作成日時の順番がをかしぃです．' + errors.add :original_created_at, 'オリジナルの作成日時の順番がをかしぃです．' end end diff --git a/backend/app/representations/post_repr.rb b/backend/app/representations/post_repr.rb index 87f59f9..e4f8198 100644 --- a/backend/app/representations/post_repr.rb +++ b/backend/app/representations/post_repr.rb @@ -2,20 +2,65 @@ module PostRepr - BASE = { include: { tags: TagRepr::BASE, uploaded_user: UserRepr::BASE }, - methods: [:parent_posts, :child_posts, :sibling_posts] }.freeze + BASE_FIELDS = [ + :id, + :version_no, + :url, + :title, + :thumbnail_base, + :original_created_from, + :original_created_before, + :created_at, + :updated_at + ].freeze module_function def base post, current_user = nil - json = post.as_json(BASE) - return json.merge(viewed: false) unless current_user + json = common(post) + json['tags'] = tag_json(post.tags) + json['uploaded_user'] = post.uploaded_user && UserRepr.base(post.uploaded_user) + json['viewed'] = current_user ? current_user.viewed?(post) : false + json + end - viewed = current_user.viewed?(post) - json.merge(viewed:) + def detail post, current_user = nil, parent_posts: [], child_posts: [], + sibling_posts: { }, related: [] + base(post, current_user).merge( + 'parent_posts' => cards(parent_posts), + 'child_posts' => cards(child_posts), + 'sibling_posts' => sibling_posts.transform_keys(&:to_s).transform_values { |posts| + cards(posts) + }, + 'related' => cards(related)) + end + + def card post + common(post).merge('parent_posts' => [], 'child_posts' => []) + end + + def cards posts + posts.map { |post| card(post) } end def many posts, current_user = nil posts.map { |p| base(p, current_user) } end + + def common post + BASE_FIELDS.to_h { |field| [field.to_s, post.public_send(field)] } + .merge('thumbnail' => thumbnail_url(post)) + end + + def tag_json tags + tags.map { |tag| TagRepr.inline(tag) } + end + + def thumbnail_url post + return nil unless post.thumbnail.attached? + + Rails.application.routes.url_helpers.rails_blob_url(post.thumbnail, only_path: false) + rescue + nil + end end diff --git a/backend/app/representations/tag_repr.rb b/backend/app/representations/tag_repr.rb index 705c096..28be332 100644 --- a/backend/app/representations/tag_repr.rb +++ b/backend/app/representations/tag_repr.rb @@ -12,5 +12,9 @@ module TagRepr parents: tag.parents.map { _1.as_json(BASE) }) end + def inline tag + tag.as_json(BASE).merge(aliases: [], parents: []) + end + def many(tags) = tags.map { |t| base(t) } end diff --git a/backend/spec/requests/posts_spec.rb b/backend/spec/requests/posts_spec.rb index 14b51e7..08329d7 100644 --- a/backend/spec/requests/posts_spec.rb +++ b/backend/spec/requests/posts_spec.rb @@ -57,6 +57,23 @@ RSpec.describe 'Posts API', type: :request do post_write_params({ base_version_no: base_version.version_no }.merge(params)) end + def count_sql_queries + count = 0 + + callback = lambda do |_name, _started, _finished, _id, payload| + next if payload[:cached] + next if ['SCHEMA', 'TRANSACTION'].include?(payload[:name]) + + count += 1 + end + + ActiveSupport::Notifications.subscribed(callback, 'sql.active_record') do + yield + end + + count + end + let!(:tag_name) { TagName.create!(name: 'spec_tag') } let!(:tag) { Tag.create!(tag_name: tag_name, category: :general) } @@ -558,6 +575,59 @@ RSpec.describe 'Posts API', type: :request do expect(sibling_ids).to include(sibling_post.id) end end + + it 'does not issue a query per tag or related post' do + user = create_member_user! + + tags = + 15.times.map do |i| + tag_name = TagName.create!(name: "show_query_tag_#{ i }") + tag = Tag.create!(tag_name:, category: :general) + TagName.create!(name: "show_query_alias_#{ i }", canonical: tag_name) + PostTag.create!(post: post_record, tag:) + tag + end + + tags.each_cons(2) do |parent_tag, child_tag| + TagImplication.create!(parent_tag:, tag: child_tag) + end + + parent_post = Post.create!( + title: 'query parent post', + url: 'https://example.com/query-parent-post' + ) + sibling_post = Post.create!( + title: 'query sibling post', + url: 'https://example.com/query-sibling-post' + ) + child_post = Post.create!( + title: 'query child post', + url: 'https://example.com/query-child-post' + ) + + PostImplication.create!(post: post_record, parent_post:) + PostImplication.create!(post: sibling_post, parent_post:) + PostImplication.create!(post: child_post, parent_post: post_record) + + 20.times do |i| + related_post = Post.create!( + title: "query related post #{ i }", + url: "https://example.com/query-related-post-#{ i }" + ) + PostSimilarity.create!(post: post_record, + target_post: related_post, + cos: 1.0 - (i / 100.0)) + end + + query_count = + count_sql_queries do + get "/posts/#{ post_record.id }", + headers: { 'X-Transfer-Code' => user.inheritance_code } + end + + expect(response).to have_http_status(:ok) + expect(query_count).to be <= 45 + end end context 'when post does not exist' do diff --git a/frontend/src/components/PostEditForm.tsx b/frontend/src/components/PostEditForm.tsx index 0d8ff44..4bc9f1b 100644 --- a/frontend/src/components/PostEditForm.tsx +++ b/frontend/src/components/PostEditForm.tsx @@ -10,6 +10,7 @@ import { toast } from '@/components/ui/use-toast' import { isApiError } from '@/lib/api' import { extractValidationError } from '@/lib/apiErrors' import { updatePost } from '@/lib/posts' +import { inputClass } from '@/lib/utils' import type { FC, FormEvent } from 'react' @@ -150,21 +151,25 @@ const PostEditForm: FC = ({ post, onSave }) => { setTitle (ev.target.value)}/> {/* 親投稿 */}
- + setParentPostIds (e.target.value)} - className="w-full border p-2 rounded"/> - + alia-invalid={fieldErrors.parentPostIds && fieldErrors.parentPostIds.length > 0} + className={inputClass (fieldErrors.parentPostIds + && fieldErrors.parentPostIds.length > 0)}/> +
{/* タグ */} @@ -181,8 +186,7 @@ const PostEditForm: FC = ({ post, onSave }) => { setOriginalCreatedFrom={setOriginalCreatedFrom} originalCreatedBefore={originalCreatedBefore} setOriginalCreatedBefore={setOriginalCreatedBefore} - fromErrors={fieldErrors.originalCreatedFrom} - beforeErrors={fieldErrors.originalCreatedBefore}/> + errors={fieldErrors.originalCreatedAt}/> {/* 送信 */}