@tomhughes requested changes on this pull request.
In addition to the inline comments please review the comment for the first
commit as it currently includes detail from the squashed commit s that makes no
sense - probably only the first line is needed now.
> @@ -15,7 +15,11 @@ def active_banners
enddate = nil
end
- startdate&.future? || enddate&.past?
+ wrong_locale = v[:locales]&.exclude?(I18n.locale.to_s)
+ remote_country = params[:country] ||
OSM.ip_to_country(request.remote_ip) || Settings.default_legale
The `default_legale` fallback is specific to the terms page so shouldn't be
used here:
```suggestion
remote_country = params[:country] || OSM.ip_to_country(request.remote_ip)
```
> + def make_banner(id, locales: nil, countries: nil)
+ {
+ :id => id,
+ :alt => "Test Banner",
+ :link => "https://example.com";,
+ :img => "banners/test.png",
+ :enddate => "2099-jan-01",
+ :locales => locales,
+ :countries => countries
+ }.compact
+ end
This can be moved to the end with `stub_const` as it doesn't need to be public.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6851#pullrequestreview-3939505417
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/6851/review/[email protected]>_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
