From c897ea8f8fe216391a561e940d043836320ed9a2 Mon Sep 17 00:00:00 2001 From: Codex Night Worker Date: Sat, 14 Nov 2026 10:55:41 +0900 Subject: [PATCH] AI: address review on PR #477 --- .../tag_descriptions_controller.rb | 65 +++++++++++++++++- .../app/controllers/tag_groups_controller.rb | 63 +++++++++++++++-- .../app/controllers/wiki_pages_controller.rb | 39 ++--------- backend/app/representations/tag_group_repr.rb | 15 ++++ .../app/services/tag_description_operation.rb | 31 +++++++++ backend/config/routes.rb | 9 ++- frontend/src/lib/wiki.ts | 68 +++++++++++++++---- frontend/src/pages/wiki/WikiEditPage.tsx | 23 +++---- frontend/src/pages/wiki/WikiNewPage.tsx | 15 ++-- 9 files changed, 250 insertions(+), 78 deletions(-) diff --git a/backend/app/controllers/tag_descriptions_controller.rb b/backend/app/controllers/tag_descriptions_controller.rb index 5c59226..8403416 100644 --- a/backend/app/controllers/tag_descriptions_controller.rb +++ b/backend/app/controllers/tag_descriptions_controller.rb @@ -1,4 +1,34 @@ class TagDescriptionsController < ApplicationController + rescue_from TagDescriptionOperation::Conflict, with: :render_conflict + + def index + locale = resolve_locale! + descriptions = HistoryAccess.current(TagDescription.all, TagDescriptionVersion) + .where(language_code: locale.language_code).includes(tag: :tag_names) + .order(updated_at: :desc, id: :asc) + title = params[:title].to_s.strip + if title.present? + pattern = "%#{ ApplicationRecord.sanitize_sql_like(title) }%" + descriptions = descriptions.joins(tag: :tag_names) + .where(tag_names: { language_code: locale.language_code, primary_flg: true }) + .where('tag_names.name LIKE ?', pattern).limit(20) + end + render json: descriptions.map { WikiPageRepr.tag_description(locale, _1) } + end + + def show_by_id + locale = resolve_locale! + reference = TagDescription.find_by(id: params[:description_id]) + return head :not_found unless reference + + HistoryAccess.require_current!(reference, TagDescriptionVersion) + description = TagDescription.find_by(tag_id: reference.tag_id, + language_code: locale.language_code) + return head :not_found unless description + + render json: representation(description) + end + def show locale = resolve_locale! description = TagDescription.find_by(tag_id: params[:id], language_code: locale.language_code) @@ -18,6 +48,25 @@ class TagDescriptionsController < ApplicationController render json: representation(description) end + def create + return head :unauthorized unless current_user + return head :forbidden unless current_user.gte_member? + + locale = resolve_locale! + title = params[:title].to_s.strip + body = params[:body].to_s + return render_unprocessable_entity('タイトルは必須です．', field: :title) if title.blank? + return render_unprocessable_entity('本文は必須です．', field: :body) if body.blank? + return render_unprocessable_entity field: :title if title.include?(':') + + tag_name = TagName.find_by(language_code: locale.language_code, name: title) + return render_unprocessable_entity field: :title unless tag_name&.tag + + description = TagDescriptionOperation.create!( + locale, tag: tag_name.tag, body:, created_by_user: current_user) + render json: representation(description), status: :created + end + def update return head :unauthorized unless current_user return head :forbidden unless current_user.gte_member? @@ -32,7 +81,9 @@ class TagDescriptionsController < ApplicationController description = TagDescription.find_by(tag:, language_code: locale.language_code) if description - TagDescriptionOperation.update!(locale, tag:, body:, updated_by_user: current_user) + TagDescriptionOperation.update_with_title!( + locale, tag:, body:, title: params[:title], actor: current_user, + base_revision_id: params[:base_revision_id].presence) description.reload render json: representation(description) else @@ -48,8 +99,20 @@ class TagDescriptionsController < ApplicationController private + def render_conflict error + render json: { error: 'conflict', message: error.message }, status: :conflict + end + def representation description HistoryAccess.require_current!(description, TagDescriptionVersion) + if params[:document] == '1' + locale = resolve_locale! + version = HistoryAccess.visible(TagDescriptionVersion.all).find_by!( + tag_id: description.tag_id, language_code: description.language_code, + version_no: description.version_no) + return WikiPageRepr.tag_description(locale, description).merge(revision_id: version.id) + end + description.as_json(only: [ :tag_id, :language_code, diff --git a/backend/app/controllers/tag_groups_controller.rb b/backend/app/controllers/tag_groups_controller.rb index fc3ec26..d545bbf 100644 --- a/backend/app/controllers/tag_groups_controller.rb +++ b/backend/app/controllers/tag_groups_controller.rb @@ -1,7 +1,22 @@ class TagGroupsController < ApplicationController + rescue_from TagGroupMarkdownParser::InvalidDocument, + TagGroupMarkdownRenderer::UnsupportedStructure, with: :render_document_error + def index locale = resolve_locale! - groups = HistoryAccess.current(TagGroup.all, TagGroupVersion).includes(:created_by_user, :updated_by_user).order(:id) + groups = HistoryAccess.current(TagGroup.all, TagGroupVersion) + .includes(:created_by_user, :updated_by_user) + if params[:document] == '1' + groups = groups.order(updated_at: :desc, id: :asc) + title = params[:title].to_s.strip + if title.present? + pattern = "%#{ ApplicationRecord.sanitize_sql_like(title) }%" + groups = groups.where('CONCAT(?, name) LIKE ? OR CONCAT(?, name) LIKE ?', + TagGroupRepr::PREFIX, pattern, 'tag_group:', pattern).limit(20) + end + return render json: groups.map { WikiPageRepr.tag_group(_1) } + end + groups = groups.order(:id) render json: TagGroupRepr.many(locale, groups.to_a) end @@ -10,7 +25,15 @@ class TagGroupsController < ApplicationController group = TagGroup.includes(:created_by_user, :updated_by_user).find_by(id: params[:id]) return head :not_found unless group - render json: TagGroupRepr.one(locale, group) + render json: representation(locale, group) + end + + def show_by_name + locale = resolve_locale! + group = TagGroup.includes(:created_by_user, :updated_by_user).find_by(name: params[:name]) + return head :not_found unless group + + render json: representation(locale, group) end def create @@ -18,8 +41,8 @@ class TagGroupsController < ApplicationController return head :forbidden unless current_user.gte_member? locale = resolve_locale! - group = TagGroupOperation.create!(locale, **write_attributes, actor: current_user) - render json: TagGroupRepr.one(locale, group), status: :created + group = TagGroupOperation.create!(locale, **write_attributes(locale), actor: current_user) + render json: representation(locale, group), status: :created end def update @@ -30,13 +53,39 @@ class TagGroupsController < ApplicationController group = TagGroup.find_by(id: params[:id]) return head :not_found unless group - TagGroupOperation.update!(locale, group:, **write_attributes, actor: current_user) - render json: TagGroupRepr.one(locale, group.reload) + TagGroupOperation.update!(locale, group:, **write_attributes(locale, group:), actor: current_user) + render json: representation(locale, group.reload) end private - def write_attributes + def render_document_error error + render_unprocessable_entity(error.message) + end + + def representation locale, group + representation = TagGroupRepr.one(locale, group) + return representation unless params[:document] == '1' + + version = TagGroupRepr.current_version(group) + raise ActiveRecord::RecordNotFound unless version + + TagGroupRepr.document(representation, version) + end + + def write_attributes locale, group: nil + if params.key?(:body) + body = params[:body].to_s + if body.blank? + group ||= TagGroup.new + group.errors.add(:body, :blank) + raise ActiveRecord::RecordInvalid, group + end + parsed = TagGroupMarkdownParser.parse(locale, body:, group:) + return parsed.except(:compatibility_body).merge( + name: params[:name], require_compatibility: true) + end + { name: params[:name], description: params[:description], members: params[:members], placements: params[:placements] } end diff --git a/backend/app/controllers/wiki_pages_controller.rb b/backend/app/controllers/wiki_pages_controller.rb index c068fcc..3b64fd2 100644 --- a/backend/app/controllers/wiki_pages_controller.rb +++ b/backend/app/controllers/wiki_pages_controller.rb @@ -253,25 +253,8 @@ class WikiPagesController < ApplicationController base_revision_id = params[:base_revision_id].presence - ApplicationRecord.transaction do - SharedResourceIdAllocator.lock_space!(Tag, 'tags') - tag.lock! - old_title = tag_name.name - if title != old_title - TagVersioning.ensure_snapshot!(tag, created_by_user: current_user) - end - - tag_name.update!(name: title) if title != old_title - - message = params[:message].presence - TagDescriptionOperation.update!(locale, tag:, body:, updated_by_user: current_user, - message:, base_revision_id:) - - if title != old_title - tag.reload - TagVersionRecorder.record!(tag:, event_type: :update, created_by_user: current_user) - end - end + TagDescriptionOperation.update_with_title!( + locale, tag:, body:, title:, actor: current_user, base_revision_id:) head :ok rescue ActiveRecord::RecordInvalid => e @@ -333,21 +316,9 @@ class WikiPagesController < ApplicationController end def render_group_representation representation, version, body: nil, status: :ok - body ||= TagGroupMarkdownRenderer.render(representation) - versions = HistoryAccess.visible(TagGroupVersion.all).where(tag_group_id: version.tag_group_id) - pred = versions.find_by(version_no: version.version_no - 1)&.id - succ = versions.find_by(version_no: version.version_no + 1)&.id - updated_at = version.created_at - render json: { - 'id' => version.tag_group_id, - 'resource_type' => 'tag_group', - 'title' => "#{ TagGroupRepr::PREFIX }#{ representation.fetch('name') }", - 'deprecated_at' => nil, - 'body' => body, - 'revision_id' => version.id, - 'pred' => pred, - 'succ' => succ, - 'updated_at' => updated_at }, status: + document = TagGroupRepr.document(representation, version) + document['body'] = body if body + render json: document, status: rescue TagGroupMarkdownRenderer::UnsupportedStructure => error render_unprocessable_entity(error.message) end diff --git a/backend/app/representations/tag_group_repr.rb b/backend/app/representations/tag_group_repr.rb index 94c9181..33be68a 100644 --- a/backend/app/representations/tag_group_repr.rb +++ b/backend/app/representations/tag_group_repr.rb @@ -56,6 +56,21 @@ module TagGroupRepr 'updated_at' => wiki_page.updated_at } end + def document representation, version + HistoryAccess.require!(version) + versions = HistoryAccess.visible(TagGroupVersion.all) + .where(tag_group_id: version.tag_group_id) + { 'id' => version.tag_group_id, + 'resource_type' => 'tag_group', + 'title' => "#{ PREFIX }#{ representation.fetch('name') }", + 'deprecated_at' => nil, + 'body' => TagGroupMarkdownRenderer.render(representation), + 'revision_id' => version.id, + 'pred' => versions.find_by(version_no: version.version_no - 1)&.id, + 'succ' => versions.find_by(version_no: version.version_no + 1)&.id, + 'updated_at' => version.created_at } + end + def current_version group HistoryAccess.visible(TagGroupVersion.all).find_by(tag_group_id: group.id, version_no: group.version_no) end diff --git a/backend/app/services/tag_description_operation.rb b/backend/app/services/tag_description_operation.rb index b59b59f..8e863aa 100644 --- a/backend/app/services/tag_description_operation.rb +++ b/backend/app/services/tag_description_operation.rb @@ -37,6 +37,37 @@ class TagDescriptionOperation end end + def self.update_with_title! locale, tag:, body:, title:, actor:, base_revision_id: nil + return update!(locale, tag:, body:, updated_by_user: actor, + base_revision_id:) if title.nil? + + title = title.to_s.strip + tag_name = tag.tag_name(locale.language_code) + if title.blank? || title.include?(':') || !tag_name || tag_name.name.include?(':') + tag.errors.add(:title, :invalid) + raise ActiveRecord::RecordInvalid, tag + end + + ApplicationRecord.transaction do + SharedResourceIdAllocator.lock_space!(Tag, 'tags') + tag.lock! + old_title = tag_name.name + if title != old_title + TagVersioning.ensure_snapshot!(tag, created_by_user: actor) + end + + tag_name.update!(name: title) if title != old_title + + update!(locale, tag:, body:, updated_by_user: actor, + base_revision_id:) + + if title != old_title + tag.reload + TagVersionRecorder.record!(tag:, event_type: :update, created_by_user: actor) + end + end + end + def self.record_version! description:, body:, user: creating = description.new_record? version_no = creating ? 1 : description.version_no + 1 diff --git a/backend/config/routes.rb b/backend/config/routes.rb index 62f7f3f..931d6ed 100644 --- a/backend/config/routes.rb +++ b/backend/config/routes.rb @@ -17,7 +17,11 @@ Rails.application.routes.draw do resources :locales, only: [:index] resources :sitemap_entries, only: [:index, :show], param: :resource - resources :tag_groups, only: [:index, :show, :create, :update] + resources :tag_groups, only: [:index, :show, :create, :update] do + collection do + get 'name/:name', action: :show_by_name + end + end resources :nico_tags, path: 'tags/nico', only: [:index, :show, :update] @@ -28,6 +32,9 @@ Rails.application.routes.draw do resources :tags, only: [:index, :show] do collection do + get :descriptions, to: 'tag_descriptions#index' + post :descriptions, to: 'tag_descriptions#create' + get 'descriptions/:description_id', to: 'tag_descriptions#show_by_id' get :autocomplete get :'with-depth', action: :with_depth get :versions, to: 'tag_versions#index' diff --git a/frontend/src/lib/wiki.ts b/frontend/src/lib/wiki.ts index 753e29c..ffcd105 100644 --- a/frontend/src/lib/wiki.ts +++ b/frontend/src/lib/wiki.ts @@ -1,6 +1,6 @@ import { isAxiosError } from 'axios' -import { apiGet } from '@/lib/api' +import { apiGet, apiPost, apiPut } from '@/lib/api' import { switchLocalePath } from '@/lib/locales' import { fetchTag, fetchTagByName } from '@/lib/tags' @@ -24,19 +24,54 @@ export const fetchWikiChanges = ( export const fetchWikiPages = async ( - { title, resourceType }: { title?: string, resourceType?: WikiPage['resourceType'] }, + { title, resourceType = 'tag_description' }: { + title?: string, resourceType?: WikiPage['resourceType'] }, locale: LocaleCode = 'ja', -): Promise => - await apiGet ('/wiki', { locale, params: { title, resource_type: resourceType } }) +): Promise => apiGet ( + resourceType === 'tag_group' ? '/tag_groups' : '/tags/descriptions', + { locale, params: { title, document: '1' } }) export const fetchWikiPage = async ( - id: string, - { version }: { version?: string }, + id: string, + { version }: { version?: string }, locale: LocaleCode = 'ja', -): Promise => - await apiGet (`/wiki/${ encodeURIComponent (id) }`, { - locale, params: version ? { version } : { } }) +): Promise => { + if (version != null) + return apiGet (`/wiki/${encodeURIComponent (id)}`, { locale, params: { version } }) + try + { + return await apiGet (`/tags/descriptions/${encodeURIComponent (id)}`, + { locale, params: { document: '1' } }) + } + catch (error) + { + if (!isAxiosError (error) || error.response?.status !== 404) + throw error + return apiGet (`/tag_groups/${encodeURIComponent (id)}`, + { locale, params: { document: '1' } }) + } +} + + +export const saveTagDocument = async ( + { title, body, page }: { title: string, body: string, page?: WikiPage }, + locale: LocaleCode, +): Promise => { + const groupName = /^(?:タググループ:|tag_group:)(.*)$/i.exec (title.trim ())?.[1] + const options = { locale, params: { document: '1' } } + if (page?.resourceType === 'tag_group' || (page == null && groupName != null)) + { + const attributes = { name: groupName ?? title.trim (), body } + return page == null + ? apiPost ('/tag_groups', attributes, options) + : apiPut (`/tag_groups/${encodeURIComponent (String (page.id))}`, attributes, options) + } + if (page == null) + return apiPost ('/tags/descriptions', { title, body }, options) + return apiPut (`/tags/${encodeURIComponent (String (page.tagId))}/description`, + { title, body }, options) +} export const fetchWikiPageByTitle = async ( @@ -46,7 +81,14 @@ export const fetchWikiPageByTitle = async ( ): Promise => { try { - return await apiGet (`/wiki/title/${ encodeURIComponent (title) }`, { locale, params: { version } }) + if (version != null) + return await apiGet (`/wiki/title/${encodeURIComponent (title)}`, + { locale, params: { version } }) + const groupName = /^(?:タググループ:|tag_group:)(.*)$/i.exec (title.trim ())?.[1] + const path = groupName == null + ? `/tags/name/${encodeURIComponent (title)}/description` + : `/tag_groups/name/${encodeURIComponent (groupName)}` + return await apiGet (path, { locale, params: { document: '1' } }) } catch (error) { @@ -72,7 +114,7 @@ export const fetchWikiPageByReference = async ( { if (!isAxiosError (error) || error.response?.status !== 404) throw error - const tag = await fetchWikiTag ({ wikiId: reference }, locale) + const tag = await fetchTagByName (reference, locale) return tag == null ? null : fetchWikiPageByTitle (tag.name, params, locale) } } @@ -86,7 +128,9 @@ export const fetchWikiTag = async ( return fetchTagByName (reference.name, locale) try { - return await apiGet (`/wiki/${encodeURIComponent (reference.wikiId)}/tag`, { locale }) + const page = await fetchWikiPage (reference.wikiId, { }, locale) + return (page.resourceType === 'tag_description' && page.tagId != null) + ? fetchTag (String (page.tagId), locale) : null } catch (error) { diff --git a/frontend/src/pages/wiki/WikiEditPage.tsx b/frontend/src/pages/wiki/WikiEditPage.tsx index 84934a6..1791ab5 100644 --- a/frontend/src/pages/wiki/WikiEditPage.tsx +++ b/frontend/src/pages/wiki/WikiEditPage.tsx @@ -11,8 +11,8 @@ import FormField from '@/components/common/FormField' import MainArea from '@/components/layout/MainArea' import { toast } from '@/components/ui/use-toast' import PageHead from '@/components/common/PageHead' -import { apiGet, apiPut } from '@/lib/api' -import { wikiKeys } from '@/lib/queryKeys' +import { fetchWikiPage, saveTagDocument } from '@/lib/wiki' +import { tagsKeys, wikiKeys } from '@/lib/queryKeys' import { canEditContent } from '@/lib/users' import { inputClass } from '@/lib/utils' import { useValidationErrors } from '@/lib/useValidationErrors' @@ -33,7 +33,7 @@ const FIXED_WIKI_EDITOR_VIEW = { menu: true, md: true, html: true } as const type Props = { user: User | null } -type WikiFormField = 'title' | 'body' +type WikiFormField = 'title' | 'name' | 'body' const WikiEditPage: FC = ({ user }) => { @@ -50,7 +50,7 @@ const WikiEditPage: FC = ({ user }) => { const source = `${locale}:${id}` const { data: wiki, isPending: loading } = useQuery ({ queryKey: wikiKeys.show (id ?? '', { }, locale), - queryFn: () => apiGet (`/wiki/${ encodeURIComponent (id ?? '') }`, { locale }), + queryFn: () => fetchWikiPage (id ?? '', { }, locale), enabled: editable }) const [body, setBody] = useState (wiki?.body ?? '') const [loadedSource, setLoadedSource] = useState (wiki == null ? '' : source) @@ -60,22 +60,19 @@ const WikiEditPage: FC = ({ user }) => { useValidationErrors () const handleSubmit = async () => { - if (pending || !editable) + if (pending || !editable || wiki == null) return clearValidationErrors () - const formData = new FormData () - formData.append ('title', title) - formData.append ('body', body) - try { - await apiPut (`/wiki/${ id }`, formData, - { locale, headers: { 'Content-Type': 'multipart/form-data' } }) + await saveTagDocument ({ title, body, page: wiki }, locale) qc.setQueryData (wikiKeys.show (title, { }, locale), (prev: WikiPage | undefined) => prev == null ? prev : { ...prev, title, body }) - qc.invalidateQueries ({ queryKey: wikiKeys.root }) + await Promise.all ([ + qc.invalidateQueries ({ queryKey: wikiKeys.root }), + qc.invalidateQueries ({ queryKey: tagsKeys.root })]) toast ({ title: label ('shared:messages.postSuccess') }) navigate (`/wiki/${ encodeURIComponent (title) }`) } @@ -110,7 +107,7 @@ const WikiEditPage: FC = ({ user }) => { {/* タイトル */} {/* TODO: タグ補完 */} - + {({ describedBy, invalid }) => ( = ({ user }) => { @@ -55,14 +55,9 @@ const WikiNewPage: FC = ({ user }) => { const handleSubmit = async () => { clearValidationErrors () - const formData = new FormData - formData.append ('title', title) - formData.append ('body', body) - try { - const data = await apiPost ('/wiki', formData, - { locale, headers: { 'Content-Type': 'multipart/form-data' } }) + const data = await saveTagDocument ({ title, body }, locale) await qc.cancelQueries ({ queryKey: wikiKeys.root }) qc.setQueryData (wikiKeys.show (data.title, { }, locale), data) await Promise.all ([ @@ -91,7 +86,7 @@ const WikiNewPage: FC = ({ user }) => { {/* タイトル */} {/* TODO: タグ補完 */} - + {({ describedBy, invalid }) => (