feat(stamp): スタンプのレコメンデーションを本番実装に移行し、スタンプ履歴の実装を削除#5077
Open
feat(stamp): スタンプのレコメンデーションを本番実装に移行し、スタンプ履歴の実装を削除#5077
Conversation
|
Preview (prod) → https://5077-prod.traq-preview.trapti.tech/ |
📝 WalkthroughWalkthroughスタンプ履歴ストアを削除し、履歴依存を stampRecommendations（recommendation）へ一本化。ローカル履歴の fetch/upsert 呼び出しを削除し、型・識別子を Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5077 +/- ##
==========================================
+ Coverage 62.60% 63.11% +0.51%
==========================================
Files 108 106 -2
Lines 3097 3045 -52
Branches 630 621 -9
==========================================
- Hits 1939 1922 -17
+ Misses 1048 1015 -33
+ Partials 110 108 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/assets/mdi.ts (1)
54-54:⚠️ Potential issue | 🟡 Minor
mdiHistoryインポートとhistoryマッピングは不要になっているため削除してくださいスタンプ履歴機能の削除に伴い、
mdiHistoryインポート（Line 54）とhistory: mdiHistoryマッピング（Line 120）は他のコンポーネントで使用されていません。これらは削除可能な死コードです。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/mdi.ts` at line 54, Remove the unused mdiHistory import and its mapping to eliminate dead code: delete the mdiHistory import symbol from the top-level imports in src/assets/mdi.ts and remove the history: mdiHistory entry from the exported mapping/object (where icons like mdiX are aggregated), ensuring no other code references mdiHistory before committing.
🧹 Nitpick comments (2)
src/store/ui/stampPicker.ts (1)
88-92:ensureCurrentStampSetValid内の直接プロパティ変更についてLines 90-91 では
currentStampSet.value.typeと.idを直接変更していますが、currentStampSetの setter（Lines 81-84）ではnewValueオブジェクトを受け取ってstate.typeとstate.idに代入する設計になっています。setter を経由してcurrentStampSet.value = { type: 'recommendation', id: '' }とした方が一貫性があり、将来 setter にロジックが追加された場合にも安全です。♻️ setter を経由する提案
const ensureCurrentStampSetValid = () => { if (isStampSetValid(currentStampSet.value)) return - currentStampSet.value.type = 'recommendation' - currentStampSet.value.id = '' + currentStampSet.value = { type: 'recommendation', id: '' } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/ui/stampPicker.ts` around lines 88 - 92, The function ensureCurrentStampSetValid mutates currentStampSet.value properties directly (currentStampSet.value.type and .id); change it to assign a new object via the existing currentStampSet setter (i.e., set currentStampSet.value to an object with type 'recommendation' and id '' ) so the setter logic in currentStampSet is invoked and future side effects/validation remain consistent.src/components/Settings/StampPaletteTab/StampPaletteEditorAddableStampList.vue (1)
75-80:Array.includesによる O(n²) ルックアップ — 大規模リスト時のパフォーマンスに注意
stampRecommendations.value.includes(stamp.id)（Line 79）はfilteredItemsの全要素に対してO(n)の線形探索を行うため、スタンプ数が多い場合は全体でO(n²)になります。レコメンデーション件数が常に少量であれば実害はありませんが、Setを使うと意図が明確になります。♻️ 修正案
const allAddableStamps = computed(() => { if (!stampsMapFetched.value) { return [] } + const recommendedSet = new Set(stampRecommendations.value) return ( filterState.query === '' ? [ ...stampRecommendations.value .map(id => stampsMap.value.get(id)) .filter(stamp => stamp !== undefined), ...filterState.filteredItems - .filter(stamp => !stampRecommendations.value.includes(stamp.id)) + .filter(stamp => !recommendedSet.has(stamp.id)) .sort((a, b) => a.name.localeCompare(b.name)) ] : filterState.filteredItems ).filter(stamp => !currentStampIds.value.includes(stamp.id)) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/StampPaletteTab/StampPaletteEditorAddableStampList.vue` around lines 75 - 80, The current filter uses stampRecommendations.value.includes(stamp.id) causing O(n²) behavior; change to build a Set from stampRecommendations.value (e.g., const recommendedIds = new Set(stampRecommendations.value)) and then use recommendedIds.has(stamp.id) when filtering filterState.filteredItems; update the logic around stampRecommendations, stampsMap and filterState.filteredItems to reference the Set so lookups become O(1) and preserve the existing mapping/sorting behavior.
🤖 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/components/Settings/StampPaletteTab/StampPaletteEditorAddableStampList.vue`:
- Around line 61-62: The code calls useStampRecommendations() twice; replace
both calls with a single call and destructure both values at once (e.g., const {
fetchStampRecommendations, stampRecommendations } = useStampRecommendations())
so fetchStampRecommendations and stampRecommendations are obtained from one
invocation of useStampRecommendations; update any references to the previous
separate variables accordingly.
---
Outside diff comments:
In `@src/assets/mdi.ts`:
- Line 54: Remove the unused mdiHistory import and its mapping to eliminate dead
code: delete the mdiHistory import symbol from the top-level imports in
src/assets/mdi.ts and remove the history: mdiHistory entry from the exported
mapping/object (where icons like mdiX are aggregated), ensuring no other code
references mdiHistory before committing.
---
Nitpick comments:
In
`@src/components/Settings/StampPaletteTab/StampPaletteEditorAddableStampList.vue`:
- Around line 75-80: The current filter uses
stampRecommendations.value.includes(stamp.id) causing O(n²) behavior; change to
build a Set from stampRecommendations.value (e.g., const recommendedIds = new
Set(stampRecommendations.value)) and then use recommendedIds.has(stamp.id) when
filtering filterState.filteredItems; update the logic around
stampRecommendations, stampsMap and filterState.filteredItems to reference the
Set so lookups become O(1) and preserve the existing mapping/sorting behavior.
In `@src/store/ui/stampPicker.ts`:
- Around line 88-92: The function ensureCurrentStampSetValid mutates
currentStampSet.value properties directly (currentStampSet.value.type and .id);
change it to assign a new object via the existing currentStampSet setter (i.e.,
set currentStampSet.value to an object with type 'recommendation' and id '' ) so
the setter logic in currentStampSet is invoked and future side
effects/validation remain consistent.
src/components/Settings/StampPaletteTab/StampPaletteEditorAddableStampList.vue
Outdated
Show resolved
Hide resolved
…mmendations()`を1回に統一
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/Settings/StampPaletteTab/StampPaletteEditorAddableStampList.vue (1)
75-77: 型推論は正常に機能しており、明示的な型述語は不要です。プロジェクトのTypeScriptバージョンは5.9.3であり、TypeScript 5.5以上のため、
.map(...).filter(stamp => stamp !== undefined)は正しくStamp[]に推論されます。現在のコードは型エラーなく動作します。より明示的にしたい場合は、以下のように型述語を追加することで可読性を向上させることができます：
♻️ オプション：明示的な型述語
- .filter(stamp => stamp !== undefined), + .filter((stamp): stamp is NonNullable<typeof stamp> => stamp !== undefined),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/StampPaletteTab/StampPaletteEditorAddableStampList.vue` around lines 75 - 77, The explicit type-guard predicate is unnecessary because TypeScript 5.5+ already narrows the type from stampRecommendations.value.map(id => stampsMap.value.get(id)).filter(...); remove any explicit type predicate and keep the chain as shown (stampRecommendations.value.map(id => stampsMap.value.get(id)).filter(stamp => stamp !== undefined)) or, if you prefer an explicit assertion for readability, replace the filter with a proper type predicate like (stamp): stamp is Stamp => stamp !== undefined; locate this logic around stampRecommendations.value, stampsMap.value.get, and the subsequent filter to apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/components/Settings/StampPaletteTab/StampPaletteEditorAddableStampList.vue`:
- Around line 75-77: The explicit type-guard predicate is unnecessary because
TypeScript 5.5+ already narrows the type from stampRecommendations.value.map(id
=> stampsMap.value.get(id)).filter(...); remove any explicit type predicate and
keep the chain as shown (stampRecommendations.value.map(id =>
stampsMap.value.get(id)).filter(stamp => stamp !== undefined)) or, if you prefer
an explicit assertion for readability, replace the filter with a proper type
predicate like (stamp): stamp is Stamp => stamp !== undefined; locate this logic
around stampRecommendations.value, stampsMap.value.get, and the subsequent
filter to apply the change.
ref: https://ja.vuejs.org/guide/essentials/computed.html#avoid-mutating-computed-value > 算出プロパティから返る値は、派生した状態です。一時的なスナップショットとして考えてください。ソースの状態が変わるたびに、新しいスナップショットが作成されます。スナップショットの値を変更することは意味がないため、計算された結果は読み取り専用として扱い、変更しないようにします。その代わり、新しい計算結果が必要な場合は、依存するソースの状態を更新します。
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/store/ui/stampPicker.ts`:
- Around line 87-91: Currently the store calls useStampSetSelector() just to get
isStampSetValid, which instantiates heavy composables (useStampCategory,
useStampPalettesStore) and creates unwanted reactive subscriptions; refactor by
extracting isStampSetValid as a standalone named export from useStampSetSelector
(export function isStampSetValid(...)) and then import that function directly in
src/store/ui/stampPicker.ts; update ensureCurrentStampSetValid to call the new
imported isStampSetValid(currentStampSet.value) instead of instantiating
useStampSetSelector, leaving currentStampSet.value assignment logic unchanged;
ensure no other store code calls useStampSetSelector to avoid reversing
dependency direction.
Comment on lines
+87
to
91
| const { isStampSetValid } = useStampSetSelector() | ||
| const ensureCurrentStampSetValid = () => { | ||
| if (isStampSetValid(currentStampSet.value)) return | ||
| currentStampSet.value = { type: 'recommendation', id: '' } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# Find the useStampSetSelector file
fd -e ts "useStampSetSelector" --type fRepository: traPtitech/traQ_S-UI
Length of output: 130
🏁 Script executed:
# Also search for the stampPicker file
fd -e ts "stampPicker" --type fRepository: traPtitech/traQ_S-UI
Length of output: 91
🏁 Script executed:
# Read the useStampSetSelector.ts file
cat -n "src/components/Main/StampPicker/composables/useStampSetSelector.ts"Repository: traPtitech/traQ_S-UI
Length of output: 1970
🏁 Script executed:
# Read the stampPicker.ts store file
cat -n "src/store/ui/stampPicker.ts"Repository: traPtitech/traQ_S-UI
Length of output: 6169
🏁 Script executed:
# Search for other usages of useStampSetSelector
rg -n "useStampSetSelector" --type ts -C 2Repository: traPtitech/traQ_S-UI
Length of output: 2137
useStampSetSelector() をストア内で呼び出してバリデーション関数のみ取得している
isStampSetValid の取得だけのために useStampSetSelector() をストア内でフルインスタンス化しています。コンポーザブルは useStampCategory() と useStampPalettesStore() をインスタンス化時に呼び出し、さらに5つの計算プロパティを持つ大規模なリアクティブ状態を作成します。これらはストアのコンテキストで実行されるため、不要なリアクティブ状態と複数のストア購読が生成されます。
また依存方向として、ストア層（下位）がコンポーネント層のコンポーザブル（上位）を参照するのは逆方向の依存になります。
isStampSetValid は useStampSetSelector.ts からスタンドアロンの名前付きエクスポートとして切り出すことを検討してください。
♻️ 改善案
useStampSetSelector.ts 側:
+export function isStampSetValid(stampSet: StampSet): boolean {
+ // 既存の検証ロジックをここに移動
+}
+
export default function useStampSetSelector() {
// ...
- const isStampSetValid = (stampSet: StampSet): boolean => { ... }
- return { ..., isStampSetValid }
+ return { ..., isStampSetValid }
}stampPicker.ts 側:
-import useStampSetSelector, {
+import {
type StampSet
-} from '/@/components/Main/StampPicker/composables/useStampSetSelector'
+ isStampSetValid
+} from '/@/components/Main/StampPicker/composables/useStampSetSelector'
- const { isStampSetValid } = useStampSetSelector()
const ensureCurrentStampSetValid = () => {
if (isStampSetValid(currentStampSet.value)) return
currentStampSet.value = { type: 'recommendation', id: '' }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/store/ui/stampPicker.ts` around lines 87 - 91, Currently the store calls
useStampSetSelector() just to get isStampSetValid, which instantiates heavy
composables (useStampCategory, useStampPalettesStore) and creates unwanted
reactive subscriptions; refactor by extracting isStampSetValid as a standalone
named export from useStampSetSelector (export function isStampSetValid(...)) and
then import that function directly in src/store/ui/stampPicker.ts; update
ensureCurrentStampSetValid to call the new imported
isStampSetValid(currentStampSet.value) instead of instantiating
useStampSetSelector, leaving currentStampSet.value assignment logic unchanged;
ensure no other store code calls useStampSetSelector to avoid reversing
dependency direction.
Takeno-hito
reviewed
Feb 22, 2026
Member
Takeno-hito
left a comment
Takeno-hito
left a comment
There was a problem hiding this comment.
本番実装で統一的に入れ替えるのはちょっと今のままだと懐疑的かなぁ… traQ だと特に 2個3個のスタンプを連続で押す、というユースケースがそこそこあって、それが非常にしづらくなると思うな
レコメンデーションをなぜ入れたいのかみたいな理由が結構ほしい
cp-20
approved these changes
Feb 23, 2026
Contributor
cp-20
left a comment
cp-20
left a comment
There was a problem hiding this comment.
コード自体は良いと思います (機能の是非はさておき)
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
3 participants
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.
概要
as titled
なぜこの PR を入れたいのか
動作確認の手順
UI 変更部分のスクリーンショット
before
省略
after
PR を出す前の確認事項
見てほしいところ・聞きたいことなど
Summary by CodeRabbit
New Features
Refactor
Bug Fixes