メンターメモに作成者と作成時間を追加し、１対多の関係に変更#10121
Open
matuaya wants to merge 29 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthrough
Changesメンターメモ管理機能
Sequence Diagram(s)sequenceDiagram
actor Mentor as メンター
participant Browser as ブラウザ
participant API as API::MentorMemosController
participant DB as mentor_memos テーブル
Mentor->>Browser: 追加/編集/削除の操作
Browser->>API: POST / PUT / DELETE
API->>API: set_user → set_memo → authorize_memo
API->>DB: save / update / destroy
DB-->>API: 結果
API-->>Browser: HTML / 200 / 204 / 400 / 403
Browser->>Browser: DOM 更新と表示切替
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/controllers/users_controller.rb (1)
51-51: ⚡ Quick winメモ取得を権限ユーザーに限定すると漏洩耐性が上がります。
Line 51 は常にメモをロードしているため、ビュー条件が崩れたときに機微情報が露出しやすくなります。取得自体を
current_user.admin_or_mentor?で絞るのが安全です。修正案
- `@mentor_memos` = `@user.received_memos.includes`(:writer).order(created_at: :desc) + `@mentor_memos` = + if current_user&.admin_or_mentor? + `@user.received_memos.includes`(:writer).order(created_at: :desc) + else + MentorMemo.none + end🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/controllers/users_controller.rb` at line 51, 現在のコード eagerly loads `@mentor_memos` unconditionally which can expose sensitive memos if view logic changes; guard the fetch by checking current_user.admin_or_mentor? before calling `@user.received_memos.includes`(...).order(...), and when the check fails ensure `@mentor_memos` is set to nil or an empty relation/array so views don't accidentally display data; locate this in UsersController where `@mentor_memos` is assigned and wrap or replace that assignment with the conditional using current_user.admin_or_mentor?.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/controllers/api/mentor_memos_controller.rb`:
- Around line 22-34: The update and destroy actions in MentorMemosController
lack an ownership check allowing other mentors to modify memos; add an
authorization guard that enforces only the memo writer or an admin may perform
updates/deletes. Implement a before_action like authorize_mentor_memo_owner! for
actions :update and :destroy, and implement authorize_mentor_memo_owner! to
return head :forbidden (or raise) unless current_user.id ==
`@mentor_memo.writer_id` || current_user.admin?; apply this to the existing update
and destroy flows so the writer or admin is required.
In `@app/javascript/initializeMentorMemo.js`:
- Around line 65-70: The emptyMessage element can be null (when initial state
has memos), causing a runtime error when calling classList.remove; update the
logic around mentorMemos/emptyMessage in initializeMentorMemo.js to guard before
manipulating DOM: ensure mentorMemos is found, then check emptyMessage is
non-null before calling classList.remove (or use optional chaining), and keep
using memoList.length to determine empty state so the remove is only invoked
when emptyMessage exists.
In `@app/javascript/mentor-memos.js`:
- Around line 61-75: After clearing the editor in the open and cancel handlers,
also reset the save-button state by setting saveButton.disabled = true so an
empty memo cannot be submitted; update the block that clears editorTextarea and
memoEditorPreview (the handler that references toggleClass(modalElements,
'is-hidden'), editorTextarea, memoEditorPreview, and emptyMessage) and the
cancelButton click handler to disable saveButton after clearing the input and
hiding the empty message.
---
Nitpick comments:
In `@app/controllers/users_controller.rb`:
- Line 51: 現在のコード eagerly loads `@mentor_memos` unconditionally which can expose
sensitive memos if view logic changes; guard the fetch by checking
current_user.admin_or_mentor? before calling
`@user.received_memos.includes`(...).order(...), and when the check fails ensure
`@mentor_memos` is set to nil or an empty relation/array so views don't
accidentally display data; locate this in UsersController where `@mentor_memos` is
assigned and wrap or replace that assignment with the conditional using
current_user.admin_or_mentor?.
🪄 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: a9843ed5-c590-47d4-9b24-74e93137e473
📒 Files selected for processing (14)
app/controllers/api/mentor_memos_controller.rbapp/controllers/users_controller.rbapp/javascript/initializeMentorMemo.jsapp/javascript/mentor-memos.jsapp/javascript/packs/application.jsapp/javascript/user_mentor_memo.jsapp/models/mentor_memo.rbapp/models/user.rbapp/views/users/_mentor_memo.html.slimapp/views/users/_user_mentor_memos.html.slimapp/views/users/show.html.slimconfig/routes/api.rbdb/migrate/20260126070916_create_mentor_memos.rbdb/schema.rb
💤 Files with no reviewable changes (1)
- app/javascript/user_mentor_memo.js
Contributor
Author
|
@coderabbitai pause |
Contributor
✅ Action performedReviews paused. |
matuaya
force-pushed
the
feature/add-author-and-timestamp-to-mentor-memos
branch
3 times, most recently
from
91a02c2 to
9fd75b3
Compare
Contributor
Author
|
@coderabbitai resume |
Contributor
✅ Action performedReviews resumed. |
matuaya
force-pushed
the
feature/add-author-and-timestamp-to-mentor-memos
branch
from
9fd75b3 to
07f5848
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
matuaya
force-pushed
the
feature/add-author-and-timestamp-to-mentor-memos
branch
3 times, most recently
from
1469b5f to
0ee0726
Compare
Contributor
Author
|
@machida |
machida
force-pushed
the
feature/add-author-and-timestamp-to-mentor-memos
branch
from
0ee0726 to
e00ee87
Compare
Member
|
@matuaya お待たせしました！！デザイン入れましたー |
machida
force-pushed
the
feature/add-author-and-timestamp-to-mentor-memos
branch
from
5114338 to
a0d69da
Compare
Member
|
@matuaya 最新のmainをrebaseしました〜 |
Contributor
Author
|
@machida |
Contributor
Author
|
@machida これまでの実装ではメモの作成者の情報はなかったのですが、今回の実装で追加したMentorMemo テーブルではメモ作成者のカラムが存在します。 |
Member
Member
|
＠machida (CC: @matuaya ) おっしゃるとおりでいいと思います〜 |
matuaya
force-pushed
the
feature/add-author-and-timestamp-to-mentor-memos
branch
from
a0d69da to
1c3507c
Compare
matuaya
force-pushed
the
feature/add-author-and-timestamp-to-mentor-memos
branch
from
1c3507c to
6a42459
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/assets/stylesheets/application/blocks/user/_mentor-memos.css`:
- Around line 25-50: The `.mentor-memo__meta` flex row is too rigid for narrow
screens, causing the writer and `__created-at` metadata to overflow or collide.
Update the styling in `mentor-memos.css` so the meta block can wrap or stack on
small widths, and make `__writer-link`/`__created-at` less restrictive (for
example by allowing wrapping or moving the date to a new line on mobile). Use
the existing `.mentor-memo__meta`, `.mentor-memo__writer-link`, and
`.mentor-memo__created-at` selectors to locate the fix.
🪄 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: 5d8f56fd-e018-487a-9a8c-d8dd20e3e7e8
📒 Files selected for processing (8)
app/assets/stylesheets/application.cssapp/assets/stylesheets/application/blocks/user/_mentor-memos.cssapp/controllers/api/mentor_memos_controller.rbapp/controllers/products_controller.rbapp/controllers/reports_controller.rbapp/controllers/talks_controller.rbapp/controllers/users_controller.rbapp/javascript/importmap_application.js
✅ Files skipped from review due to trivial changes (3)
- app/controllers/talks_controller.rb
- app/controllers/reports_controller.rb
- app/controllers/users_controller.rb
🚧 Files skipped from review as they are similar to previous changes (2)
- app/controllers/products_controller.rb
- app/controllers/api/mentor_memos_controller.rb
Comment on lines
+25
to
+50
| .mentor-memo__meta { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 0.375rem; | ||
| margin-bottom: 0.5rem; | ||
| } | ||
|
|
||
| .mentor-memo__writer-link { | ||
| display: inline-flex; | ||
| flex: 0 0 auto; | ||
| line-height: 1; | ||
| } | ||
|
|
||
| .mentor-memo__writer { | ||
| font-size: 0.875rem; | ||
| line-height: 1.4; | ||
| font-weight: 600; | ||
| } | ||
|
|
||
| .mentor-memo__created-at { | ||
| margin-left: auto; | ||
| font-size: 0.75rem; | ||
| line-height: 1.4; | ||
| white-space: nowrap; | ||
| color: var(--muted-text); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
狭い画面でメタ情報が横にはみ出します。
.mentor-memo__meta が1行固定の flex で、__writer-link も __created-at もほぼ縮まないため、ユーザー名が長いケースだと日時と衝突してカード幅を超えます。折り返しを許可するか、少なくともモバイルでは日時を次行へ落とせるようにしておいた方が安全です。
💡 修正例
.mentor-memo__meta {
display: flex;
align-items: center;
+ flex-wrap: wrap;
gap: 0.375rem;
margin-bottom: 0.5rem;
}
.mentor-memo__writer-link {
display: inline-flex;
- flex: 0 0 auto;
+ flex: 0 1 auto;
+ min-width: 0;
line-height: 1;
}
+@media (width <= 47.9375em) {
+ .mentor-memo__created-at {
+ margin-left: 0;
+ }
+}📝 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
| .mentor-memo__meta { | |
| display: flex; | |
| align-items: center; | |
| gap: 0.375rem; | |
| margin-bottom: 0.5rem; | |
| } | |
| .mentor-memo__writer-link { | |
| display: inline-flex; | |
| flex: 0 0 auto; | |
| line-height: 1; | |
| } | |
| .mentor-memo__writer { | |
| font-size: 0.875rem; | |
| line-height: 1.4; | |
| font-weight: 600; | |
| } | |
| .mentor-memo__created-at { | |
| margin-left: auto; | |
| font-size: 0.75rem; | |
| line-height: 1.4; | |
| white-space: nowrap; | |
| color: var(--muted-text); | |
| } | |
| .mentor-memo__meta { | |
| display: flex; | |
| align-items: center; | |
| flex-wrap: wrap; | |
| gap: 0.375rem; | |
| margin-bottom: 0.5rem; | |
| } | |
| .mentor-memo__writer-link { | |
| display: inline-flex; | |
| flex: 0 1 auto; | |
| min-width: 0; | |
| line-height: 1; | |
| } | |
| .mentor-memo__writer { | |
| font-size: 0.875rem; | |
| line-height: 1.4; | |
| font-weight: 600; | |
| } | |
| .mentor-memo__created-at { | |
| margin-left: auto; | |
| font-size: 0.75rem; | |
| line-height: 1.4; | |
| white-space: nowrap; | |
| color: var(--muted-text); | |
| } | |
| `@media` (width <= 47.9375em) { | |
| .mentor-memo__created-at { | |
| margin-left: 0; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/assets/stylesheets/application/blocks/user/_mentor-memos.css` around
lines 25 - 50, The `.mentor-memo__meta` flex row is too rigid for narrow
screens, causing the writer and `__created-at` metadata to overflow or collide.
Update the styling in `mentor-memos.css` so the meta block can wrap or stack on
small widths, and make `__writer-link`/`__created-at` less restrictive (for
example by allowing wrapping or moving the date to a new line on mobile). Use
the existing `.mentor-memo__meta`, `.mentor-memo__writer-link`, and
`.mentor-memo__created-at` selectors to locate the fix.
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.
Issue
概要
今まではメンターメモはUserの属性として扱われていたので作成者や作成日時などの情報をもつことはできなかったが、本PRでメンターメモとUserを１対多の関係になるように再設計し、個々のデータとして管理できるように変更した。
変更確認方法
feature/add-author-and-timestamp-to-mentor-memosをローカルに取り込むkomagataでログインし、任意のユーザーのプロフィールに移動するmentormentaroでログインし、先ほどと同じユーザーのプロフィールに移動するkomagataで作成したメモの編集ボタンと削除ボタンが表示されず、メモの編集と削除ができないことを確認するScreenshot
変更前
変更後
Flakewatch Report
Summary by CodeRabbit
Review App