-
Notifications
You must be signed in to change notification settings - Fork 223
Staging #712
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: do
Are you sure you want to change the base?
Staging #712
Changes from all commits
fc8e087
4faac26
d727464
8f82a55
d95102f
3f940ac
524eda2
3b97d98
f421160
f15d8ce
1cfc5ef
a9fd514
39a2d15
de4269c
01ea25b
ad18cf7
c58b9fa
6515c4d
10114a7
acea928
4fd514a
f4c955a
89dfce3
fd338f6
31d17d1
d38d00e
febb520
6c5b69c
ae205fc
ba2e3d4
870d7b7
efed618
3057492
3d05d42
138c567
6593e59
8dd5c6c
0cb1f18
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,3 +36,5 @@ public/sitemaps | |
/config/credentials/production.key | ||
|
||
/config/credentials/development.key | ||
|
||
.env | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
# frozen_string_literal: true | ||
|
||
module Api::Qdc | ||
class ChapterMetadataController < ApiController | ||
before_action :init_presenter | ||
|
||
def metadata | ||
render | ||
end | ||
|
||
protected | ||
|
||
def init_presenter | ||
@presenter = Qdc::ChapterMetadataPresenter.new(params) | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,5 +2,18 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
module Api::Qdc | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class QuranController < Api::V4::QuranController | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def tafsir | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@presenter = Qdc::TafsirsPresenter.new(params) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
filters = resource_filters(@resource) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@filter_names = humanize_filter_names(filters) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@tafsirs = if (@resource = fetch_tafsir_resource) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Tafsir.order('verse_id ASC').where(filters) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
render | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+5
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: filters computed before @resource is set, dropping resource_content_id resource_filters(@resource) runs with nil, so queries miss resource scoping. Move filter construction after assigning @resource (and build @filter_names from the corrected filters): def tafsir
- @presenter = Qdc::TafsirsPresenter.new(params)
- filters = resource_filters(@resource)
- @filter_names = humanize_filter_names(filters)
-
- @tafsirs = if (@resource = fetch_tafsir_resource)
- Tafsir.order('verse_id ASC').where(filters)
- else
- []
- end
+ @presenter = Qdc::TafsirsPresenter.new(params)
+ if (@resource = fetch_tafsir_resource)
+ filters = resource_filters(@resource)
+ @filter_names = humanize_filter_names(filters)
+ @tafsirs = Tafsir.where(filters).order(verse_id: :asc)
+ else
+ @filter_names = {}
+ @tafsirs = []
+ end
render
end 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
end |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -116,6 +116,113 @@ def languages | |||||||||||||||||||||||||||||||||
render | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
def country_language_preference | ||||||||||||||||||||||||||||||||||
user_device_language = request.query_parameters[:user_device_language].presence | ||||||||||||||||||||||||||||||||||
country = request.query_parameters[:country].presence&.upcase | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
# Require a valid user_device_language always | ||||||||||||||||||||||||||||||||||
if user_device_language.blank? | ||||||||||||||||||||||||||||||||||
return render_bad_request('user_device_language is required') | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
Comment on lines
+119
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Normalize language param; ensure case-insensitive matching Downcase the ISO code before validation to avoid false negatives. - user_device_language = request.query_parameters[:user_device_language].presence
+ user_device_language = request.query_parameters[:user_device_language].to_s.downcase.presence 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||
unless Language.exists?(iso_code: user_device_language) | ||||||||||||||||||||||||||||||||||
return render_bad_request('Invalid user_device_language') | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
# Validate country only if provided | ||||||||||||||||||||||||||||||||||
if country.present? | ||||||||||||||||||||||||||||||||||
valid_countries = ISO3166::Country.all.map(&:alpha2) | ||||||||||||||||||||||||||||||||||
unless valid_countries.include?(country) | ||||||||||||||||||||||||||||||||||
return render_bad_request('Invalid country code') | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
Comment on lines
+133
to
+137
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Optimize country validation Avoid allocating all countries on each request; use a direct lookup. - valid_countries = ISO3166::Country.all.map(&:alpha2)
- unless valid_countries.include?(country)
+ unless ISO3166::Country.find_country_by_alpha2(country)
return render_bad_request('Invalid country code')
end 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
if country.present? | ||||||||||||||||||||||||||||||||||
# First try to find country-specific preference | ||||||||||||||||||||||||||||||||||
preferences = CountryLanguagePreference.with_includes | ||||||||||||||||||||||||||||||||||
.where(user_device_language: user_device_language, country: country) | ||||||||||||||||||||||||||||||||||
@preference = preferences.first | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
# If no country-specific preference found, try global preference | ||||||||||||||||||||||||||||||||||
unless @preference | ||||||||||||||||||||||||||||||||||
@preference = CountryLanguagePreference.with_includes | ||||||||||||||||||||||||||||||||||
.find_by(user_device_language: user_device_language, country: nil) | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||||
# No country provided: search by user_device_language only | ||||||||||||||||||||||||||||||||||
# Prefer global (country: nil), then fall back to any match for that language | ||||||||||||||||||||||||||||||||||
@preference = CountryLanguagePreference.with_includes | ||||||||||||||||||||||||||||||||||
.find_by(user_device_language: user_device_language, country: nil) | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
unless @preference | ||||||||||||||||||||||||||||||||||
@preference = CountryLanguagePreference.with_includes | ||||||||||||||||||||||||||||||||||
.where(user_device_language: user_device_language) | ||||||||||||||||||||||||||||||||||
.first | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
if @preference | ||||||||||||||||||||||||||||||||||
# Filter out unapproved resources when building the response | ||||||||||||||||||||||||||||||||||
@data = build_preference_data(@preference) | ||||||||||||||||||||||||||||||||||
render | ||||||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||||
render_404("No matching country language preference found") | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
Comment on lines
+119
to
+171
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainEnd-to-end verification suggestion After applying the gating changes, verify no QDC/V4 endpoints return rejected resources. 🏁 Script executed: #!/bin/bash
# Find ResourceContent queries missing allowed_to_share in controllers/presenters
rg -nP --type=ruby -C2 "(ResourceContent|Recitation).*approved" app/controllers app/presenters \
| sed -n '1,200p'
echo "---- Potential misses (heuristic):"
rg -nP --type=ruby -C2 "(ResourceContent|Recitation).*\\.(eager_load|includes|joins)\\(.*\\).*\\.(translations|tafsirs|chapter_info|media|one_verse|one_chapter)" app/controllers app/presenters \
| rg -v "allowed_to_share" -n -C2 Length of output: 5273 Add
Re-run the end-to-end check to confirm no QDC/V4 endpoints surface rejected resources. 🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
private | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
def build_preference_data(preference) | ||||||||||||||||||||||||||||||||||
# Sanitize CSV IDs for default translations | ||||||||||||||||||||||||||||||||||
ids = if preference.default_translation_ids.present? | ||||||||||||||||||||||||||||||||||
preference.default_translation_ids | ||||||||||||||||||||||||||||||||||
.split(',') | ||||||||||||||||||||||||||||||||||
.map(&:strip) | ||||||||||||||||||||||||||||||||||
.reject(&:blank?) | ||||||||||||||||||||||||||||||||||
.map(&:to_i) | ||||||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||||
[] | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
# QR specific default translations ids | ||||||||||||||||||||||||||||||||||
qr_ids = if preference.qr_default_translations_ids.present? | ||||||||||||||||||||||||||||||||||
preference.qr_default_translations_ids | ||||||||||||||||||||||||||||||||||
.split(',') | ||||||||||||||||||||||||||||||||||
.map(&:strip) | ||||||||||||||||||||||||||||||||||
.reject(&:blank?) | ||||||||||||||||||||||||||||||||||
.map(&:to_i) | ||||||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||||
[] | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
# QR default arabic fonts | ||||||||||||||||||||||||||||||||||
qr_font_ids = if preference.qr_default_arabic_fonts.present? | ||||||||||||||||||||||||||||||||||
preference.qr_default_arabic_fonts | ||||||||||||||||||||||||||||||||||
.split(',') | ||||||||||||||||||||||||||||||||||
.map(&:strip) | ||||||||||||||||||||||||||||||||||
.reject(&:blank?) | ||||||||||||||||||||||||||||||||||
.map(&:to_i) | ||||||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||||
[] | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
preference: preference, | ||||||||||||||||||||||||||||||||||
default_mushaf: preference.mushaf&.enabled ? preference.mushaf : nil, | ||||||||||||||||||||||||||||||||||
default_translations: ids.any? ? | ||||||||||||||||||||||||||||||||||
ResourceContent.where(id: ids).approved.includes(:translated_name) : [], | ||||||||||||||||||||||||||||||||||
qr_default_translations: qr_ids.any? ? | ||||||||||||||||||||||||||||||||||
ResourceContent.where(id: qr_ids).approved.includes(:translated_name) : [], | ||||||||||||||||||||||||||||||||||
default_tafsir: preference.tafsir&.approved? ? preference.tafsir : nil, | ||||||||||||||||||||||||||||||||||
default_wbw_language: preference.wbw_language, | ||||||||||||||||||||||||||||||||||
default_reciter: preference.reciter, | ||||||||||||||||||||||||||||||||||
ayah_reflections_languages: Language.where(iso_code: preference.ayah_reflections_languages&.split(',') || []), | ||||||||||||||||||||||||||||||||||
qr_reflection_languages: Language.where(iso_code: preference.qr_reflection_languages&.split(',') || []), | ||||||||||||||||||||||||||||||||||
learning_plan_languages: Language.where(iso_code: preference.learning_plan_languages&.split(',') || []), | ||||||||||||||||||||||||||||||||||
qr_default_arabic_fonts: qr_font_ids | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
protected | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
def load_translations | ||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ class TranslationsController < Api::V4::TranslationsController | |
|
||
protected | ||
def init_presenter | ||
@presenter = TranslationsPresenter.new(params) | ||
@presenter = Qdc::TranslationsPresenter.new(params) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Confirm presenter-side param mutation is intentional. Qdc::TranslationsPresenter may mutate params (e.g., resource_id). Verify downstream usage in this controller relies on the mutated params; otherwise consider passing a copy to avoid side effects. 🤖 Prompt for AI Agents
|
||
end | ||
|
||
def load_translations | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -13,6 +13,7 @@ def translations | |||||||||||||
.eager_load(:translated_name) | ||||||||||||||
.one_verse | ||||||||||||||
.translations | ||||||||||||||
.allowed_to_share | ||||||||||||||
.approved | ||||||||||||||
.order('priority ASC') | ||||||||||||||
|
||||||||||||||
|
@@ -31,7 +32,7 @@ def translation_info | |||||||||||||
end | ||||||||||||||
|
||||||||||||||
def word_by_word_translations | ||||||||||||||
list = ResourceContent.eager_load(:translated_name).approved.one_word.translations_only.order('priority ASC') | ||||||||||||||
list = ResourceContent.eager_load(:translated_name).approved.allowed_to_share.one_word.translations_only.order('priority ASC') | ||||||||||||||
|
||||||||||||||
@word_by_word_translations = eager_load_translated_name(list) | ||||||||||||||
|
||||||||||||||
|
@@ -43,6 +44,7 @@ def tafsirs | |||||||||||||
.eager_load(:translated_name) | ||||||||||||||
.tafsirs | ||||||||||||||
.approved | ||||||||||||||
.allowed_to_share | ||||||||||||||
.order('priority ASC') | ||||||||||||||
|
||||||||||||||
@presenter = ResourcePresenter.new(params) | ||||||||||||||
|
@@ -62,6 +64,8 @@ def tafsir_info | |||||||||||||
def recitations | ||||||||||||||
list = Recitation | ||||||||||||||
.eager_load(reciter: :translated_name) | ||||||||||||||
.joins(:resource_content) | ||||||||||||||
.where.not(resource_contents: { permission_to_share: :rejected }) | ||||||||||||||
.approved | ||||||||||||||
Comment on lines
+67
to
69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Prefer merging the ResourceContent scope over duplicating the WHERE Avoid re-encoding scope logic; use merge(ResourceContent.allowed_to_share). - .joins(:resource_content)
- .where.not(resource_contents: { permission_to_share: :rejected })
+ .joins(:resource_content)
+ .merge(ResourceContent.allowed_to_share) 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||
.order('translated_names.language_priority desc') | ||||||||||||||
|
||||||||||||||
|
@@ -93,6 +97,7 @@ def chapter_infos | |||||||||||||
.chapter_info | ||||||||||||||
.one_chapter | ||||||||||||||
.approved | ||||||||||||||
.allowed_to_share | ||||||||||||||
|
||||||||||||||
@chapter_infos = eager_load_translated_name(list) | ||||||||||||||
|
||||||||||||||
|
@@ -103,8 +108,9 @@ def verse_media | |||||||||||||
@media = ResourceContent | ||||||||||||||
.includes(:language) | ||||||||||||||
.media | ||||||||||||||
.one_verse.approved | ||||||||||||||
|
||||||||||||||
.one_verse | ||||||||||||||
.approved | ||||||||||||||
.allowed_to_share | ||||||||||||||
render | ||||||||||||||
end | ||||||||||||||
|
||||||||||||||
|
@@ -121,6 +127,7 @@ def changes | |||||||||||||
.change_log(after: time) | ||||||||||||||
.filter_subtype(params[:type]) | ||||||||||||||
.approved | ||||||||||||||
.allowed_to_share | ||||||||||||||
|
||||||||||||||
render | ||||||||||||||
else | ||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -39,12 +39,14 @@ def fetch_locale | |||||||||||||||||||||||||
end | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
def set_cache_headers | ||||||||||||||||||||||||||
if action_name != 'random' | ||||||||||||||||||||||||||
expires_in 7.day, public: true | ||||||||||||||||||||||||||
# https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security | ||||||||||||||||||||||||||
headers['Strict-Transport-Security'] = 'max-age=31536000; includeSubDomains; preload' | ||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||
headers['Cache-Control'] = 'no-store, no-cache, max-age=0, private, must-revalidate' | ||||||||||||||||||||||||||
headers['Pragma'] = 'no-cache' | ||||||||||||||||||||||||||
headers['Expires'] = '0' | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
Comment on lines
+42
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Don’t blanket “no-store”; this kills CDN/browser caching for all endpoints. Prefer targeted no-cache or conditional policy to avoid a perf regression on heavy GETs. Apply scoped caching and keep no-store only when required: - headers['Cache-Control'] = 'no-store, no-cache, max-age=0, private, must-revalidate'
- headers['Pragma'] = 'no-cache'
- headers['Expires'] = '0'
+ if request.method != 'GET' || params[:no_cache].present?
+ headers['Cache-Control'] = 'no-store, no-cache, max-age=0, private, must-revalidate'
+ headers['Pragma'] = 'no-cache'
+ headers['Expires'] = '0'
+ else
+ # cacheable GETs; tune s-maxage per endpoint if needed
+ headers['Cache-Control'] = 'public, max-age=0, s-maxage=300, stale-while-revalidate=300'
+ headers['Vary'] = ['Origin', headers['Vary']].compact.join(', ')
+ end 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
# Keep HSTS header | ||||||||||||||||||||||||||
headers['Strict-Transport-Security'] = 'max-age=31536000; includeSubDomains; preload' | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainGate HSTS to HTTPS production only. Avoid setting HSTS on HTTP, dev, or staging domains not ready for preload. - headers['Strict-Transport-Security'] = 'max-age=31536000; includeSubDomains; preload'
+ if Rails.env.production? && request.ssl?
+ headers['Strict-Transport-Security'] = 'max-age=31536000; includeSubDomains; preload'
+ end Verify your edge/proxy isn’t already injecting HSTS to prevent duplicate/conflicting headers. 🏁 Script executed: #!/bin/bash
# Find HSTS header assignment in ApplicationController
rg -n "Strict-Transport-Security" -n app/controllers/application_controller.rb
# Show context around the HSTS header assignment
sed -n '30,80p' app/controllers/application_controller.rb
# Check if force_ssl is enabled in production
grep -R "force_ssl" -n config/environments/production.rb Length of output: 1663 🏁 Script executed: #!/bin/bash
# Search for HSTS or Strict-Transport-Security in config directories
rg -n "hsts" -n config || true
rg -n "Strict-Transport-Security" -n config || true
# Check for secure_headers gem usage
rg -n "secure_headers" -n Gemfile* || true
rg -n "secure_headers" -n Gemfile.lock || true
# Look for HSTS configuration in initializers
rg -n "hsts" -n config/initializers || true
rg -n "secure_headers" -n config/initializers || true Length of output: 627 Gate HSTS header to production over HTTPS only - headers['Strict-Transport-Security'] = 'max-age=31536000; includeSubDomains; preload'
+ if Rails.env.production? && request.ssl?
+ headers['Strict-Transport-Security'] = 'max-age=31536000; includeSubDomains; preload'
+ end Ensure your edge or CDN isn’t already injecting HSTS to avoid duplicate/conflicting headers. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
# Keep CORS header | ||||||||||||||||||||||||||
headers['Access-Control-Allow-Origin'] = '*' | ||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||
Comment on lines
50
to
51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainCORS: add Vary and basic preflight headers (or delegate to Rack::Cors). With "*" you should set Vary: Origin to protect shared caches; consider methods/headers too. headers['Access-Control-Allow-Origin'] = '*'
+ headers['Access-Control-Allow-Methods'] = 'GET, OPTIONS'
+ headers['Access-Control-Allow-Headers'] = 'Origin, Content-Type, Accept, Authorization'
+ headers['Vary'] = ['Origin', headers['Vary']].compact.join(', ') If you need credentials, replace "*" with the request Origin echo and set Allow-Credentials: true. 🏁 Script executed: #!/bin/bash
echo "Searching for Rack::Cors usage..."
rg -n "rack-cors" || true
echo "Searching for gem 'rack-cors' in Gemfile..."
grep -R "rack-cors" -n Gemfile* || true
echo "Searching for CORS configuration in initializers..."
grep -R "cors" -n config/initializers || true
echo "Searching routes for OPTIONS preflight handling..."
grep -R "match.*options" -n config/routes.rb || true
echo "Inspecting application_controller.rb around CORS headers..."
rg -n -C3 "Access-Control-Allow-Origin" app/controllers/application_controller.rb || true Length of output: 1343 🏁 Script executed: #!/bin/bash
sed -n '1,200p' config/initializers/cros.rb Length of output: 528 Remove manual CORS headers in ApplicationController 🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
# frozen_string_literal: true | ||
|
||
class HealthController < ApplicationController | ||
# Lightweight liveness endpoint: confirms app process & middleware stack respond. | ||
def show | ||
render json: { status: 'ok' } | ||
end | ||
Comment on lines
+3
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainSkip heavy filters (auth, locale, rate limiting) if any. Health should bypass non-essential before_actions to stay fast/reliable. Run to check for inherited filters and add skips if present: If needed: class HealthController < ApplicationController
+ skip_before_action :authenticate_user!, :set_locale, :verify_authenticity_token, raise: false
def show
render json: { status: 'ok' }
end
end 🏁 Script executed: #!/bin/bash
rg -nC2 -g 'app/controllers/**/*.rb' -P 'class\s+ApplicationController\b|before_action|around_action' Length of output: 10266 Skip non-essential ApplicationController before_actions for the /health endpoint ApplicationController registers before_action :set_cache_headers and :set_default_response_format — add skip_before_action :set_cache_headers, :set_default_response_format, raise: false in app/controllers/health_controller.rb so /health remains minimal (see app/controllers/application_controller.rb). 🤖 Prompt for AI Agents
|
||
end |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3,7 +3,7 @@ | |||||||||||||||||
module Mobile | ||||||||||||||||||
class TranslationsController < ApplicationController | ||||||||||||||||||
def index | ||||||||||||||||||
resources = ResourceContent.includes(:language).translations.one_verse.approved | ||||||||||||||||||
resources = ResourceContent.includes(:language).translations.one_verse.approved.allowed_to_share | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enforce deterministic ordering to match other endpoints Add priority ordering so clients get stable results across requests (consistent with v3/v4 controllers). - resources = ResourceContent.includes(:language).translations.one_verse.approved.allowed_to_share
+ resources = ResourceContent
+ .includes(:language)
+ .translations
+ .one_verse
+ .approved
+ .allowed_to_share
+ .order('priority ASC') 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||
|
||||||||||||||||||
render json: resources, root: :data, each_serializer: Mobile::TranslationSerializer | ||||||||||||||||||
end | ||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Broaden secrets ignore patterns and keep a checked-in example.
Avoid leaking future credentials keys and env variants.
Apply:
📝 Committable suggestion
🤖 Prompt for AI Agents