add field for billboard target geolocations, with unit and e2e tests (#19855)
Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com> Co-authored-by: Duke Greene <dukegreene@gmail.com> Co-authored-by: PJ <pj@forem.com> Co-authored-by: Anna Buianova <lightallloy@gmail.com> Co-authored-by: Mac Siri <mac@forem.com> Co-authored-by: zhangted <tedcbook@gmail.com> Co-authored-by: Josh Klar <jklar@forem.com>
This commit is contained in:
Duke Greene
GitHub
parent
477432b9a0
commit
1ef88365c4
4 changed files with 101 additions and 22 deletions
|
|
@ -59,8 +59,9 @@ module Admin
|
|||
private
|
||||
|
||||
def billboard_params
|
||||
params.permit(:organization_id, :body_markdown, :placement_area, :published, :approved, :name, :display_to,
|
||||
:tag_list, :type_of, :exclude_article_ids, :audience_segment_id, :priority)
|
||||
params.permit(:organization_id, :body_markdown, :placement_area, :target_geolocations,
|
||||
:published, :approved, :name, :display_to, :tag_list, :type_of,
|
||||
:exclude_article_ids, :audience_segment_id, :priority)
|
||||
end
|
||||
|
||||
def authorize_admin
|
||||
|
|
|
|||
|
|
@ -31,6 +31,13 @@
|
|||
<div id="display-ad-targeted-tags"></div>
|
||||
</div>
|
||||
|
||||
<% if FeatureFlag.enabled?(Geolocation::FEATURE_FLAG) %>
|
||||
<div class="crayons-field">
|
||||
<%= label_tag :target_geolocations, "Target Geolocations:", class: "crayons-field__label" %>
|
||||
<%= text_field_tag :target_geolocations, @billboard.target_geolocations.map(&:to_iso3166).join(", "), class: "crayons-textfield", placeholder: "US-NY, CA-ON", autocomplete: "off" %>
|
||||
</div>
|
||||
<% end %>
|
||||
|
||||
<div class="crayons-field hidden">
|
||||
<%= label_tag :tag_list, "Tag List:", class: "crayons-field__label" %>
|
||||
<%= text_field_tag :tag_list, @billboard.tag_list.to_s, class: "crayons-textfield js-tags-textfield", autocomplete: "off" %>
|
||||
|
|
|
|||
|
|
@ -65,7 +65,9 @@ describe('Billboards Form', () => {
|
|||
.should('be.visible');
|
||||
|
||||
cy.get('@audienceSegments').select('Are trusted').should('exist');
|
||||
cy.get('@audienceSegments').select('Have not posted yet').should('exist');
|
||||
cy.get('@audienceSegments')
|
||||
.select('Have not posted yet')
|
||||
.should('exist');
|
||||
cy.get('@audienceSegments')
|
||||
.select('Have not set an experience level')
|
||||
.should('exist');
|
||||
|
|
@ -75,30 +77,85 @@ describe('Billboards Form', () => {
|
|||
.should('not.exist');
|
||||
});
|
||||
|
||||
context('when editing a display ad with a manually managed audience', () => {
|
||||
beforeEach(() => {
|
||||
cy.visit('/admin/customization/billboards');
|
||||
cy.findByRole('link', { name: 'Manual Audience Billboard' }).click({
|
||||
context(
|
||||
'when editing a display ad with a manually managed audience',
|
||||
() => {
|
||||
beforeEach(() => {
|
||||
cy.visit('/admin/customization/billboards');
|
||||
cy.findByRole('link', { name: 'Manual Audience Billboard' }).click({
|
||||
force: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('shows the audience segment field but disabled', () => {
|
||||
cy.findByLabelText('Users who:')
|
||||
.as('audienceSegments')
|
||||
.should('be.disabled');
|
||||
|
||||
cy.get('@audienceSegments')
|
||||
.find(':selected')
|
||||
.should('have.text', 'Managed elsewhere');
|
||||
|
||||
cy.get('@audienceSegments')
|
||||
.contains('Are trusted')
|
||||
.should('not.exist');
|
||||
cy.get('@audienceSegments')
|
||||
.contains('Have not posted yet')
|
||||
.should('not.exist');
|
||||
cy.get('@audienceSegments')
|
||||
.contains('Have not set an experience level')
|
||||
.should('not.exist');
|
||||
});
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe('Target Geolocations Field', () => {
|
||||
beforeEach(() => {
|
||||
cy.enableFeatureFlag('billboard_location_targeting');
|
||||
cy.get('@user').then((user) => {
|
||||
cy.loginAndVisit(user, '/admin/customization/billboards');
|
||||
cy.findByRole('link', { name: 'Make A New Display Ad' }).click({
|
||||
force: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
afterEach(() => {
|
||||
cy.disableFeatureFlag('billboard_location_targeting');
|
||||
});
|
||||
|
||||
it('shows the audience segment field but disabled', () => {
|
||||
cy.findByLabelText('Users who:')
|
||||
.as('audienceSegments')
|
||||
.should('be.disabled');
|
||||
it('should show a placeholder with valid geolocation codes', () => {
|
||||
cy.findByLabelText('Target Geolocations:')
|
||||
.as('targetGeolocations')
|
||||
.should('exist');
|
||||
cy.get('input[placeholder="US-NY, CA-ON"]').should('exist');
|
||||
});
|
||||
|
||||
cy.get('@audienceSegments')
|
||||
.find(':selected')
|
||||
.should('have.text', 'Managed elsewhere');
|
||||
it('should not return iso3166 errors if given valid geolocation code inputs', () => {
|
||||
cy.findByRole('textbox', { name: 'Target Geolocations:' }).type(
|
||||
'CA-ON, US-OH, US-MI',
|
||||
);
|
||||
cy.findByRole('button', { name: 'Save Display Ad' }).click();
|
||||
cy.get('#flash-0').should(($flashMessage) => {
|
||||
expect($flashMessage).to.not.contain(
|
||||
'is not a supported ISO 3166-2 code',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
cy.get('@audienceSegments').contains('Are trusted').should('not.exist');
|
||||
cy.get('@audienceSegments')
|
||||
.contains('Have not posted yet')
|
||||
.should('not.exist');
|
||||
cy.get('@audienceSegments')
|
||||
.contains('Have not set an experience level')
|
||||
.should('not.exist');
|
||||
it('should generate errors if some or all of the input is invalid', () => {
|
||||
cy.findByRole('textbox', { name: 'Target Geolocations:' }).type(
|
||||
'US-NY, MX-CMX',
|
||||
);
|
||||
cy.findByRole('button', { name: 'Save Display Ad' }).click();
|
||||
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',
|
||||
);
|
||||
expect($flashMessage).to.not.contain(
|
||||
'US-NY is not a supported ISO 3166-2 code',
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -4,8 +4,10 @@ require "requests/shared_examples/internal_policy_dependant_request"
|
|||
RSpec.describe "/admin/customization/billboards" do
|
||||
let(:get_resource) { get admin_billboards_path }
|
||||
let(:org) { create(:organization) }
|
||||
let(:geolocations) { "US-NY, US-ME, CA-ON" }
|
||||
let(:params) do
|
||||
{ organization_id: org.id, body_markdown: "[Click here!](https://example.com)", placement_area: "sidebar_left",
|
||||
{ organization_id: org.id, body_markdown: "[Click here!](https://example.com)",
|
||||
target_geolocations: geolocations, placement_area: "sidebar_left",
|
||||
approved: true, published: true, priority: true }
|
||||
end
|
||||
let(:post_resource) { post admin_billboards_path, params: params }
|
||||
|
|
@ -61,6 +63,18 @@ RSpec.describe "/admin/customization/billboards" do
|
|||
post_resource
|
||||
expect(DisplayAd.last.creator_id).to eq(super_admin.id)
|
||||
end
|
||||
|
||||
it "fails to create a new billboard with invalid target geolocations" do
|
||||
expect do
|
||||
post admin_billboards_path, params: params.merge(target_geolocations: "US-UM, CA-UH")
|
||||
end.not_to change { DisplayAd.all.count }
|
||||
end
|
||||
|
||||
it "creates a new billboard with no target geolocations" do
|
||||
expect do
|
||||
post admin_billboards_path, params: params.merge(target_geolocations: nil)
|
||||
end.to change { DisplayAd.all.count }.by(1)
|
||||
end
|
||||
end
|
||||
|
||||
describe "PUT /admin/customization/billboards" do
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue