Add bookmark edit and delete actions#2
Open
goofmint wants to merge 1 commit into
Open
Conversation
📝 Walkthrough概要BookmarkController に編集・削除の POST エンドポイントを追加し、BookmarkRepository の UPDATE / DELETE 操作、CSS スタイル設定、Thymeleaf テンプレート統合を実装しました。 変更内容ブックマーク編集・削除機能の実装
🎯 3 (Moderate) | ⏱️ ~20 分関連する可能性のある PR
🐰 詩
🚥 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)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/resources/templates/index.html (1)
38-41:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winOGP画像の
altを空文字ではなく意味のある文言にしてください。この画像は内容理解に寄与するため、
alt=""だとスクリーンリーダー利用者に情報が渡りません。ブックマークタイトルを使った代替テキストを推奨します。差分例
- <img class="bookmark-image" + <img class="bookmark-image" th:if="${bookmark.ogpImageUrl() != null}" th:src="${bookmark.ogpImageUrl()}" - alt=""> + th:alt="${bookmark.title()} + ' のサムネイル'">As per coding guidelines, 「アクセシビリティ: WCAG 2.1 AA に沿って、img の alt 属性...を確認する。」
🤖 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 `@src/main/resources/templates/index.html` around lines 38 - 41, The img tag uses an empty alt attribute; change it to provide meaningful alt text using the bookmark title (e.g., replace alt="" with a Thymeleaf attribute like th:alt="${bookmark.title}" and optionally add a fallback such as th:alt="${bookmark.title} ?: 'Bookmark image'") so screen readers receive the bookmark's title when bookmark.ogpImageUrl() is present; keep the existing th:if and th:src logic.
🧹 Nitpick comments (2)
src/main/resources/static/styles.css (1)
204-206: ⚡ Quick winアイコン操作に
:focus-visibleを追加してください。ホバー時の見た目はありますが、キーボード操作時の視認性を明示する
:focus-visibleがないため、操作フォーカスが分かりにくくなります。差分例
.edit-panel summary:hover, .icon-button:hover { background: var(--surface-muted); } + +.edit-panel summary:focus-visible, +.icon-button:focus-visible { + outline: 2px solid var(--accent); + outline-offset: 2px; +}Also applies to: 232-244
🤖 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 `@src/main/resources/static/styles.css` around lines 204 - 206, The hover-only focus styles reduce keyboard accessibility: update the CSS rule that targets ".edit-panel summary:hover, .icon-button:hover" to also include the corresponding ":focus-visible" selectors (e.g. ".edit-panel summary:focus-visible, .icon-button:focus-visible") so keyboard focus receives the same visual treatment; repeat the same change for the other similar hover-only rule later in the file that styles icon/button hover states so all interactive controls support :focus-visible.src/main/resources/templates/index.html (1)
47-65: ⚡ Quick win操作コンテナに適切なセマンティクスを付与してください。
aria-labelを付けたdivは支援技術で意図どおり解釈されにくいです。role="group"を併用して「編集/削除の操作グループ」であることを明示してください。差分例
- <div class="bookmark-actions" aria-label="ブックマーク操作"> + <div class="bookmark-actions" role="group" aria-label="ブックマーク操作">As per coding guidelines, 「アクセシビリティ: WCAG 2.1 AA に沿って、...aria 属性、キーボード操作可能性を確認する。」
🤖 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 `@src/main/resources/templates/index.html` around lines 47 - 65, The container div with class "bookmark-actions" currently only uses aria-label which may not convey grouping semantics to assistive tech; update the element (the div with class="bookmark-actions") to include role="group" (and keep the existing aria-label) so the edit-panel <details> and delete-form are announced as a single operation group, ensuring keyboard/ARIA semantics are explicit for this "編集/削除の操作グループ".
🤖 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 `@src/main/java/com/example/bookmark/BookmarkController.java`:
- Around line 56-60: Add server-side validation in the controller methods (at
least in edit and the other endpoints noted) so you validate input before
calling the repository: check `@PathVariable` id is > 0, enforce max-length limits
for description and tags (define constants like DESCRIPTION_MAX and TAGS_MAX
that match the client-side maxlength), and if validation fails throw a
ResponseStatusException(HttpStatus.BAD_REQUEST, "<field> invalid") or return an
appropriate 400 response; ensure these checks occur at the start of edit(...)
and the other methods referenced (lines around 71-73 and 81-86) and that
repository methods are only called when validation passes.
In `@src/main/java/com/example/bookmark/BookmarkRepository.java`:
- Around line 40-53: The updateDetails and delete methods currently discard
jdbcTemplate.update(...) return values so callers can't detect "0 rows updated";
change both methods (updateDetails and delete in BookmarkRepository) to return
int and return the result of jdbcTemplate.update(...), e.g. int rows =
jdbcTemplate.update(...); return rows; then update controller call sites to
check for rows == 0 and handle as an error/redirect accordingly so missing-id
updates/deletes are surfaced to the user.
---
Outside diff comments:
In `@src/main/resources/templates/index.html`:
- Around line 38-41: The img tag uses an empty alt attribute; change it to
provide meaningful alt text using the bookmark title (e.g., replace alt="" with
a Thymeleaf attribute like th:alt="${bookmark.title}" and optionally add a
fallback such as th:alt="${bookmark.title} ?: 'Bookmark image'") so screen
readers receive the bookmark's title when bookmark.ogpImageUrl() is present;
keep the existing th:if and th:src logic.
---
Nitpick comments:
In `@src/main/resources/static/styles.css`:
- Around line 204-206: The hover-only focus styles reduce keyboard
accessibility: update the CSS rule that targets ".edit-panel summary:hover,
.icon-button:hover" to also include the corresponding ":focus-visible" selectors
(e.g. ".edit-panel summary:focus-visible, .icon-button:focus-visible") so
keyboard focus receives the same visual treatment; repeat the same change for
the other similar hover-only rule later in the file that styles icon/button
hover states so all interactive controls support :focus-visible.
In `@src/main/resources/templates/index.html`:
- Around line 47-65: The container div with class "bookmark-actions" currently
only uses aria-label which may not convey grouping semantics to assistive tech;
update the element (the div with class="bookmark-actions") to include
role="group" (and keep the existing aria-label) so the edit-panel <details> and
delete-form are announced as a single operation group, ensuring keyboard/ARIA
semantics are explicit for this "編集/削除の操作グループ".
🪄 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: d5b6a17a-85c1-4766-b4cb-9a5058a67cdf
📒 Files selected for processing (4)
src/main/java/com/example/bookmark/BookmarkController.javasrc/main/java/com/example/bookmark/BookmarkRepository.javasrc/main/resources/static/styles.csssrc/main/resources/templates/index.html
Comment on lines
+56
to
+60
| @PostMapping("/bookmarks/{id}/edit") | ||
| public String edit( | ||
| @PathVariable long id, | ||
| @RequestParam(required = false) String description, | ||
| @RequestParam(required = false) String tags, |
There was a problem hiding this comment.
編集・削除エンドポイントにサーバー側バリデーションを追加してください。
id の正値チェックと、description/tags の長さチェックが未実装です。クライアント側 maxlength だけでは回避可能なので、Controller で必ず検証してから Repository を呼んでください。
差分例
+ private static final int MAX_TAGS_LENGTH = 500;
+ private static final int MAX_DESCRIPTION_LENGTH = 2000;
+
`@PostMapping`("/bookmarks/{id}/edit")
public String edit(
`@PathVariable` long id,
`@RequestParam`(required = false) String description,
`@RequestParam`(required = false) String tags,
RedirectAttributes redirectAttributes
) {
+ if (id <= 0) {
+ redirectAttributes.addFlashAttribute("error", "不正なIDです。");
+ return "redirect:/";
+ }
+ String normalizedDescription = blankToNull(description);
+ String normalizedTags = blankToNull(tags);
+ if (normalizedDescription != null && normalizedDescription.length() > MAX_DESCRIPTION_LENGTH) {
+ redirectAttributes.addFlashAttribute("error", "メモが長すぎます。");
+ return "redirect:/";
+ }
+ if (normalizedTags != null && normalizedTags.length() > MAX_TAGS_LENGTH) {
+ redirectAttributes.addFlashAttribute("error", "タグが長すぎます。");
+ return "redirect:/";
+ }
try {
- bookmarkRepository.updateDetails(id, blankToNull(description), blankToNull(tags));
+ bookmarkRepository.updateDetails(id, normalizedDescription, normalizedTags);
} catch (Exception ex) {
redirectAttributes.addFlashAttribute("error", "ブックマークを更新できませんでした。");
}
return "redirect:/";
}
`@PostMapping`("/bookmarks/{id}/delete")
public String delete(`@PathVariable` long id, RedirectAttributes redirectAttributes) {
+ if (id <= 0) {
+ redirectAttributes.addFlashAttribute("error", "不正なIDです。");
+ return "redirect:/";
+ }
try {
bookmarkRepository.delete(id);As per coding guidelines, 「入力検証: @RequestParam 等の外部入力に対するバリデーション（必須・形式・長さ）が網羅されているか確認する。」
Also applies to: 71-73, 81-86
🤖 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 `@src/main/java/com/example/bookmark/BookmarkController.java` around lines 56 -
60, Add server-side validation in the controller methods (at least in edit and
the other endpoints noted) so you validate input before calling the repository:
check `@PathVariable` id is > 0, enforce max-length limits for description and
tags (define constants like DESCRIPTION_MAX and TAGS_MAX that match the
client-side maxlength), and if validation fails throw a
ResponseStatusException(HttpStatus.BAD_REQUEST, "<field> invalid") or return an
appropriate 400 response; ensure these checks occur at the start of edit(...)
and the other methods referenced (lines around 71-73 and 81-86) and that
repository methods are only called when validation passes.
Comment on lines
+40
to
+53
| public void updateDetails(long id, String description, String tags) { | ||
| jdbcTemplate.update(""" | ||
| UPDATE bookmarks | ||
| SET description = ?, tags = ?, updated_at = datetime('now') | ||
| WHERE id = ? | ||
| """, description, tags, id); | ||
| } | ||
|
|
||
| public void delete(long id) { | ||
| jdbcTemplate.update(""" | ||
| DELETE FROM bookmarks | ||
| WHERE id = ? | ||
| """, id); | ||
| } |
There was a problem hiding this comment.
更新・削除の「0件更新」を検知できる形にしてください。
jdbcTemplate.update(...) の戻り値（更新件数）を捨てているため、存在しない id でも成功扱いになります。int を返して Controller 側で 0 件時にエラー表示へ分岐できるようにすると、失敗時の挙動が一貫します。
差分例
- public void updateDetails(long id, String description, String tags) {
- jdbcTemplate.update("""
+ public int updateDetails(long id, String description, String tags) {
+ return jdbcTemplate.update("""
UPDATE bookmarks
SET description = ?, tags = ?, updated_at = datetime('now')
WHERE id = ?
""", description, tags, id);
}
- public void delete(long id) {
- jdbcTemplate.update("""
+ public int delete(long id) {
+ return jdbcTemplate.update("""
DELETE FROM bookmarks
WHERE id = ?
""", id);
}As per coding guidelines, 「エラーハンドリング: 例外処理・エラー応答・リダイレクトの方針が一貫しているか確認する。」
🤖 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 `@src/main/java/com/example/bookmark/BookmarkRepository.java` around lines 40 -
53, The updateDetails and delete methods currently discard
jdbcTemplate.update(...) return values so callers can't detect "0 rows updated";
change both methods (updateDetails and delete in BookmarkRepository) to return
int and return the result of jdbcTemplate.update(...), e.g. int rows =
jdbcTemplate.update(...); return rows; then update controller call sites to
check for rows == 0 and handle as an error/redirect accordingly so missing-id
updates/deletes are surfaced to the user.
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
Tests
Summary by CodeRabbit
リリースノート
New Features
Style