コミットを比較

..
2 コミット
作成者 SHA1 メッセージ 日付
Codex Night Worker 3dba626e3f AI: address review on PR #467 2026-11-13 12:42:35 +09:00
Codex Night Worker 76ca985096 AI: address issue #320 2026-11-13 07:40:54 +09:00
12個のファイルの変更、90行の追加、40行の削除
+6 -6
ファイルの表示
@@ -37,7 +37,7 @@ class PostsController < ApplicationController
offset = (page - 1) * limit offset = (page - 1) * limit
history_join_required = order[0] == 'updated_at' || updated_between.any?(&:present?) needs_history_join = order[0] == 'updated_at' || updated_between.any?(&:present?)
q = q =
filtered_posts(locale) filtered_posts(locale)
@@ -57,8 +57,8 @@ class PostsController < ApplicationController
q = q.where('posts.created_at >= ?', created_between[0]) if created_between[0] q = q.where('posts.created_at >= ?', created_between[0]) if created_between[0]
q = q.where('posts.created_at <= ?', created_between[1]) if created_between[1] q = q.where('posts.created_at <= ?', created_between[1]) if created_between[1]
count_scope = q count_scope = q
if history_join_required if needs_history_join
history_times = PostVersion.all history_times = HistoryAccess.visible(PostVersion.all)
.select('post_id, MAX(created_at) AS latest_at').group(:post_id) .select('post_id, MAX(created_at) AS latest_at').group(:post_id)
q = q.joins("LEFT JOIN (#{ history_times.to_sql }) history_times " + q = q.joins("LEFT JOIN (#{ history_times.to_sql }) history_times " +
'ON history_times.post_id = posts.id') 'ON history_times.post_id = posts.id')
@@ -72,7 +72,7 @@ class PostsController < ApplicationController
q = q.where("#{ updated_at_all_sql } <= ?", updated_between[1]) q = q.where("#{ updated_at_all_sql } <= ?", updated_between[1])
end end
count_scope = q if updated_between.any?(&:present?) count_scope = q if updated_between.any?(&:present?)
if history_join_required if needs_history_join
q = q.select('posts.*', "#{ updated_at_all_sql } AS updated_at") q = q.select('posts.*', "#{ updated_at_all_sql } AS updated_at")
end end
@@ -92,10 +92,10 @@ class PostsController < ApplicationController
.offset(offset) .offset(offset)
.to_a .to_a
latest_version_times = latest_version_times =
if history_join_required if needs_history_join
{ } { }
else else
PostVersion.where(post_id: posts.map(&:id)) HistoryAccess.visible(PostVersion.where(post_id: posts.map(&:id)))
.group(:post_id).maximum(:created_at) .group(:post_id).maximum(:created_at)
end end
+1
ファイルの表示
@@ -49,6 +49,7 @@ class TagDescriptionsController < ApplicationController
private private
def representation description def representation description
HistoryAccess.require_current!(description, TagDescriptionVersion)
description.as_json(only: [ description.as_json(only: [
:tag_id, :tag_id,
:language_code, :language_code,
+1 -1
ファイルの表示
@@ -1,7 +1,7 @@
class TagGroupsController < ApplicationController class TagGroupsController < ApplicationController
def index def index
locale = resolve_locale! locale = resolve_locale!
groups = TagGroup.includes(:created_by_user, :updated_by_user).order(:id) groups = HistoryAccess.current(TagGroup.all, TagGroupVersion).includes(:created_by_user, :updated_by_user).order(:id)
render json: TagGroupRepr.many(locale, groups.to_a) render json: TagGroupRepr.many(locale, groups.to_a)
end end
+6 -5
ファイルの表示
@@ -8,11 +8,12 @@ class WikiPagesController < ApplicationController
title = params[:title].to_s.strip title = params[:title].to_s.strip
descriptions = descriptions =
TagDescription.where(language_code: locale.language_code) HistoryAccess.current(TagDescription.all, TagDescriptionVersion)
.where(language_code: locale.language_code)
.includes(tag: :tag_names) .includes(tag: :tag_names)
descriptions = descriptions.order(updated_at: :desc, id: :asc) descriptions = descriptions.order(updated_at: :desc, id: :asc)
tag_groups = TagGroup.order(updated_at: :desc, id: :asc) tag_groups = HistoryAccess.current(TagGroup.all, TagGroupVersion).order(updated_at: :desc, id: :asc)
if title.present? if title.present?
pattern = "%#{ ApplicationRecord.sanitize_sql_like(title) }%" pattern = "%#{ ApplicationRecord.sanitize_sql_like(title) }%"
descriptions = descriptions =
@@ -40,13 +41,13 @@ class WikiPagesController < ApplicationController
return head :not_found unless language.is_a?(String) return head :not_found unless language.is_a?(String)
return head :not_found unless Language.exists?(code: language) return head :not_found unless Language.exists?(code: language)
names = TagDescription.joins(tag: :tag_names) names = HistoryAccess.current(TagDescription.all, TagDescriptionVersion).joins(tag: :tag_names)
.where(language_code: locale.language_code, tag_names: { language_code: language }) .where(language_code: locale.language_code, tag_names: { language_code: language })
.pluck('tag_descriptions.tag_id', 'tag_descriptions.id', 'tag_names.name') .pluck('tag_descriptions.tag_id', 'tag_descriptions.id', 'tag_names.name')
candidates = names.map do |tag_id, description_id, name| candidates = names.map do |tag_id, description_id, name|
WikiPageRepr.autolink_tag(locale, tag_id:, description_id:, name:, language:) WikiPageRepr.autolink_tag(locale, tag_id:, description_id:, name:, language:)
end end
candidates.concat(TagGroup.order(:id).pluck(:id, :name).map do |id, name| candidates.concat(HistoryAccess.current(TagGroup.all, TagGroupVersion).order(:id).pluck(:id, :name).map do |id, name|
WikiPageRepr.autolink_group(locale, id:, name:, language:) WikiPageRepr.autolink_group(locale, id:, name:, language:)
end) end)
render json: candidates render json: candidates
@@ -410,7 +411,7 @@ class WikiPagesController < ApplicationController
end end
def tag_description_by_title locale, title def tag_description_by_title locale, title
TagDescription.joins(tag: :tag_names) HistoryAccess.current(TagDescription.all, TagDescriptionVersion).joins(tag: :tag_names)
.find_by(language_code: locale.language_code, .find_by(language_code: locale.language_code,
tag_names: { language_code: locale.language_code, name: title }) tag_names: { language_code: locale.language_code, name: title })
end end
+23
ファイルの表示
@@ -147,6 +147,29 @@ class HistoryAccess < ActiveSupport::CurrentAttributes
scope.where(Arel.sql(sql(scope.klass, row))) scope.where(Arel.sql(sql(scope.klass, row)))
end end
def self.current(scope, version_model)
return scope if admin?
table = scope.klass.table_name
keys = KEYS.fetch(version_model.table_name)
matches = keys.map do |key|
column = scope.klass.column_names.include?(key) ? key : 'id'
"current_snapshot.#{ key } = #{ table }.#{ column }"
end.join(' AND ')
boundary = "#{ matches } AND current_snapshot.version_no >= #{ table }.version_no " +
"AND current_snapshot.event_type = 'discard'"
scope = scope.where(Arel.sql("NOT EXISTS (SELECT 1 FROM #{ version_model.table_name } " +
"current_snapshot WHERE #{ boundary })"))
condition = "#{ matches } AND current_snapshot.version_no = #{ table }.version_no"
scope.where(Arel.sql("NOT EXISTS (SELECT 1 FROM #{ version_model.table_name } " +
"current_snapshot WHERE #{ condition } AND NOT (#{ sql(version_model, 'current_snapshot') }))"))
end
def self.require_current!(record, version_model)
return if admin?
raise Forbidden unless current(record.class.where(id: record.id), version_model).exists?
end
def self.blob_visible?(blob, seen = []) def self.blob_visible?(blob, seen = [])
return true if admin? return true if admin?
return false if seen.include?(blob.id) return false if seen.include?(blob.id)
+1
ファイルの表示
@@ -44,6 +44,7 @@ module TagGroupRepr
end end
def one locale, group def one locale, group
HistoryAccess.require_current!(group, TagGroupVersion)
many(locale, [group]).first many(locale, [group]).first
end end
+1
ファイルの表示
@@ -5,6 +5,7 @@ module WikiPageRepr
module_function module_function
def tag_description locale, description def tag_description locale, description
HistoryAccess.require_current!(description, TagDescriptionVersion)
tag = description.tag tag = description.tag
{ 'id' => description.id, { 'id' => description.id,
+3 -3
ファイルの表示
@@ -14,7 +14,7 @@ RSpec.describe 'History boundaries shared with Counts', type: :request do
end end
end end
it 'keeps discarded history private while exposing the current description' do it 'denies the same discarded description through Counts, the facade and the ordinary API' do
get '/counts/query', params: { resource: 'tag_descriptions', definition: 'history', get '/counts/query', params: { resource: 'tag_descriptions', definition: 'history',
mode: 'details', locale: 'ja', execution_at: '2025-01-01T00:00:00Z' } mode: 'details', locale: 'ja', execution_at: '2025-01-01T00:00:00Z' }
expect(response).to have_http_status(:ok) expect(response).to have_http_status(:ok)
@@ -22,8 +22,8 @@ RSpec.describe 'History boundaries shared with Counts', type: :request do
'excluded_count' => 2, 'versions' => []) 'excluded_count' => 2, 'versions' => [])
["/wiki/#{ description.id }", "/tags/#{ tag.id }/description"].each do |path| ["/wiki/#{ description.id }", "/tags/#{ tag.id }/description"].each do |path|
get path, params: { locale: 'ja' } get path, params: { locale: 'ja' }
expect(response).to have_http_status(:ok) expect(response).to have_http_status(:not_found)
expect(response.parsed_body.fetch('body')).to eq(description.body) expect(response.body).not_to include('private body')
end end
get '/sitemap_entries/descriptions', params: { locale: 'ja' } get '/sitemap_entries/descriptions', params: { locale: 'ja' }
expect(response).to have_http_status(:ok) expect(response).to have_http_status(:ok)
+10 -8
ファイルの表示
@@ -69,38 +69,40 @@ RSpec.describe 'Post history privacy', type: :request do
sign_in_as(create(:user, role.to_sym)) unless role == 'anonymous' sign_in_as(create(:user, role.to_sym)) unless role == 'anonymous'
end end
it 'uses the latest version timestamp for display regardless of role' do it 'uses only authorised history timestamps for display' do
get '/posts' get '/posts'
expect(response).to have_http_status(:ok) expect(response).to have_http_status(:ok)
row = response.parsed_body.fetch('posts').find { _1.fetch('id') == post_record.id } row = response.parsed_body.fetch('posts').find { _1.fetch('id') == post_record.id }
expected = origin + 4.days expected = origin + (role == 'admin' ? 4 : 3).days
expect(Time.zone.parse(row.fetch('updated_at'))).to eq(expected) expect(Time.zone.parse(row.fetch('updated_at'))).to eq(expected)
end end
it 'uses the latest version timestamp for date bounds and count regardless of role' do it 'uses the same visibility for date bounds and count' do
get '/posts', params: { updated_from: (origin + 4.days).iso8601, get '/posts', params: { updated_from: (origin + 4.days).iso8601,
updated_to: (origin + 4.days).iso8601 } updated_to: (origin + 4.days).iso8601 }
expect(response).to have_http_status(:ok) expect(response).to have_http_status(:ok)
ids = response.parsed_body.fetch('posts').pluck('id') ids = response.parsed_body.fetch('posts').pluck('id')
expect(ids).to eq([post_record.id]) expect(ids).to eq(role == 'admin' ? [post_record.id] : [])
expect(response.parsed_body.fetch('count')).to eq(ids.length) expect(response.parsed_body.fetch('count')).to eq(ids.length)
end end
it 'sorts using the latest version timestamps in both directions regardless of role' do it 'sorts using authorised timestamps in both directions' do
['asc', 'desc'].each do |direction| ['asc', 'desc'].each do |direction|
get '/posts', params: { order: "updated_at:#{ direction }" } get '/posts', params: { order: "updated_at:#{ direction }" }
expect(response).to have_http_status(:ok) expect(response).to have_http_status(:ok)
expected = [other_post.id, post_record.id] expected = role == 'admin' ? [other_post.id, post_record.id] :
[post_record.id, other_post.id]
expected.reverse! if direction == 'desc' expected.reverse! if direction == 'desc'
expect(response.parsed_body.fetch('posts').pluck('id')).to eq(expected) expect(response.parsed_body.fetch('posts').pluck('id')).to eq(expected)
end end
end end
it 'applies latest version timestamp ordering before pagination regardless of role' do it 'applies authorised timestamp ordering before pagination' do
get '/posts', params: { order: 'updated_at:desc', limit: 1 } get '/posts', params: { order: 'updated_at:desc', limit: 1 }
expect(response).to have_http_status(:ok) expect(response).to have_http_status(:ok)
expect(response.parsed_body.fetch('posts').pluck('id')).to eq([post_record.id]) expected = role == 'admin' ? post_record.id : other_post.id
expect(response.parsed_body.fetch('posts').pluck('id')).to eq([expected])
expect(response.parsed_body.fetch('count')).to eq(2) expect(response.parsed_body.fetch('count')).to eq(2)
end end
+8 -2
ファイルの表示
@@ -36,10 +36,13 @@ const replaceToken = (value: string, start: number, end: number, text: string) =
type Props = Omit<ComponentPropsWithoutRef<'textarea'>, 'value' | 'onChange' | 'onBlur'> & { type Props = Omit<ComponentPropsWithoutRef<'textarea'>, 'value' | 'onChange' | 'onBlur'> & {
tags: string tags: string
setTags: (tags: string) => void setTags: (tags: string) => void
label?: ReactNode
errors?: ReactNode[] } errors?: ReactNode[] }
const PostFormTagsArea: FC<Props> = ({ tags, setTags, errors, ...rest }) => { const PostFormTagsArea: FC<Props> = (
{ tags, setTags, label: fieldLabel, errors, ...rest },
) => {
const { label } = useUITranslation ('navigation') const { label } = useUITranslation ('navigation')
const locale = useLocale () const locale = useLocale ()
const ref = useRef<HTMLTextAreaElement> (null) const ref = useRef<HTMLTextAreaElement> (null)
@@ -117,7 +120,10 @@ const PostFormTagsArea: FC<Props> = ({ tags, setTags, errors, ...rest }) => {
}, [locale]) }, [locale])
return ( return (
<FormField className="relative w-full" label={label ('tags')} messages={errors}> <FormField
className="relative w-full"
label={fieldLabel ?? label ('tags')}
messages={errors}>
{({ describedBy, invalid }) => ( {({ describedBy, invalid }) => (
<> <>
<TextArea <TextArea
+22 -3
ファイルの表示
@@ -162,12 +162,13 @@ describe ('NicoTagListPage', () => {
}) })
}) })
it ('updates links from a tag card', async () => { it ('saves links after completing the tag at the cursor', async () => {
api.apiGet api.apiGet
.mockResolvedValueOnce ({ .mockResolvedValueOnce ({
tags: [buildNicoTag ({ id: 7, name: 'nico:source' })], tags: [buildNicoTag ({ id: 7, name: 'nico:source' })],
count: 1, count: 1,
}) })
.mockResolvedValueOnce ([buildTag ({ id: 8, name: '連携先', postCount: 3 })])
.mockResolvedValueOnce ({ .mockResolvedValueOnce ({
tags: [ tags: [
buildNicoTag ({ buildNicoTag ({
@@ -183,8 +184,24 @@ describe ('NicoTagListPage', () => {
renderPage () renderPage ()
fireEvent.click (await screen.findByRole ('button', { name: '編集' })) fireEvent.click (await screen.findByRole ('button', { name: '編集' }))
fireEvent.change (screen.getByLabelText ('連携する広場タグ'), { const textarea = screen.getByLabelText ('連携する広場タグ')
target: { value: '連携先' }, fireEvent.change (textarea, { target: { value: '既存 連携\n後続' } })
fireEvent.focus (textarea)
fireEvent.select (textarea, {
target: { selectionStart: 4, selectionEnd: 4 } })
await waitFor (() => {
expect (api.apiGet).toHaveBeenCalledWith (
'/tags/autocomplete',
{ locale: 'ja', params: { q: '連携', nico: '0' },
signal: expect.any (AbortSignal) })
})
fireEvent.mouseDown (await screen.findByText ('連携先'))
expect (textarea).toHaveValue ('既存 連携先 \n後続')
await waitFor (() => {
expect ((textarea as HTMLTextAreaElement).selectionStart).toBe (7)
expect ((textarea as HTMLTextAreaElement).selectionEnd).toBe (7)
}) })
fireEvent.click (screen.getByRole ('button', { name: '保存' })) fireEvent.click (screen.getByRole ('button', { name: '保存' }))
@@ -195,6 +212,8 @@ describe ('NicoTagListPage', () => {
{ locale: 'ja', headers: { 'Content-Type': 'multipart/form-data' } }, { locale: 'ja', headers: { 'Content-Type': 'multipart/form-data' } },
) )
}) })
const payload = api.apiPut.mock.calls[0][1] as FormData
expect (payload.get ('tags')).toBe ('既存 連携先 \n後続')
expect (await screen.findByText ('1 件')).toBeInTheDocument () expect (await screen.findByText ('1 件')).toBeInTheDocument ()
}) })
+8 -12
ファイルの表示
@@ -6,11 +6,10 @@ import { useLocation } from 'react-router-dom'
import { useLocale, useLocaleNavigate } from '@/lib/useLocale' import { useLocale, useLocaleNavigate } from '@/lib/useLocale'
import TagLink from '@/components/TagLink' import TagLink from '@/components/TagLink'
import SortHeader from '@/components/SortHeader' import SortHeader from '@/components/SortHeader'
import FieldError from '@/components/common/FieldError' import PostFormTagsArea from '@/components/PostFormTagsArea'
import FormField from '@/components/common/FormField' import FormField from '@/components/common/FormField'
import PageTitle from '@/components/common/PageTitle' import PageTitle from '@/components/common/PageTitle'
import Pagination from '@/components/common/Pagination' import Pagination from '@/components/common/Pagination'
import TextArea from '@/components/common/TextArea'
import useDialogue from '@/lib/dialogues/useDialogue' import useDialogue from '@/lib/dialogues/useDialogue'
import MainArea from '@/components/layout/MainArea' import MainArea from '@/components/layout/MainArea'
import { toast } from '@/components/ui/use-toast' import { toast } from '@/components/ui/use-toast'
@@ -357,21 +356,18 @@ const NicoTagListPage: FC<Props> = ({ user }) => {
dark:bg-rose-950/30"> dark:bg-rose-950/30">
<td colSpan={editable ? 5 : 4} className="p-3"> <td colSpan={editable ? 5 : 4} className="p-3">
<div className="space-y-2"> <div className="space-y-2">
<label htmlFor={`nico-links-${ tag.id }`} <PostFormTagsArea
className="block text-sm font-semibold">
{label ('nico.tagsToLink')}
</label>
<TextArea
id={`nico-links-${ tag.id }`} id={`nico-links-${ tag.id }`}
value={rawTags[tag.id] ?? ''} label={label ('nico.tagsToLink')}
invalid={(errorsByTagId[tag.id] ?? []).length > 0} aria-label={message ('nico.tagsToLink').text}
tags={rawTags[tag.id] ?? ''}
errors={errors}
className="min-h-24 resize-y" className="min-h-24 resize-y"
placeholder={message ('nico.tagsPlaceholder').text} placeholder={message ('nico.tagsPlaceholder').text}
lang={message ('nico.tagsPlaceholder').language} lang={message ('nico.tagsPlaceholder').language}
onChange={e => setRawTags (rawTags => ({ setTags={value => setRawTags (rawTags => ({
...rawTags, ...rawTags,
[tag.id]: e.target.value}))}/> [tag.id]: value}))}/>
<FieldError messages={errors}/>
<div className="flex justify-end gap-2"> <div className="flex justify-end gap-2">
<button <button
type="button" type="button"