Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughギルド単位のログ設定永続化モデルと API ルーター、ダッシュボードの UI/API、ボットのログ送信コグおよびクライアント API を追加し、イベント単位でチャンネル・Webhook・無視チャネルを保存・操作できるようにします。 Changes
Sequence Diagram(s)sequenceDiagram
participant Dashboard as Dashboard(UI)
participant DashAPI as Dashboard API(Route)
participant BackendAPI as Backend API(Go)
participant DB as Database(GORM)
participant Bot as Bot(Python)
participant Discord as Discord(Webhook)
Dashboard->>DashAPI: POST /api/guilds/{id}/modules/logging (保存)
DashAPI->>DashAPI: validateAdmin(guildId)
DashAPI->>BackendAPI: POST /guilds/logging/{id} (LoggingSetting JSON)
BackendAPI->>DB: Upsert LoggingSetting (OnConflict/Save)
DB-->>BackendAPI: 保存完了
BackendAPI-->>DashAPI: 200 OK
DashAPI-->>Dashboard: 成功応答
Bot->>BackendAPI: GET /guilds/logging/{id}
BackendAPI->>DB: Query LoggingSetting
DB-->>BackendAPI: setting JSON
BackendAPI-->>Bot: LoggingSetting
Bot->>Bot: Resolve or create webhook (共有再利用チェック)
Bot->>Discord: POST webhook (Embed)
Discord-->>Bot: 2xx / NotFound/Forbidden
alt NotFound or Forbidden
Bot->>BackendAPI: DELETE event config (on failure)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
src/dashboard/src/lib/api/requests.ts (1)
272-323: logging 系API関数にも Guild ID バリデーションを揃えることを推奨します。同ファイル内の他APIヘルパーと同様に、invalid ID をリクエスト前に弾く方がデバッグしやすくなります。
リファクタ案（共通ガード）
+function assertValidGuildId(guildId: string) { + if (!isValidDiscordId(guildId)) { + throw new Error("Invalid Guild ID"); + } +} + export async function fetchLoggingSetting(guildId: string): Promise<LoggingSetting> { + assertValidGuildId(guildId); const response = await fetch(`${RESOURCE_API_BASE_URL}/guilds/logging/${guildId}`); @@ export async function saveLoggingSetting(guildId: string, data: Partial<LoggingSetting>) { + assertValidGuildId(guildId); const response = await fetch(`${RESOURCE_API_BASE_URL}/guilds/logging/${guildId}`, { @@ export async function saveOneLoggingEvent(guildId: string, eventName: string, data: LoggingEvent) { + assertValidGuildId(guildId); @@ export async function deleteOneLoggingEvent(guildId: string, eventName: string) { + assertValidGuildId(guildId); @@ export async function deleteLoggingSetting(guildId: string) { + assertValidGuildId(guildId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/lib/api/requests.ts` around lines 272 - 323, Add the same pre-request guild ID validation used elsewhere to all logging API helpers (fetchLoggingSetting, saveLoggingSetting, saveOneLoggingEvent, deleteOneLoggingEvent, deleteLoggingSetting): call the shared validation guard (e.g., validateGuildId or request_verification utility) at the top of each function and throw/return early for invalid IDs before making fetch calls; if no shared guard exists, extract a small helper (validateGuildId) and use it from each of these functions to keep behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/src/internal/model/LoggingSetting.go`:
- Around line 13-19: StringSlice.Scan currently only asserts value as []byte
which fails for NULL (nil) or when the DB returns a string; update the
StringSlice.Scan implementation to handle three cases: if value == nil return
nil, if value is a string convert it to []byte (e.g., []byte(str)) then
json.Unmarshal, and if value is []byte behave as today; keep using
json.Unmarshal into s and mirror the nil/string handling used in
ModeratorSetting.go to avoid type assertion errors.
In `@src/api/src/internal/router/logging.go`:
- Around line 13-23: The RegisterLoggingSetting routes are currently exposed
without middleware; require authentication/authorization by attaching the
appropriate middleware to the group or enforcing checks in each handler: update
RegisterLoggingSetting to accept/attach the auth middleware (or call
router.Group("/guilds/logging", <AuthMiddleware>) ) and ensure handlers
getLoggingSetting, saveLoggingSetting, deleteLoggingSetting,
getOneLoggingEventConfig, setOneLoggingEvent, and deleteOneLoggingEvent perform
authorization verifying the requester is the guild admin or a valid service
token before returning or mutating webhook/logging settings. Ensure failure
paths return proper unauthorized/forbidden responses.
- Around line 98-113: The handlers setOneLoggingEvent and deleteOneLoggingEvent
currently ignore errors from db.FirstOrCreate and db.Save so they always return
HTTP 200 even when DB writes fail; update both functions to check the returned
gorm.Error after db.FirstOrCreate(&setting, model.LoggingSetting{GuildID: id})
and after db.Save(&setting) (and after delete operations), log the error, and
return an appropriate non-200 response (e.g., 500 with a JSON error) when
persistence fails so callers can detect webhook save/delete failures; reference
the variables and symbols setting, input, eventName, and the DB calls in each
handler when adding these checks.
In `@src/bot/cogs/logging.py`:
- Around line 56-72: _process_log currently only checks
event_config.ignored_channels and never consults the global ignored list; call
the API helper get_logging_setting(str(guild.id), event_name) (from
src/bot/lib/api.py) to obtain the merged logging setting (which includes
global_ignored_channels and per-event ignored channels) and use that combined
ignored list to decide early-return; also ensure callers like channel_create and
channel_delete pass the trigger_channel_id through to _process_log so per-event
ignores are respected (reference symbols: _process_log, get_logging_setting,
event_config, global_ignored_channels, trigger_channel_id, channel_create,
channel_delete).
- Around line 83-98: The current early returns in on_message_delete and
on_message_edit skip logging for messages without text; update on_message_delete
(listener on_message_delete) to only return for bot authors or missing guild,
then build the embed using message.content if present else include
attachment/sticker summary and always include message ID and any attachment
filenames/URLs before calling _process_log; for on_message_edit (listener
on_message_edit) change the guard to return only when author is a bot or when
before and after are identical in both content and attachments/stickers (i.e.,
compare before.content==after.content AND attachments/stickers equality), and
when creating the edit embed include both old/new text if present and list
attachments/stickers (or indicate "(添付のみ)" when text is empty) so edits with no
text are still logged.
In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/logging/route.ts:
- Around line 41-52: The current error handling in the route conflates auth
errors and internal errors; update the catch blocks around
validateAdmin()/fetchLoggingSetting()/createLoggingSetting()/deleteLoggingSetting()
so that you only map errors thrown by validateAdmin() to 401 (Unauthorized) or
403 (Forbidden) based on the error message, and treat all other errors as 500
(Internal Server Error); specifically, in the GET/POST/DELETE handlers inspect
the caught error to see if it originated from validateAdmin (e.g., check
error.message === "Unauthorized" or "Forbidden") and return NextResponse.json({
error: error.message }, { status: 401/403 }) for those cases, otherwise return
NextResponse.json({ error: error.message }, { status: 500 }).
In `@src/dashboard/src/app/dashboard/`[guildId]/logging/LoggingClient.tsx:
- Around line 144-148: ChannelSelecter の onChange ハンドラは log_channel_id
のみを更新して既存の webhook_url を残しているため、チャンネル変更後も古い webhook が使われ続けます；修正は ChannelSelecter
の onChange 内で evs をコピーして evs[index].log_channel_id を新しい val にセットするのに加え、同じイベントの
webhook_url（おそらく event.webhook_url または evs[index].webhook_url）を明示的に null
か空文字にクリアしてから setSetting({...setting, events: evs}) を呼ぶようにしてください（関係するシンボル:
ChannelSelecter, setting.events, evs[index].log_channel_id,
evs[index].webhook_url, setSetting）。
- Around line 131-132: The map over setting.events in LoggingClient.tsx is using
index as the React key which causes state reuse issues (e.g., ChannelSelecter)
when items are added/removed; change the key passed to the mapped element from
index to a stable unique identifier such as event.event_name (or another unique
event id on the event object) in the JSX returned by setting.events.map to
ensure each event's component instance and state stay tied to that specific
event.
- Around line 47-55: The current handleSave incorrectly uses
setting.events.filter(...) in the validation which always evaluates truthy and
also leaves loading stuck true; change the validation to check for any empty
channel id (e.g., use Array.prototype.some on setting.events to detect
event.log_channel_id === "") and, if validation fails, call setLoading(false)
before returning so the component isn't stuck in loading state; update
references in the handleSave function to use the corrected predicate and ensure
setLoading is properly reset on all early returns or error paths.
In `@src/dashboard/src/lib/api/requests.ts`:
- Around line 293-307: The URL construction in saveOneLoggingEvent and
deleteOneLoggingEvent risks breaking when eventName contains slashes or spaces;
update both functions to wrap eventName with encodeURIComponent(eventName) in
the fetch URL (same approach used for moduleName elsewhere) so the path segments
are safely encoded and requests resolve correctly.
- Around line 21-26: The TypeScript LoggingEvent interface's webhook_url is
required but the backend Go model LoggingSetting uses WebhookUrl *string
(nullable), causing a type mismatch when fetchLoggingSetting returns raw JSON;
change the LoggingEvent definition (and any usages) to accept null by making
webhook_url type string | null (or optional) and update code paths that read or
pass LoggingEvent.webhook_url to handle null safely (e.g., guard before using
the URL) so the TS types match LoggingSetting.go and fetchLoggingSetting's raw
response.
---
Nitpick comments:
In `@src/dashboard/src/lib/api/requests.ts`:
- Around line 272-323: Add the same pre-request guild ID validation used
elsewhere to all logging API helpers (fetchLoggingSetting, saveLoggingSetting,
saveOneLoggingEvent, deleteOneLoggingEvent, deleteLoggingSetting): call the
shared validation guard (e.g., validateGuildId or request_verification utility)
at the top of each function and throw/return early for invalid IDs before making
fetch calls; if no shared guard exists, extract a small helper (validateGuildId)
and use it from each of these functions to keep behavior consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0b10d6be-1ab0-4ba7-8f93-b78a91220fc0
📒 Files selected for processing (11)
src/api/src/cmd/main.gosrc/api/src/internal/model/LoggingSetting.gosrc/api/src/internal/router/logging.gosrc/bot/cogs/logging.pysrc/bot/lib/api.pysrc/dashboard/Dockerfilesrc/dashboard/src/app/api/guilds/[guildId]/modules/logging/route.tssrc/dashboard/src/app/dashboard/[guildId]/logging/LoggingClient.tsxsrc/dashboard/src/app/dashboard/[guildId]/logging/page.tsxsrc/dashboard/src/lib/api/requests.tssrc/dashboard/src/lib/modules.ts
💤 Files with no reviewable changes (1)
- src/dashboard/Dockerfile
Comment on lines
+56
to
+72
| async def _process_log(self, guild: discord.Guild, event_name: str, embed: discord.Embed, trigger_channel_id: str = None): | ||
| if not guild: return | ||
|
|
||
| is_enabled = await self.bot.api.is_module_enabled(str(guild.id), "logging") | ||
| if not is_enabled or not is_enabled.get('enabled'): | ||
| return | ||
|
|
||
| event_config = await self.bot.api.get_event_config(str(guild.id), event_name) | ||
| if not event_config or not self.bot.api._is_valid_discord_id(event_config.log_channel_id): | ||
| return | ||
|
|
||
| if trigger_channel_id: | ||
| ignored = event_config.ignored_channels or [] | ||
| if trigger_channel_id in ignored: | ||
| return | ||
|
|
||
| webhook_url = await self._get_or_create_webhook(guild, event_config, event_name) |
There was a problem hiding this comment.
無視チャンネル設定が bot 側で反映しきれていません。
_process_log() は event_config.ignored_channels しか見ておらず、global_ignored_channels を一度も参照していません。さらに channel_create / channel_delete では trigger_channel_id を渡していないので、そのイベント個別の無視設定も効きません。src/bot/lib/api.py には get_logging_setting() が追加済みなので、ここで統合判定するのが安全です。
Also applies to: 130-156
🧰 Tools
🪛 Ruff (0.15.7)
[warning] 56-56: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
[error] 57-57: Multiple statements on one line (colon)
(E701)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bot/cogs/logging.py` around lines 56 - 72, _process_log currently only
checks event_config.ignored_channels and never consults the global ignored list;
call the API helper get_logging_setting(str(guild.id), event_name) (from
src/bot/lib/api.py) to obtain the merged logging setting (which includes
global_ignored_channels and per-event ignored channels) and use that combined
ignored list to decide early-return; also ensure callers like channel_create and
channel_delete pass the trigger_channel_id through to _process_log so per-event
ignores are respected (reference symbols: _process_log, get_logging_setting,
event_config, global_ignored_channels, trigger_channel_id, channel_create,
channel_delete).
Comment on lines
+47
to
+55
| const handleSave = async () => { | ||
| if (loading) return; | ||
|
|
||
| setLoading(true); | ||
|
|
||
| if (setting.events.filter((value) => value.log_channel_id === "")) { | ||
| alert("ログには必ずチャンネルidを指定してください。"); | ||
| return | ||
| } |
There was a problem hiding this comment.
保存前バリデーションが常に失敗し、以後ずっと保存不能になります。
filter() は空配列でもオブジェクトとして真扱いされるため、Line 52 の条件は常に成立します。さらに Line 50 で loading を true にしたまま return しているので、最初のクリック以降は永続的に保存できません。
修正案
const handleSave = async () => {
if (loading) return;
- setLoading(true);
-
- if (setting.events.filter((value) => value.log_channel_id === "")) {
+ if (setting.events.some((value) => value.log_channel_id === "")) {
alert("ログには必ずチャンネルidを指定してください。");
- return
+ return;
}
+
+ setLoading(true);
try {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/src/app/dashboard/`[guildId]/logging/LoggingClient.tsx around
lines 47 - 55, The current handleSave incorrectly uses
setting.events.filter(...) in the validation which always evaluates truthy and
also leaves loading stuck true; change the validation to check for any empty
channel id (e.g., use Array.prototype.some on setting.events to detect
event.log_channel_id === "") and, if validation fails, call setLoading(false)
before returning so the component isn't stuck in loading state; update
references in the handleSave function to use the corrected predicate and ensure
setLoading is properly reset on all early returns or error paths.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
src/bot/cogs/logging.py (2)
139-148:⚠️ Potential issue | 🟠 Major
channel_create/channel_deleteにも対象 channel ID を渡してください。
_process_log()の ignore 判定はtrigger_channel_id前提ですが、監査ログ経由のチャンネルイベントでは未設定のまま呼んでいます。これだと channel 単位の ignore 設定が効かないので、entry.target.idを渡して同じ判定に乗せたいです。Also applies to: 161-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/logging.py` around lines 139 - 148, The channel_create and channel_delete audit-log branches don't pass the affected channel's ID into _process_log, so per-channel ignore rules aren't applied; update the handling for discord.AuditLogAction.channel_create and channel_delete to call _process_log with trigger_channel_id=entry.target.id (use entry.target.id as the unique channel identifier) so the same ignore logic applies as for other events; ensure you reference the branches that set event_key/embed (channel_create/channel_delete) and pass entry.target.id into the _process_log invocation used for audit log events.
94-95:⚠️ Potential issue | 🟠 Major本文が空の削除/編集もログ対象にしてください。
not message.content/not after.contentで早期 return しているため、画像・ファイル・スタンプだけのメッセージが欠落します。編集側もbefore.content == after.contentだけだと添付差分を見落とすので、本文がなくても添付情報やメッセージ ID を残した方がよいです。Also applies to: 106-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/logging.py` around lines 94 - 95, Remove the early-return that skips messages with empty content in the message handlers so image/file/sticker-only messages aren’t dropped: in the on_message handler (the function checking "if message.author.bot or not message.guild or not message.content: return"), stop checking "not message.content" and only return for bot/guild cases; in the on_message_edit handler (the block currently returning when "before.content == after.content"), extend the equality check to also compare attachments/stickers/embeds (e.g., compare before.attachments vs after.attachments, before.stickers vs after.stickers, and before.embeds vs after.embeds) and log when any of those differ, and always include message.id and attachment metadata in the logged payload so content-less messages are still recorded.src/dashboard/src/app/dashboard/[guildId]/logging/LoggingClient.tsx (1)
47-55:⚠️ Potential issue | 🔴 Critical保存前バリデーションが常に失敗し、
loadingも戻りません。
filter()は空配列でも truthy なのでこの条件は毎回成立します。さらにsetLoading(true)の後で return しているため、1 回目以降ずっと保存不能になります。💡 修正例
const handleSave = async () => { if (loading) return; - - setLoading(true); - - if (setting.events.filter((value) => value.log_channel_id === "")) { + if (setting.events.some((value) => value.log_channel_id === "")) { alert("ログには必ずチャンネルidを指定してください。"); - return + return; } + + setLoading(true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/logging/LoggingClient.tsx around lines 47 - 55, The save handler handleSave incorrectly uses setting.events.filter(...) as a boolean which always evaluates truthy and also calls setLoading(true) before returning, causing the save to always abort and loading to remain true; change the validation to check whether any event has an empty log_channel_id (e.g., use setting.events.some(ev => ev.log_channel_id === "") or check the filtered array's length > 0) and, if validation fails, call setLoading(false) before returning so loading is reset and the alert is shown; ensure you reference handleSave, setting.events, and setLoading when updating the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/src/internal/router/logging.go`:
- Around line 70-72: The handlers getOneLoggingEventConfig and
deleteOneLoggingEvent currently treat any db.Where(...).First(&setting).Error as
a 404; change their error handling to match getLoggingSetting: if err ==
gorm.ErrRecordNotFound return 404 with "Settings not found", otherwise log the
error and return HTTP 500 (internal server error) so transient DB/SQL errors
aren't misclassified; adjust the error branch around db.Where("guild_id = ?",
id).First(&setting).Error in both functions to check for gorm.ErrRecordNotFound
first and handle other errors as server errors.
In `@src/bot/cogs/logging.py`:
- Around line 12-13: 現在の _get_or_create_webhook()/event_config.webhook_url
ロジックはキャッシュされた webhook_url を無条件に返しており、webhook.send()
が失敗してもキャッシュを消さないため復旧できません。対処として _process_log() 呼び出し時に webhook.send() を
try/except でラップし、送信失敗時には event_config.webhook_url をクリアして永続化（保存）し、続けて
_get_or_create_webhook() を再呼び出して新しい webhook を取得して再試行するか、事前に webhook
の有効性チェックを行うよう修正してください。対象シンボル: event_config.webhook_url、_process_log(),
_get_or_create_webhook(), webhook.send().
- Around line 97-100: The embed fields currently use message.content,
before.content, and after.content directly which can exceed Discord's
1024-character limit and cause webhook.send() to 400; update the code that
builds the embed (where embed is created in the message deletion and message
edit handlers) to ensure each field value is at most 1024 chars—truncate long
strings (e.g., value[:1021] + '...') before embed.add_field or, for very long
messages, place the full text into embed.description or attach it as a file
instead; apply this truncation/relocation logic for the fields that use
message.content, before.content, and after.content.
---
Duplicate comments:
In `@src/bot/cogs/logging.py`:
- Around line 139-148: The channel_create and channel_delete audit-log branches
don't pass the affected channel's ID into _process_log, so per-channel ignore
rules aren't applied; update the handling for
discord.AuditLogAction.channel_create and channel_delete to call _process_log
with trigger_channel_id=entry.target.id (use entry.target.id as the unique
channel identifier) so the same ignore logic applies as for other events; ensure
you reference the branches that set event_key/embed
(channel_create/channel_delete) and pass entry.target.id into the _process_log
invocation used for audit log events.
- Around line 94-95: Remove the early-return that skips messages with empty
content in the message handlers so image/file/sticker-only messages aren’t
dropped: in the on_message handler (the function checking "if message.author.bot
or not message.guild or not message.content: return"), stop checking "not
message.content" and only return for bot/guild cases; in the on_message_edit
handler (the block currently returning when "before.content == after.content"),
extend the equality check to also compare attachments/stickers/embeds (e.g.,
compare before.attachments vs after.attachments, before.stickers vs
after.stickers, and before.embeds vs after.embeds) and log when any of those
differ, and always include message.id and attachment metadata in the logged
payload so content-less messages are still recorded.
In `@src/dashboard/src/app/dashboard/`[guildId]/logging/LoggingClient.tsx:
- Around line 47-55: The save handler handleSave incorrectly uses
setting.events.filter(...) as a boolean which always evaluates truthy and also
calls setLoading(true) before returning, causing the save to always abort and
loading to remain true; change the validation to check whether any event has an
empty log_channel_id (e.g., use setting.events.some(ev => ev.log_channel_id ===
"") or check the filtered array's length > 0) and, if validation fails, call
setLoading(false) before returning so loading is reset and the alert is shown;
ensure you reference handleSave, setting.events, and setLoading when updating
the logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7789d20-917c-4e18-8872-920983efc846
📒 Files selected for processing (5)
src/api/src/internal/model/LoggingSetting.gosrc/api/src/internal/router/logging.gosrc/bot/cogs/logging.pysrc/dashboard/src/app/dashboard/[guildId]/logging/LoggingClient.tsxsrc/dashboard/src/lib/api/requests.ts
✅ Files skipped from review due to trivial changes (2)
- src/api/src/internal/model/LoggingSetting.go
- src/dashboard/src/lib/api/requests.ts
Comment on lines
+70
to
+72
| if err := db.Where("guild_id = ?", id).First(&setting).Error; err != nil { | ||
| c.JSON(http.StatusNotFound, gin.H{"error": "Settings not found"}) | ||
| return |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/api/src/internal/router/logging.go | head -150Repository: SharkBot-Dev/NewSharkBot
Length of output: 4886
DB エラーをすべて 404 として返さないでください。
getLoggingSetting()（行31-38）と異なり、getOneLoggingEventConfig()（行70-72）と deleteOneLoggingEvent()（行128-130）は First() の失敗をすべて 404 に割り当てています。接続障害や SQL エラーまで 404 になると、呼び出し側が「未設定」と「一時障害」を区別できず、誤った再保存や誤表示につながります。
💡 修正例
var setting model.LoggingSetting
if err := db.Where("guild_id = ?", id).First(&setting).Error; err != nil {
- c.JSON(http.StatusNotFound, gin.H{"error": "Settings not found"})
- return
+ if errors.Is(err, gorm.ErrRecordNotFound) {
+ c.JSON(http.StatusNotFound, gin.H{"error": "Settings not found"})
+ return
+ }
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to load settings"})
+ return
}同じパターンを両関数に適用してください。getLoggingSetting() はすでに正しく処理されています。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if err := db.Where("guild_id = ?", id).First(&setting).Error; err != nil { | |
| c.JSON(http.StatusNotFound, gin.H{"error": "Settings not found"}) | |
| return | |
| if err := db.Where("guild_id = ?", id).First(&setting).Error; err != nil { | |
| if errors.Is(err, gorm.ErrRecordNotFound) { | |
| c.JSON(http.StatusNotFound, gin.H{"error": "Settings not found"}) | |
| return | |
| } | |
| c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to load settings"}) | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/src/internal/router/logging.go` around lines 70 - 72, The handlers
getOneLoggingEventConfig and deleteOneLoggingEvent currently treat any
db.Where(...).First(&setting).Error as a 404; change their error handling to
match getLoggingSetting: if err == gorm.ErrRecordNotFound return 404 with
"Settings not found", otherwise log the error and return HTTP 500 (internal
server error) so transient DB/SQL errors aren't misclassified; adjust the error
branch around db.Where("guild_id = ?", id).First(&setting).Error in both
functions to check for gorm.ErrRecordNotFound first and handle other errors as
server errors.
Comment on lines
+12
to
+13
| if event_config.webhook_url: | ||
| return event_config.webhook_url |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l src/bot/cogs/logging.pyRepository: SharkBot-Dev/NewSharkBot
Length of output: 95
🏁 Script executed:
cat -n src/bot/cogs/logging.pyRepository: SharkBot-Dev/NewSharkBot
Length of output: 8286
キャッシュされた webhook URL が無効になった場合、復旧できません
保存済み webhook_url をここで無条件に返しているため、Discord 側で webhook が手動削除・再生成されると、以降の webhook.send() 呼び出しが例外を発生させます。しかし _process_log() の呼び出し側で例外処理がないため、ログ送信は失敗したまま、キャッシュされた無効な URL は保存されたままになります。その後の呼び出しでも同じ無効な URL が返され続けるため、恒久的にログが送信されません。
webhook 送信時のエラーハンドリングを追加し、失敗時は保存済み URL をクリアして _get_or_create_webhook() を再試行する、または webhook の有効性を事前に確認する必要があります。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bot/cogs/logging.py` around lines 12 - 13, 現在の
_get_or_create_webhook()/event_config.webhook_url ロジックはキャッシュされた webhook_url
を無条件に返しており、webhook.send() が失敗してもキャッシュを消さないため復旧できません。対処として _process_log() 呼び出し時に
webhook.send() を try/except でラップし、送信失敗時には event_config.webhook_url
をクリアして永続化（保存）し、続けて _get_or_create_webhook() を再呼び出して新しい webhook を取得して再試行するか、事前に
webhook の有効性チェックを行うよう修正してください。対象シンボル: event_config.webhook_url、_process_log(),
_get_or_create_webhook(), webhook.send().
Comment on lines
+97
to
+100
| embed = discord.Embed(title="🗑️ メッセージ削除", color=discord.Color.red(), timestamp=message.created_at) | ||
| embed.add_field(name="送信者", value=f"{message.author.mention} (`{message.author.id}`)") | ||
| embed.add_field(name="チャンネル", value=message.channel.mention) | ||
| embed.add_field(name="内容", value=message.content, inline=False) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/bot/cogs/logging.py | sed -n '85,125p'Repository: SharkBot-Dev/NewSharkBot
Length of output: 2345
🏁 Script executed:
cat -n src/bot/cogs/logging.py | sed -n '1,95p' | tail -30Repository: SharkBot-Dev/NewSharkBot
Length of output: 1410
🏁 Script executed:
cat -n src/bot/cogs/logging.py | sed -n '55,90p'Repository: SharkBot-Dev/NewSharkBot
Length of output: 1679
🏁 Script executed:
# Check if there's any truncation logic elsewhere in the file
rg -n "truncate|1024|2000|\[:1024\]|\[:2000\]" src/bot/cogs/logging.pyRepository: SharkBot-Dev/NewSharkBot
Length of output: 50
🏁 Script executed:
# Check the full _process_log method and see if there's any error handling
cat -n src/bot/cogs/logging.py | sed -n '56,91p'Repository: SharkBot-Dev/NewSharkBot
Length of output: 1679
メッセージ内容が 1024 文字を超える場合、embed 送信が失敗します。
Discord の embed field 値は 1024 文字までですが、メッセージ本文は 2000 文字まで入ります。現在のコードはmessage.content、before.content、after.content を検証なしに embed field に直接割り当てているため、長文メッセージの場合 webhook.send() が 400 エラーで失敗します。各 field を 1024 文字以下に切り詰めるか、embed.description に逃がす必要があります。
該当箇所
行 97-100（メッセージ削除）:
embed.add_field(name="内容", value=message.content, inline=False)行 109-113（メッセージ編集）:
embed.add_field(name="変更前", value=before.content, inline=False)
embed.add_field(name="変更後", value=after.content, inline=False)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bot/cogs/logging.py` around lines 97 - 100, The embed fields currently
use message.content, before.content, and after.content directly which can exceed
Discord's 1024-character limit and cause webhook.send() to 400; update the code
that builds the embed (where embed is created in the message deletion and
message edit handlers) to ensure each field value is at most 1024 chars—truncate
long strings (e.g., value[:1021] + '...') before embed.add_field or, for very
long messages, place the full text into embed.description or attach it as a file
instead; apply this truncation/relocation logic for the fields that use
message.content, before.content, and after.content.
There was a problem hiding this comment.
♻️ Duplicate comments (5)
src/bot/cogs/logging.py (4)
100-100:⚠️ Potential issue | 🟠 MajorEmbed field の文字数上限未考慮でWebhook送信が失敗します。
message.content/before.content/after.contentをそのまま field に入れており、長文時に Discord API 側で 400 になります。field 値は 1024 文字以内へ切り詰める処理を追加してください。✂️ 例: field用の共通トリムを追加
+ def _trim_field(self, value: str) -> str: + if value is None: + return "(空)" + return value if len(value) <= 1024 else value[:1021] + "..." - embed.add_field(name="内容", value=message.content, inline=False) + embed.add_field(name="内容", value=self._trim_field(message.content), inline=False) - embed.add_field(name="変更前", value=before.content, inline=False) - embed.add_field(name="変更後", value=after.content, inline=False) + embed.add_field(name="変更前", value=self._trim_field(before.content), inline=False) + embed.add_field(name="変更後", value=self._trim_field(after.content), inline=False)Also applies to: 112-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/logging.py` at line 100, The embed field is using message.content (and similarly before.content / after.content) raw which can exceed Discord's 1024-character limit and cause webhook 400 errors; add a small helper (e.g., trim_embed_field or truncate_field) that truncates strings to 1024 chars (optionally appending "…" when cut) and use it when calling embed.add_field for the message.content, before.content and after.content fields in the logging cog so all field values are guaranteed <=1024 characters.
12-13:⚠️ Potential issue | 🟠 Major無効なWebhook URLを自己回復できない実装です。
Line 12 で
webhook_urlを無条件再利用し、Line 86 のwebhook.send()失敗時の再取得処理がないため、URL失効後に恒久的に送信不能になります。送信失敗時にwebhook_urlをクリアして再作成・再送するフローを入れてください。Also applies to: 85-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/logging.py` around lines 12 - 13, The code unconditionally reuses event_config.webhook_url (event_config.webhook_url) and never recovers if the webhook becomes invalid, so wrap the webhook.send() call in a try/except and on failure clear event_config.webhook_url, persist that change, recreate the webhook via the existing helper used to obtain webhooks (e.g., the module's get_or_create_webhook / create_webhook_for_channel function), update event_config.webhook_url with the new URL, and retry the send once; ensure you catch specific send errors, persist the cleared/updated event_config state, and avoid infinite retry loops.
94-95:⚠️ Potential issue | 🟠 Major本文なしメッセージを除外しており、削除/編集ログが欠損します。
Line 94 と Line 106 の早期 return により、添付・スタンプのみのメッセージ削除/編集がログされません。本文空でも
(添付のみ)と添付情報を含めて記録するべきです。Also applies to: 106-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/logging.py` around lines 94 - 95, 早期 return が message.content のみを条件にしているため、本文なし（添付・スタンプのみ）のメッセージが削除/編集ログから除外されています; message.author.bot と not message.guild のチェックは保持しつつ、message.content が空でも message.attachments / message.stickers / message.embeds 等が存在する場合は処理を継続するよう、該当の早期 return 条件（現在の message.author.bot or not message.guild or not message.content）を修正してください。さらに、ログ作成箇所（削除/編集のハンドラで使われている message 内容を組み立てるロジック）に「(添付のみ)」というプレースホルダを導入し、添付情報（attachment.filename/URL など）やスタンプ情報をログ本文に含めるよう更新してください。参照シンボル: message.author.bot, message.guild, message.content, message.attachments, message.stickers, 該当の削除/編集ログハンドラ関数。
139-140:⚠️ Potential issue | 🟠 Majorチャンネル系監査イベントでignore判定が効いていません。
channel_create/channel_deleteでも_process_logにtrigger_channel_idを渡していないため、グローバル/イベント別の無視チャンネル設定が適用されません。🎯 最小修正例
- if event_key: + if event_key: embed.add_field(name="実行者", value=f"{entry.user.mention}") if entry.reason: embed.add_field(name="理由", value=entry.reason, inline=False) - await self._process_log(guild, event_key, embed) + trigger_channel_id = None + if event_key in {"channel_create", "channel_delete"} and getattr(entry.target, "id", None): + trigger_channel_id = str(entry.target.id) + await self._process_log(guild, event_key, embed, trigger_channel_id)Also applies to: 145-146, 165-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/logging.py` around lines 139 - 140, The channel audit handlers for discord.AuditLogAction.channel_create and channel_delete are not passing the trigger_channel_id into _process_log, so per-channel/global ignore settings never apply; update the calls to _process_log in the blocks handling channel_create (event_key "channel_create"), channel_delete (event_key "channel_delete") and the related channel event block referenced around the same area to pass trigger_channel_id (use the channel target's id, e.g., target.id) as the corresponding argument to _process_log so the ignore filtering logic receives the channel id.src/dashboard/src/app/dashboard/[guildId]/logging/LoggingClient.tsx (1)
49-54:⚠️ Potential issue | 🔴 Criticalバリデーション失敗時に
loadingが固定化し、以後保存できません。Line 49 で
setLoading(true)した後、Line 53 で return しておりloadingが戻りません。次回以降は Line 47 のガードで常時中断されます。✅ 修正例
const handleSave = async () => { if (loading) return; - setLoading(true); - if (setting.events.some((event) => event.log_channel_id === "")) { alert("ログには必ずチャンネルidを指定してください。"); return; } + + setLoading(true); try { const response = await fetch(`/api/guilds/${guildId}/modules/logging`, {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/logging/LoggingClient.tsx around lines 49 - 54, The code sets setLoading(true) before validating events and returns early when an event has an empty log_channel_id, leaving loading stuck; either perform the validation on setting.events before calling setLoading(true) or ensure you call setLoading(false) immediately before the early return in the save handler so loading is reset; locate the save handler where setLoading(true) is called and the guard that checks setting.events.some(event => event.log_channel_id === "") and implement one of these fixes (move validation above setLoading(true) or add setLoading(false) just before the return).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/bot/cogs/logging.py`:
- Line 100: The embed field is using message.content (and similarly
before.content / after.content) raw which can exceed Discord's 1024-character
limit and cause webhook 400 errors; add a small helper (e.g., trim_embed_field
or truncate_field) that truncates strings to 1024 chars (optionally appending
"…" when cut) and use it when calling embed.add_field for the message.content,
before.content and after.content fields in the logging cog so all field values
are guaranteed <=1024 characters.
- Around line 12-13: The code unconditionally reuses event_config.webhook_url
(event_config.webhook_url) and never recovers if the webhook becomes invalid, so
wrap the webhook.send() call in a try/except and on failure clear
event_config.webhook_url, persist that change, recreate the webhook via the
existing helper used to obtain webhooks (e.g., the module's
get_or_create_webhook / create_webhook_for_channel function), update
event_config.webhook_url with the new URL, and retry the send once; ensure you
catch specific send errors, persist the cleared/updated event_config state, and
avoid infinite retry loops.
- Around line 94-95: 早期 return が message.content
のみを条件にしているため、本文なし（添付・スタンプのみ）のメッセージが削除/編集ログから除外されています; message.author.bot と not
message.guild のチェックは保持しつつ、message.content が空でも message.attachments /
message.stickers / message.embeds 等が存在する場合は処理を継続するよう、該当の早期 return 条件（現在の
message.author.bot or not message.guild or not
message.content）を修正してください。さらに、ログ作成箇所（削除/編集のハンドラで使われている message
内容を組み立てるロジック）に「(添付のみ)」というプレースホルダを導入し、添付情報（attachment.filename/URL
など）やスタンプ情報をログ本文に含めるよう更新してください。参照シンボル: message.author.bot, message.guild,
message.content, message.attachments, message.stickers, 該当の削除/編集ログハンドラ関数。
- Around line 139-140: The channel audit handlers for
discord.AuditLogAction.channel_create and channel_delete are not passing the
trigger_channel_id into _process_log, so per-channel/global ignore settings
never apply; update the calls to _process_log in the blocks handling
channel_create (event_key "channel_create"), channel_delete (event_key
"channel_delete") and the related channel event block referenced around the same
area to pass trigger_channel_id (use the channel target's id, e.g., target.id)
as the corresponding argument to _process_log so the ignore filtering logic
receives the channel id.
In `@src/dashboard/src/app/dashboard/`[guildId]/logging/LoggingClient.tsx:
- Around line 49-54: The code sets setLoading(true) before validating events and
returns early when an event has an empty log_channel_id, leaving loading stuck;
either perform the validation on setting.events before calling setLoading(true)
or ensure you call setLoading(false) immediately before the early return in the
save handler so loading is reset; locate the save handler where setLoading(true)
is called and the guard that checks setting.events.some(event =>
event.log_channel_id === "") and implement one of these fixes (move validation
above setLoading(true) or add setLoading(false) just before the return).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2a972b55-3a89-4238-b568-3b9877df1f0f
📒 Files selected for processing (3)
src/bot/cogs/logging.pysrc/dashboard/src/app/api/guilds/[guildId]/modules/logging/route.tssrc/dashboard/src/app/dashboard/[guildId]/logging/LoggingClient.tsx
✅ Files skipped from review due to trivial changes (1)
- src/dashboard/src/app/api/guilds/[guildId]/modules/logging/route.ts
This was referenced Mar 29, 2026
Merged
Merged
Merged
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
新機能
改善