Admin setting to control enabled countries for billboard geotargeting (#20083)

* use instance setting for enabled target geolocations

* add validation for enabled geolocations setting

* a start on the UI?

* backend tweaks for UI

* proper crack at autocomplete component

* fix region targeting toggle

* e2e spec
This commit is contained in:
PJ 2023-09-12 17:06:00 +01:00 committed by GitHub
parent eb17b73690
commit 3f922921e5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
26 changed files with 622 additions and 33 deletions

View file

@ -28,17 +28,25 @@ module Admin
end
def settings_params
params.require(:settings_general)&.permit(
params.require(:settings_general)&.merge(parsed_countries)&.permit(
settings_keys.map(&:to_sym),
social_media_handles: ::Settings::General::SOCIAL_MEDIA_SERVICES,
meta_keywords: ::Settings::General.meta_keywords.keys,
credit_prices_in_cents: ::Settings::General.credit_prices_in_cents.keys,
billboard_enabled_countries: ISO3166::Country.codes,
)
end
def settings_keys
::Settings::General.keys + SPECIAL_PARAMS_TO_ADD
end
def parsed_countries
countries = params[:settings_general][:billboard_enabled_countries]
return {} unless countries
{ billboard_enabled_countries: JSON.parse(countries) }
end
end
end
end

View file

@ -0,0 +1,11 @@
module Admin
module SettingsHelper
def billboard_enabled_countries_for_editing
::Settings::General.billboard_enabled_countries.to_json
end
def billboard_all_countries_for_editing
ISO3166::Country.all.to_h { |country| [country.alpha2, country.common_name] }
end
end
end

View file

@ -0,0 +1,62 @@
import { h } from 'preact';
import { useCallback } from 'preact/hooks';
import PropTypes from 'prop-types';
import { MultiSelectAutocomplete } from '@crayons';
export { SelectedLocation } from './templates';
export const Locations = ({
defaultValue = [],
allLocations,
inputId,
onChange,
placeholder = 'Enter a country name...',
template,
}) => {
const autocompleteLocations = useCallback(
(query) => {
return new Promise((resolve) => {
queueMicrotask(() => {
const suggestions = [];
const caseInsensitiveQuery = query.toLowerCase();
Object.keys(allLocations).forEach((name) => {
if (name.toLowerCase().indexOf(caseInsensitiveQuery) > -1) {
suggestions.push(allLocations[name]);
}
});
resolve(suggestions);
});
});
},
[allLocations],
);
return (
<MultiSelectAutocomplete
defaultValue={defaultValue}
fetchSuggestions={autocompleteLocations}
border
labelText="Enabled countries for targeting"
placeholder={placeholder}
SelectionTemplate={template}
onSelectionsChanged={onChange}
inputId={inputId}
allowUserDefinedSelections={false}
/>
);
};
const locationsShape = PropTypes.shape({
name: PropTypes.string.isRequired,
code: PropTypes.string.isRequired,
withRegions: PropTypes.bool,
});
Locations.propTypes = {
defaultValue: PropTypes.arrayOf(locationsShape),
allLocations: PropTypes.objectOf(locationsShape).isRequired,
inputId: PropTypes.string.isRequired,
onChange: PropTypes.func.isRequired,
placeholder: PropTypes.string,
template: PropTypes.elementType,
};

View file

@ -0,0 +1,48 @@
import { h } from 'preact';
import { useCallback } from 'preact/hooks';
import { ButtonNew as Button, Icon } from '@crayons';
import { Close } from '@images/x.svg';
/**
* Higher-order component that returns a template responsible for the layout of
* a selected location
*
* @returns {h.JSX.ElementType}
*/
export const SelectedLocation = ({
displayName,
onNameClick,
label,
ExtraInfo,
}) => {
const Template = ({ onEdit: _, onDeselect, ...location }) => {
const onClick = useCallback(() => onNameClick(location), [location]);
return (
<div
role="group"
aria-label={location.name}
className="c-autocomplete--multi__tag-selection flex mr-2 mb-2 w-max"
>
<Button
aria-label={label}
onClick={onClick}
className="c-autocomplete--multi__selected p-1 flex flex-col"
>
{location.name}
{ExtraInfo && <ExtraInfo {...location} />}
</Button>
<Button
aria-label={`Remove ${location.name}`}
onClick={onDeselect}
className="c-autocomplete--multi__selected p-1"
>
<Icon src={Close} />
</Button>
</div>
);
};
Template.displayName = displayName;
return Template;
};

View file

@ -0,0 +1,91 @@
import { h, render } from 'preact';
import { Locations, SelectedLocation } from '../../billboard/locations';
const RegionMarker = ({ withRegions }) => {
return (
<span className="fs-xs fw-bold">
{withRegions ? 'Including' : 'Excluding'} regions
</span>
);
};
function parseDOMState(hiddenField) {
const countriesByCode = JSON.parse(hiddenField.dataset.allCountries);
const allCountries = {};
for (const [code, name] of Object.entries(countriesByCode)) {
allCountries[name] = { name, code };
}
const existingSetting = JSON.parse(hiddenField.value);
const selectedCountries = Object.keys(existingSetting).map((code) => ({
name: countriesByCode[code],
code,
withRegions: existingSetting[code] === 'with_regions',
}));
return { allCountries, selectedCountries };
}
function syncSelectionsToDOM(hiddenField, countries) {
const newValue = countries.reduce((value, { code, withRegions }) => {
value[code] = withRegions ? 'with_regions' : 'without_regions';
return value;
}, {});
hiddenField.value = JSON.stringify(newValue);
}
/**
* Sets up and renders a Preact component to handle searching for and enabling
* countries for targeting (and, per country, to enable region-level targeting).
*/
function setupEnabledCountriesEditor() {
const editor = document.getElementById('billboard-enabled-countries-editor');
const hiddenField = document.querySelector('.geolocation-multiselect');
if (!(editor && hiddenField)) return;
const { allCountries, selectedCountries } = parseDOMState(hiddenField);
let currentSelections = selectedCountries;
function setCountriesSelection(countries) {
currentSelections = countries;
syncSelectionsToDOM(hiddenField, currentSelections);
}
function updateRegionSetting(country) {
const selected = currentSelections.find(
(selectedCountry) => selectedCountry.code === country.code,
);
selected.withRegions = !selected.withRegions;
syncSelectionsToDOM(hiddenField, currentSelections);
renderLocations();
}
const EnabledCountry = SelectedLocation({
displayName: 'EnabledCountry',
onNameClick: updateRegionSetting,
label: 'Toggle region targeting',
ExtraInfo: RegionMarker,
});
function renderLocations() {
render(
<Locations
defaultValue={currentSelections}
onChange={setCountriesSelection}
inputId="billboard-enabled-countries-editor"
allLocations={allCountries}
template={EnabledCountry}
/>,
editor,
);
}
renderLocations();
}
if (document.readyState !== 'loading') {
setupEnabledCountriesEditor();
} else {
document.addEventListener('DOMContentLoaded', setupEnabledCountriesEditor);
}

View file

@ -8,6 +8,9 @@ module Constants
ahoy_tracking: {
description: I18n.t("lib.constants.settings.general.ahoy_tracking.description")
},
billboard_enabled_countries: {
description: I18n.t("lib.constants.settings.general.billboard_enabled_countries.description")
},
contact_email: {
description: I18n.t("lib.constants.settings.general.contact_email.description"),
placeholder: "hello@example.com"

View file

@ -160,7 +160,7 @@ class Billboard < ApplicationRecord
def validate_geolocations
target_geolocations.each do |geo|
unless geo.valid?
unless geo.valid?(:targeting)
errors.add(:target_geolocations, I18n.t("models.billboard.invalid_location", location: geo.to_iso3166))
end
end

View file

@ -30,11 +30,10 @@ class Geolocation
FEATURE_FLAG = :billboard_location_targeting
ISO3166_SEPARATOR = "-".freeze
LTREE_SEPARATOR = ".".freeze
# Maybe this should be a config of some kind...?
PERMITTED_COUNTRIES = [
ISO3166::Country.code_from_name("United States"),
ISO3166::Country.code_from_name("Canada"),
].freeze
DEFAULT_ENABLED_COUNTRIES = {
ISO3166::Country.code_from_name("United States") => :with_regions,
ISO3166::Country.code_from_name("Canada") => :with_regions
}.freeze
attr_reader :country_code, :region_code
@ -60,11 +59,13 @@ class Geolocation
@region_code = region_code
end
# validates :country_code, inclusion: { in: ISO3166::Country.codes }
validates :country_code, inclusion: { in: PERMITTED_COUNTRIES }
validates :country_code, inclusion: {
in: ->(_) { Settings::General.billboard_enabled_countries.keys }
}
validates :region_code, inclusion: {
in: ->(geolocation) { ISO3166::Country.region_codes_if_exists(geolocation.country_code) }
}, allow_nil: true
validate :valid_region_for_targeting, on: :targeting
def ==(other)
country_code == other.country_code && region_code == other.region_code
@ -88,6 +89,13 @@ class Geolocation
"'#{lquery}' ~ #{column_name}"
end
def valid_region_for_targeting
return if region_code.nil? ||
Settings::General.billboard_enabled_countries[country_code] == :with_regions
errors.add(:region_code, "was provided on a country with region targeting disabled")
end
def errors_as_sentence
errors.full_messages.to_sentence
end

View file

@ -72,6 +72,10 @@ module Settings
setting :payment_pointer, type: :string
setting :stripe_api_key, type: :string, default: ApplicationConfig["STRIPE_SECRET_KEY"]
setting :stripe_publishable_key, type: :string, default: ApplicationConfig["STRIPE_PUBLISHABLE_KEY"]
# Billboard-related. Not sure this is the best place for it, but it's a start.
setting :billboard_enabled_countries, type: :hash, default: Geolocation::DEFAULT_ENABLED_COUNTRIES, validates: {
enabled_countries_hash: true
}
# Newsletter
# <https://mailchimp.com/developer/>

View file

@ -26,6 +26,7 @@ module Settings
settings[param] = settings[param]&.downcase&.delete(" ") if settings[param]
end
settings[:credit_prices_in_cents]&.transform_values!(&:to_i)
settings[:billboard_enabled_countries]&.transform_values!(&:to_sym)
settings
end

View file

@ -0,0 +1,21 @@
class EnabledCountriesHashValidator < ActiveModel::EachValidator
VALID_HASH_VALUES = %i[with_regions without_regions].freeze
def validate_each(record, attribute, value)
if value.blank? || !value.is_a?(Hash)
record.errors.add(attribute,
options[:message] || I18n.t("validators.iso3166_hash_validator.is_blank"))
return
end
unless value.keys.all? { |key| ISO3166::Country.codes.include? key }
record.errors.add(attribute,
options[:message] || I18n.t("validators.iso3166_hash_validator.invalid_key"))
end
return if value.values.all? { |value| VALID_HASH_VALUES.include? value }
record.errors.add(attribute,
options[:message] || I18n.t("validators.iso3166_hash_validator.invalid_value"))
end
end

View file

@ -32,6 +32,23 @@
value: Settings::General.payment_pointer,
placeholder: Constants::Settings::General.details[:payment_pointer][:placeholder] %>
</div>
<% if FeatureFlag.enabled?(Geolocation::FEATURE_FLAG) %>
<div class="crayons-field">
<%= admin_config_label :billboard_enabled_countries %>
<%= admin_config_description Constants::Settings::General.details[:billboard_enabled_countries][:description] %>
<div id="billboard-enabled-countries-editor"></div>
</div>
<div class="crayons-field hidden">
<%= f.text_field :billboard_enabled_countries,
class: "crayons-textfield geolocation-multiselect",
value: billboard_enabled_countries_for_editing,
autocomplete: "off",
data: { all_countries: billboard_all_countries_for_editing } %>
</div>
<%= javascript_packs_with_chunks_tag "admin/billboardEnabledCountries", defer: true %>
<% end %>
</fieldset>
<%= render "update_setting_button", f: f %>
</div>

View file

@ -96,6 +96,8 @@ en:
general:
ahoy_tracking:
description: Track visits and events - data is stored in your database. A restart is required for changes to this configuration to take effect.
billboard_enabled_countries:
description: Countries that can be geotargeted by billboards. Click on a selected country to toggle targeting on a sub-country level.
contact_email:
description: Used for contact links. Please provide an email address where users can get in touch with you or your team.
credit:

View file

@ -96,6 +96,8 @@ fr:
general:
ahoy_tracking:
description: Track visits and events - data is stored in your database. A restart is required for changes to this configuration to take effect.
billboard_enabled_countries:
description: Countries that can be geotargeted by billboards. Click on a selected country to toggle targeting on a sub-country level.
contact_email:
description: Used for contact links. Please provide an email address where users can get in touch with you or your team.
credit:

View file

@ -42,7 +42,7 @@ en:
experience4: Have experience level four
experience5: Have experience level five
billboard:
invalid_location: '%{location} is not a supported ISO 3166-2 code'
invalid_location: '%{location} is not an enabled target ISO 3166-2 code'
broadcast:
single_active: You can only have one active announcement broadcast
comment:

View file

@ -41,7 +41,7 @@ fr:
experience4: Have experience level four
experience5: Have experience level five
billboard:
invalid_location: "%{location} n'est pas un code pris en charge de la norme ISO 3166-2"
invalid_location: "%{location} n'est pas un code cible activé de la norme ISO 3166-2"
broadcast:
single_active: Vous ne pouvez avoir qu'une seule diffusion d'annonce active
comment:

View file

@ -17,3 +17,7 @@ en:
is_reserved: is reserved
valid_domain_csv_validator:
invalid_list_format: must be a comma-separated list of valid domains
iso3166_hash_validator:
is_blank: must be provided
invalid_key: contains an invalid ISO 3166 code
invalid_value: must have values `:with_regions` or `:without_regions`

View file

@ -17,3 +17,7 @@ fr:
is_reserved: est réservé
valid_domain_csv_validator:
invalid_list_format: doit être une liste de domaines valides, séparés par des virgules.
iso3166_hash_validator:
is_blank: doit être fourni
invalid_key: contient un code non valide de la norme ISO 3166-2
invalid_value: doit avoir des valeurs `:with_regions` ou `:without_regions`

View file

@ -138,7 +138,7 @@ describe('Billboards Form', () => {
cy.findByRole('button', { name: 'Save Billboard' }).click();
cy.get('#flash-0').should(($flashMessage) => {
expect($flashMessage).to.not.contain(
'is not a supported ISO 3166-2 code',
'is not an enabled target ISO 3166-2 code',
);
});
});
@ -151,10 +151,10 @@ describe('Billboards Form', () => {
cy.get('#flash-0').should(($flashMessage) => {
// We currently support only the US and CA
expect($flashMessage).to.contain(
'MX-CMX is not a supported ISO 3166-2 code',
'MX-CMX is not an enabled target ISO 3166-2 code',
);
expect($flashMessage).to.not.contain(
'US-NY is not a supported ISO 3166-2 code',
'US-NY is not an enabled target ISO 3166-2 code',
);
});
});

View file

@ -0,0 +1,155 @@
describe('Monetization section', () => {
beforeEach(() => {
cy.testSetup();
cy.fixture('users/adminUser.json').as('user');
cy.get('@user').then((user) => cy.loginUser(user));
});
function withinMonetizationSection(callback) {
cy.findByText('Monetization')
.as('monetizationSectionHeader')
.parent()
.within(() => {
cy.get('@monetizationSectionHeader').click();
callback();
});
}
context('when location targeting is not enabled', () => {
beforeEach(() => {
cy.visit('/admin/customization/config');
});
it('does not show countries for billboard geotargeting', () => {
withinMonetizationSection(() => {
cy.findByText('Billboard enabled countries').should('not.exist');
cy.findByRole('combobox').should('not.exist');
cy.findByLabelText('Enabled countries for targeting').should(
'not.exist',
);
});
});
});
context('when location targeting is enabled', () => {
beforeEach(() => {
cy.enableFeatureFlag('billboard_location_targeting');
cy.visit('/admin/customization/config');
});
it('autocompletes countries for selection', () => {
withinMonetizationSection(() => {
cy.findByText('Billboard enabled countries').should('exist');
cy.findByLabelText('Enabled countries for targeting').as('textbox');
cy.findByRole('listbox').should('not.exist');
cy.get('@textbox').type('ger');
cy.findByRole('listbox')
.should('contain.text', 'Germany')
.should('contain.text', 'Algeria')
.should('contain.text', 'Niger')
.should('contain.text', 'Nigeria');
cy.get('@textbox').clear();
cy.findByRole('listbox').should('not.exist');
cy.get('@textbox').type('IND');
cy.findByRole('listbox')
.should('contain.text', 'Indonesia')
.should('contain.text', 'India');
cy.get('@textbox').clear();
cy.findByRole('listbox').should('not.exist');
cy.get('@textbox').type('Uni');
cy.findByRole('listbox')
.should('contain.text', 'Réunion')
.should('contain.text', 'Tunisia')
.should('contain.text', 'United Arab Emirates')
.should('contain.text', 'United Kingdom');
});
});
it('allows the user to select and deselect countries', () => {
withinMonetizationSection(() => {
cy.findByRole('combobox')
.as('selections')
.findByLabelText('France')
.should('not.exist');
cy.get('@selections').findByLabelText('Canada').should('exist');
cy.get('@selections').findByLabelText('United States').should('exist');
cy.findByLabelText('Enabled countries for targeting').type('Fra');
cy.findByRole('listbox').findByText('France').click();
cy.get('@selections').findByLabelText('France').should('exist');
cy.get('@selections').findByLabelText('Canada').should('exist');
cy.get('@selections').findByLabelText('United States').should('exist');
cy.findByRole('button', { name: 'Remove Canada' }).click();
cy.get('@selections').findByLabelText('France').should('exist');
cy.get('@selections').findByLabelText('Canada').should('not.exist');
cy.get('@selections').findByLabelText('United States').should('exist');
cy.findByRole('button', { name: 'Update Settings' }).click();
});
cy.findByTestId('snackbar')
.should('be.visible')
.should('have.text', 'Successfully updated settings.');
cy.reload();
withinMonetizationSection(() => {
cy.findByRole('combobox').as('selections');
cy.get('@selections').findByLabelText('France').should('exist');
cy.get('@selections').findByLabelText('Canada').should('not.exist');
cy.get('@selections').findByLabelText('United States').should('exist');
});
});
it('allows the user to enable and disable region targeting', () => {
const toggle = (country, { expectedText }) => {
cy.get('@selections')
.findByRole('group', { name: country })
.findByRole('button', { name: 'Toggle region targeting' })
.click();
cy.get('@selections')
.findByRole('group', { name: country })
.should('contain.text', expectedText);
};
withinMonetizationSection(() => {
cy.findByRole('combobox').as('selections');
cy.get('@selections')
.findByRole('group', { name: 'United States' })
.should('contain.text', 'Including regions');
cy.get('@selections')
.findByRole('group', { name: 'Canada' })
.should('contain.text', 'Including regions');
toggle('United States', { expectedText: 'Excluding regions' });
toggle('United States', { expectedText: 'Including regions' });
toggle('Canada', { expectedText: 'Excluding regions' });
cy.findByRole('button', { name: 'Update Settings' }).click();
});
cy.findByTestId('snackbar')
.should('be.visible')
.should('have.text', 'Successfully updated settings.');
cy.reload();
withinMonetizationSection(() => {
cy.findByRole('combobox').as('selections');
cy.get('@selections')
.findByRole('group', { name: 'United States' })
.should('contain.text', 'Including regions');
cy.get('@selections')
.findByRole('group', { name: 'Canada' })
.should('contain.text', 'Excluding regions');
});
});
});
});

View file

@ -108,6 +108,7 @@
"@babel/plugin-proposal-class-properties": "^7.16.0",
"@babel/plugin-proposal-export-default-from": "^7.22.5",
"@babel/plugin-syntax-dynamic-import": "^7.8.3",
"@babel/plugin-transform-private-methods": "^7.22.5",
"@babel/plugin-transform-react-jsx": "^7.22.5",
"@babel/preset-env": "^7.22.10",
"@babel/preset-typescript": "^7.22.5",

View file

@ -361,7 +361,7 @@ RSpec.describe Billboard do
it "does not permit them" do
expect(billboard).not_to be_valid
expect(billboard.errors_as_sentence).to include("NOT-REAL is not a supported ISO 3166-2 code")
expect(billboard.errors_as_sentence).to include("NOT-REAL is not an enabled target ISO 3166-2 code")
end
end
@ -370,7 +370,45 @@ RSpec.describe Billboard do
it "does not permit them" do
expect(billboard).not_to be_valid
expect(billboard.errors_as_sentence).to include("CA-FAKE is not a supported ISO 3166-2 code")
expect(billboard.errors_as_sentence).to include("CA-FAKE is not an enabled target ISO 3166-2 code")
end
end
context "with valid ISO 3166-2 country codes that are not enabled" do
let(:geo_input) { %w[MX NG] }
it "does not permit them" do
expect(billboard).not_to be_valid
expect(billboard.errors_as_sentence).to include("MX is not an enabled target ISO 3166-2 code")
expect(billboard.errors_as_sentence).to include("NG is not an enabled target ISO 3166-2 code")
end
end
context "with countries that don't have region targeting enabled" do
let(:geo_input) { %w[FR-BRE NL GB US-CA] }
let(:enabled_countries) do
{
"US" => :with_regions,
"FR" => :without_regions,
"NL" => :without_regions,
"GB" => :without_regions
}
end
it "permits country-level targets but not region-level ones" do
allow(Settings::General).to receive(:billboard_enabled_countries).and_return(enabled_countries)
expect(billboard).not_to be_valid
expect(billboard.errors_as_sentence).to include("FR-BRE is not an enabled target ISO 3166-2 code")
# Remove faulty region targeting
billboard.target_geolocations = %w[FR NL GB US-CA]
expect(billboard).to be_valid
# Allow French region targeting
enabled_countries["FR"] = :with_regions
billboard.target_geolocations = geo_input
expect(billboard).to be_valid
end
end
end

View file

@ -3,15 +3,15 @@ require "rails_helper"
RSpec.describe Geolocation do
describe "validations" do
describe "country validation" do
it "allows only the US and Canada (for now)" do
united_states = described_class.new("US")
canada = described_class.new("CA")
netherlands = described_class.new("NL")
india = described_class.new("IN")
south_africa = described_class.new("ZA")
unassigned_country = described_class.new("ZZ")
invalid_code_country = described_class.new("not iso3166")
let(:united_states) { described_class.new("US") }
let(:canada) { described_class.new("CA") }
let(:netherlands) { described_class.new("NL") }
let(:india) { described_class.new("IN") }
let(:south_africa) { described_class.new("ZA") }
let(:unassigned_country) { described_class.new("ZZ") }
let(:invalid_code_country) { described_class.new("not iso3166") }
it "allows only the US and Canada by default" do
expect(united_states).to be_valid
expect(canada).to be_valid
expect(netherlands).not_to be_valid
@ -20,27 +20,68 @@ RSpec.describe Geolocation do
expect(unassigned_country).not_to be_valid
expect(invalid_code_country).not_to be_valid
end
it "respects the specified enabled locations in the settings" do
allow(Settings::General).to receive(:billboard_enabled_countries).and_return(
"CA" => :without_regions,
"IN" => :without_regions,
"ZA" => :without_regions,
)
expect(united_states).not_to be_valid
expect(canada).to be_valid
expect(netherlands).not_to be_valid
expect(india).to be_valid
expect(south_africa).to be_valid
expect(unassigned_country).not_to be_valid
expect(invalid_code_country).not_to be_valid
end
end
describe "region validation" do
# WA: State of Washington (within the US)
# QC: Province of Québec (within Canada)
let(:us_with_us_region) { described_class.new("US", "WA") }
let(:canada_with_canada_region) { described_class.new("CA", "QC") }
let(:us_with_canada_region) { described_class.new("US", "QC") }
let(:canada_with_us_region) { described_class.new("CA", "WA") }
it "allows an empty region" do
without_region = described_class.new("US")
expect(without_region).to be_valid
end
it "allows only regions that are a part of the country" do
# WA: State of Washington (within the US)
# QC: Province of Québec (within Canada)
us_with_us_region = described_class.new("US", "WA")
canada_with_canada_region = described_class.new("CA", "QC")
us_with_canada_region = described_class.new("US", "QC")
canada_with_us_region = described_class.new("CA", "WA")
expect(us_with_us_region).to be_valid
expect(canada_with_canada_region).to be_valid
expect(us_with_canada_region).not_to be_valid
expect(canada_with_us_region).not_to be_valid
end
# rubocop:disable RSpec/NestedGroups
context "when the country does not allow region targeting" do
before do
allow(Settings::General).to receive(:billboard_enabled_countries).and_return(
"US" => :without_regions,
"CA" => :with_regions,
)
end
it "allows regions for the country by default (for querying)" do
expect(us_with_us_region).to be_valid
expect(canada_with_canada_region).to be_valid
expect(us_with_canada_region).not_to be_valid
expect(canada_with_us_region).not_to be_valid
end
it "does not allow regions for the country in a targeting context" do
expect(us_with_us_region).not_to be_valid(:targeting)
expect(canada_with_canada_region).to be_valid(:targeting)
expect(us_with_canada_region).not_to be_valid(:targeting)
expect(canada_with_us_region).not_to be_valid(:targeting)
end
end
# rubocop:enable RSpec/NestedGroups
end
end

View file

@ -49,5 +49,41 @@ RSpec.describe Settings::General do
expect { described_class.feed_pinned_article_id = article.id }.not_to raise_error
end
end
describe "validating :billboard_enabled_countries" do
it "does not accept non-hash or empty values" do
expect { described_class.billboard_enabled_countries = "string" }.to raise_error(ActiveRecord::RecordInvalid)
expect { described_class.billboard_enabled_countries = [] }.to raise_error(ActiveRecord::RecordInvalid)
expect { described_class.billboard_enabled_countries = {} }.to raise_error(ActiveRecord::RecordInvalid)
end
it "accepts any valid ISO 3166-2 codes as keys" do
countries = ISO3166::Country.codes.sample(5).index_with { :with_regions }
expect { described_class.billboard_enabled_countries = countries }.not_to raise_error
end
it "does not accept invalid ISO 3166-2 codes as keys" do
countries = { "XX" => :with_regions, "ZZ" => :with_regions }
expect { described_class.billboard_enabled_countries = countries }.to raise_error(ActiveRecord::RecordInvalid)
end
it "allows 'with_regions' and 'without_regions' marker as values to enable/disable region targeting" do
countries = { "CA" => :with_regions, "ZA" => :with_regions, "GB" => :without_regions }
expect { described_class.billboard_enabled_countries = countries }.not_to raise_error
end
it "does not allow arbitrary strings or symbols as values" do
countries = { "CA" => :with_regions, "US" => "string" }
other_countries = { "CA" => :with_regions, "US" => :string }
expect { described_class.billboard_enabled_countries = countries }.to raise_error(ActiveRecord::RecordInvalid)
expect do
described_class.billboard_enabled_countries = other_countries
end.to raise_error(ActiveRecord::RecordInvalid)
end
end
end
end

View file

@ -284,6 +284,38 @@ RSpec.describe Billboards::FilteredAdsQuery, type: :query do
targets_maine_alberta_and_ontario,
)
end
it "shows only billboards with no targeting if the user's location is an unsupported country" do
filtered = filter_billboards(location: "FR-BRE")
expect(filtered).to include(no_targets)
expect(filtered).not_to include(
targets_canada,
targets_new_york_and_canada,
targets_california_and_texas,
targets_quebec_and_newfoundland,
targets_maine_alberta_and_ontario,
)
end
it "correctly shows targeted billboards if country but not region targeting is enabled" do
allow(Settings::General).to receive(:billboard_enabled_countries).and_return(
"FR" => :without_regions,
"US" => :with_regions,
"CA" => :with_regions,
)
targets_france_and_canada = create_billboard(target_geolocations: "CA, FR")
filtered = filter_billboards(location: "FR-BRE")
expect(filtered).to include(no_targets, targets_france_and_canada)
expect(filtered).not_to include(
targets_canada,
targets_new_york_and_canada,
targets_california_and_texas,
targets_quebec_and_newfoundland,
targets_maine_alberta_and_ontario,
)
end
end
end
end

View file

@ -94,7 +94,7 @@ RSpec.describe "Api::V1::Billboards" do
expect(response).to have_http_status(:unprocessable_entity)
expect(response.media_type).to eq("application/json")
expect(response.parsed_body.keys).to contain_exactly("error", "status")
expect(response.parsed_body["error"]).to include("US-FAKE is not a supported ISO 3166-2 code")
expect(response.parsed_body["error"]).to include("US-FAKE is not an enabled target ISO 3166-2 code")
end
end
@ -158,7 +158,7 @@ RSpec.describe "Api::V1::Billboards" do
expect(response).to have_http_status(:unprocessable_entity)
expect(response.media_type).to eq("application/json")
expect(response.parsed_body.keys).to contain_exactly("error", "status")
expect(response.parsed_body["error"]).to include("US-FAKE is not a supported ISO 3166-2 code")
expect(response.parsed_body["error"]).to include("US-FAKE is not an enabled target ISO 3166-2 code")
end
end