feat: #337 viewsをClass Based Viewに変換

[izumin2000]

概要

• subekashi/views/ および article/views/ の全ビューをClass Based View（CBV）に変換
• dataD / detaD を context に統一

変換方針

パターン	対象	理由
TemplateView	ad_complete.py, anniversary6.py	固定コンテキストのGETのみ
dispatch() + get_base_context()	song_edit.py, song_delete.py, contact.py	オブジェクト取得・404/lockチェックをGET/POSTで共通化
get_base_context()	song_new.py	metatitleの重複排除
_handle()	songs.py	GET/POSTが同一ロジック
View (getのみ)	その他	重複なし、シンプルに実装

変換しないファイル

• ad.py — 対象外
• errors.py — Django error handlerはFBV必須
• file.py — 単純なリダイレクト、CBV化の恩恵なし

Test plan

• トップページ表示
• 曲一覧・検索（GET/POST）
• 曲詳細・編集・削除・新規登録
• 記事一覧・記事詳細
• 各種404/lockリダイレクト動作

🤖 Generated with Claude Code

[claude]

Code Review

This is a clean, well-structured refactoring PR. The FBV-to-CBV conversion is consistent, the context rename improves readability, and the appropriate CBV variants are chosen well (TemplateView for static views, dispatch() for shared pre-processing).

---

Code Quality & Best Practices

Positives:

• TemplateView is correctly used for anniversary6.py and ad_complete.py — ideal for static-context views
• dispatch() in SongEditView, SongDeleteView, and ContactView correctly centralizes object fetching and 404/lock checks shared between GET and POST
• @method_decorator(never_cache, name='dispatch') in top.py is the correct Django idiom for applying function-based decorators to CBVs
• except Article.DoesNotExist in default_article.py replaces a bare except: — good cleanup
• The context rename from dataD/detaD significantly improves readability

Nitpick — songs.py _handle() method:
Both get() and post() delegate to _handle(request). This works but slightly defeats the purpose of CBVs. If GET and POST are truly identical, a comment explaining why would help future readers.

Nitpick — UPPER_CASE local variables in songs.py:
REQUEST_DATA, COOKIES etc. are local variables using constant-style naming (PEP 8 reserves this for module-level constants). Pre-existing issue, but worth noting.

---

Potential Bugs

[Carried over] UnboundLocalError risk in SongView.get():
br_cleaned_lyrics is only assigned inside if/elif branches for "normal", "pack", "brless". If the cookie has any other value (e.g., tampered), referencing context["br_cleaned_lyrics"] will raise UnboundLocalError. Recommend adding a fallback else branch.

[Carried over] value != None in songs.py:
Should use value is not None per PEP 8.

---

Performance Considerations

• No new N+1 query risks introduced — logic is faithfully preserved
• ContactView.dispatch() always fetches Contact.get_answered() regardless of method, but Django QuerySets are lazy so the DB hit only occurs on template render — acceptable
• [Carried over] aiIns.order_by('?')[:25] in ai_result.py triggers a full table scan + sort on every request. This is a pre-existing concern worth addressing separately

---

Security Concerns

[Carried over] Potential XSS in top.py:
news_htmls is built by concatenating article.title directly into an HTML f-string. If Django's auto-escaping is disabled for this variable in the template, this is an XSS risk. Please verify the template uses auto-escaping (not |safe) for this value.

[Carried over] Reflected XSS risk in song_edit.py:
context["error"] = f"URL：{cleaned_url_item}は信頼されていないURLと判断されました..." — if cleaned_url_item is rendered without escaping in the template, this is a reflected XSS vector. Please verify template escaping.

[Carried over] httponly not set on preference cookies in SaveSettingsView:
Low severity since these are preference cookies (not session/auth), but worth addressing.

No regressions introduced: CSRF handling, the ALLOWED_SETTING_KEYS/ALLOWED_SETTING_VALUES allowlist, and HTTP method restrictions are all preserved correctly.

---

Test Coverage

No automated tests accompany this large refactor (675 additions, 620 deletions across ~20 files). The behavior-preserving nature of the conversion reduces risk, but automated regression tests would significantly increase confidence. At minimum, please confirm the existing test suite passes before merging. The manual test plan in the PR description is a good start.

---

Summary

The conversion is well-executed and idiomatic. The pre-existing bugs and security concerns noted above are out of scope for this PR, but this would be a good time to address them since the code is already being touched. The main actionable item before merging is verifying the XSS concerns in the templates and running the test suite.