Conversation
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/controllers/v1/daily_verses_controller.rb (2)
23-25: Add a test assertion for translation selection (nbv21).Current request coverage (spec/requests/v1/daily_verses_controller/index_spec.rb, Line 1-42) checks caching and status, but not which translation key is serialized. A focused assertion would prevent silent regression back to another key.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/v1/daily_verses_controller.rb` around lines 23 - 25, Add an assertion in the existing request spec (spec/requests/v1/daily_verses_controller/index_spec.rb) to verify the controller serializes the expected translation key "nbv21": after performing the GET and parsing the JSON response, assert that the returned verse object includes the nbv21 fields (e.g., response_body['data'][0]['attributes']['content'] or the specific key you expect) and that the content matches the sanitized string you expect for nbv21; this ensures the controller code paths that set copyright, reference, and content using the nbv21 keys in DailyVersesController are exercised and prevents silent fallback to another translation.
23-25: Avoid duplicating the translation key literal.
'nbv21'appears in multiple places; extracting it to a constant will reduce drift if this key changes again.Refactor sketch
class V1::DailyVersesController < V1::ApplicationController + TRANSLATION_KEY = 'nbv21'.freeze + def parse_verse(response) data = response['data'][0] - copyright = ActionView::Base.full_sanitizer.sanitize(response['copyrights']['nbv21']) + copyright = ActionView::Base.full_sanitizer.sanitize(response['copyrights'][TRANSLATION_KEY]) reference = ActionView::Base.full_sanitizer.sanitize(data['source']) - content = ActionView::Base.full_sanitizer.sanitize(data['text']['nbv21']) + content = ActionView::Base.full_sanitizer.sanitize(data['text'][TRANSLATION_KEY]) { data: [{ id: 1, type: 'daily_verse', attributes: { copyright:, reference:, content: } }] } end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/v1/daily_verses_controller.rb` around lines 23 - 25, Extract the repeated translation key literal 'nbv21' into a single constant (e.g. NBV21_KEY or TRANSLATION_KEY) at the top of the DailyVersesController (or as a private constant inside the controller) and replace the three literal uses in the assignments to copyright, reference, and content with that constant; ensure the constant name is descriptive and update any other occurrences in this controller to reference it so future key changes require a single edit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/controllers/v1/daily_verses_controller.rb`:
- Around line 23-25: Add an assertion in the existing request spec
(spec/requests/v1/daily_verses_controller/index_spec.rb) to verify the
controller serializes the expected translation key "nbv21": after performing the
GET and parsing the JSON response, assert that the returned verse object
includes the nbv21 fields (e.g.,
response_body['data'][0]['attributes']['content'] or the specific key you
expect) and that the content matches the sanitized string you expect for nbv21;
this ensures the controller code paths that set copyright, reference, and
content using the nbv21 keys in DailyVersesController are exercised and prevents
silent fallback to another translation.
- Around line 23-25: Extract the repeated translation key literal 'nbv21' into a
single constant (e.g. NBV21_KEY or TRANSLATION_KEY) at the top of the
DailyVersesController (or as a private constant inside the controller) and
replace the three literal uses in the assignments to copyright, reference, and
content with that constant; ensure the constant name is descriptive and update
any other occurrences in this controller to reference it so future key changes
require a single edit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d00875a6-1925-4f15-be5c-9ae0bc344608
📒 Files selected for processing (1)
app/controllers/v1/daily_verses_controller.rb
There was a problem hiding this comment.
Pull request overview
Updates the daily verse API response parsing to use the NBV21 translation fields instead of BGT.
Changes:
- Switches parsed copyright source from
response['copyrights']['bgt']toresponse['copyrights']['nbv21'] - Switches parsed verse content from
data['text']['bgt']todata['text']['nbv21']
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| copyright = ActionView::Base.full_sanitizer.sanitize(response['copyrights']['nbv21']) | ||
| reference = ActionView::Base.full_sanitizer.sanitize(data['source']) | ||
| content = ActionView::Base.full_sanitizer.sanitize(data['text']['bgt']) | ||
| content = ActionView::Base.full_sanitizer.sanitize(data['text']['nbv21']) |
There was a problem hiding this comment.
The request spec for this endpoint only checks status/caching and does not assert which translation is returned. Since this change switches the translation key from bgt to nbv21, please add an assertion in the existing request spec that verifies attributes.content and attributes.copyright are derived from the nbv21 fields (e.g., by comparing against the VCR cassette response). This will prevent silent regressions if the translation key changes again or the API response shape differs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## staging #625 +/- ##
========================================
Coverage 99.92% 99.92%
========================================
Files 197 197
Lines 2665 2665
========================================
Hits 2663 2663
Misses 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fix the bible translation
Summary by CodeRabbit