From 0ddb133c4959954f92d1249f6acb9933032abcec Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Thu, 2 Nov 2023 16:58:28 -0400 Subject: [PATCH] Add ability for admins to request specific billboards for inspection (#20311) --- app/controllers/billboards_controller.rb | 10 +++++ app/javascript/__tests__/billboard.test.js | 27 +++++++++++ app/javascript/packs/billboard.js | 9 ++-- config/fastly/snippets/safe_params_list.vcl | 2 +- spec/requests/billboards_spec.rb | 50 +++++++++++++++++++++ 5 files changed, 94 insertions(+), 4 deletions(-) diff --git a/app/controllers/billboards_controller.rb b/app/controllers/billboards_controller.rb index 7bf030213..959ce92d2 100644 --- a/app/controllers/billboards_controller.rb +++ b/app/controllers/billboards_controller.rb @@ -15,6 +15,12 @@ class BillboardsController < ApplicationController end if placement_area + if return_test_billboard? + @billboard = Billboard.find_by(id: params[:bb_test_id]) + render layout: false + return + end + if params[:username].present? && params[:slug].present? @article = Article.find_by(slug: params[:slug]) end @@ -59,4 +65,8 @@ class BillboardsController < ApplicationController request.headers["X-Cacheable-Client-Geo"] end end + + def return_test_billboard? + params[:bb_test_placement_area] == placement_area && params[:bb_test_id].present? && current_user&.any_admin? + end end diff --git a/app/javascript/__tests__/billboard.test.js b/app/javascript/__tests__/billboard.test.js index e27fe9066..23fd91628 100644 --- a/app/javascript/__tests__/billboard.test.js +++ b/app/javascript/__tests__/billboard.test.js @@ -90,6 +90,33 @@ describe('getBillboard', () => { expect(script.innerHTML).toEqual('console.log("test")'); // Should retain content }); }); + + test('should add current URL parameters to asyncUrl if bb_test_placement_area exists', async () => { + // Mocking window.location.href + delete window.location; + window.location = new URL( + 'http://example.com?bb_test_placement_area=post_sidebar&bb_test_id=1', + ); + + document.body.innerHTML = ` +
+
+
+ `; + + global.fetch = jest.fn(() => + Promise.resolve({ + text: () => Promise.resolve('
Some HTML content
'), + }), + ); + + await getBillboard(); + + // Check if the fetch function was called with the modified URL including the parameters + expect(global.fetch).toHaveBeenCalledWith( + '/billboards/post_sidebar?bb_test_placement_area=post_sidebar&bb_test_id=1', + ); + }); }); describe('executeBBScripts', () => { diff --git a/app/javascript/packs/billboard.js b/app/javascript/packs/billboard.js index c76cc3d62..1fa677ac3 100644 --- a/app/javascript/packs/billboard.js +++ b/app/javascript/packs/billboard.js @@ -14,11 +14,14 @@ export async function getBillboard() { } async function generateBillboard(element) { - const { asyncUrl } = element.dataset; - + let { asyncUrl } = element.dataset; + const currentParams = window.location.href.split('?')[1]; + if (currentParams && currentParams.includes('bb_test_placement_area')) { + asyncUrl = `${asyncUrl}?${currentParams}`; + } if (asyncUrl) { try { - const response = await window.fetch(`${asyncUrl}`); + const response = await window.fetch(asyncUrl); const htmlContent = await response.text(); const generatedElement = document.createElement('div'); diff --git a/config/fastly/snippets/safe_params_list.vcl b/config/fastly/snippets/safe_params_list.vcl index 70e1bfd58..9f8cd3ded 100644 --- a/config/fastly/snippets/safe_params_list.vcl +++ b/config/fastly/snippets/safe_params_list.vcl @@ -2,6 +2,6 @@ import querystring; sub vcl_recv { # return this URL with only the parameters that match this regular expression if (req.url !~ "/admin/" && req.url !~ "/search/" && req.url !~ "/bulk_show") { - set req.url = querystring.regfilter_except(req.url, "^(a_id|args|article_id|article_ids|articles|asc|callback_url|category|client_id|code|collection_id|commentable_id|commentable_type|confirmation_token|created_at|end|email|filter|followable_id|followable_type|forem_owner_secret|fork_id|i|key|message_offset|name|oauth_token|oauth_verifier|offset|onboarding|org_id|organization_id|p|page|per_page|p_id|placement_area|prefill|preview|purchaser|q|reactable_ids|redirect_uri|reported_url|reporter_username|response_type|scope|search|signature|sort|source_id|source_type|start|state|status|tag|tag_list|top|type_of|url|username|invitation_token|reset_password_token|ut|verb|invitation_slug|period|comments_sort|billboard|controller_action)$"); + set req.url = querystring.regfilter_except(req.url, "^(a_id|args|article_id|article_ids|articles|asc|callback_url|category|client_id|code|collection_id|commentable_id|commentable_type|confirmation_token|created_at|end|email|filter|followable_id|followable_type|forem_owner_secret|fork_id|i|key|message_offset|name|oauth_token|oauth_verifier|offset|onboarding|org_id|organization_id|p|page|per_page|p_id|placement_area|prefill|preview|purchaser|q|reactable_ids|redirect_uri|reported_url|reporter_username|response_type|scope|search|signature|sort|source_id|source_type|start|state|status|tag|tag_list|top|type_of|url|username|invitation_token|reset_password_token|ut|verb|invitation_slug|period|comments_sort|billboard|controller_action|bb_test_placement_area|bb_test_id)$"); } } diff --git a/spec/requests/billboards_spec.rb b/spec/requests/billboards_spec.rb index 1f12ddb42..7e2cc7bea 100644 --- a/spec/requests/billboards_spec.rb +++ b/spec/requests/billboards_spec.rb @@ -165,5 +165,55 @@ RSpec.describe "Billboards" do expect(response.body).not_to include "crayons-sponsorship__header relative" end end + + context "when requesting test billboard" do + let(:admin) { create(:user, :admin) } + let!(:test_billboard) { create_billboard(id: 123, placement_area: "post_sidebar", approved: false) } + + before do + sign_in admin + end + + it "returns the test billboard when proper parameters are provided" do + get article_billboard_path( + username: article.username, + slug: article.slug, + placement_area: "post_sidebar", + bb_test_placement_area: "post_sidebar", + bb_test_id: test_billboard.id, + ) + + expect(response).to have_http_status(:ok) + expect(response.body).to include(test_billboard.processed_html) + end + + it "does not return the test billboard when parameters are missing" do + get article_billboard_path( + username: article.username, + slug: article.slug, + placement_area: "post_sidebar", + bb_test_id: test_billboard.id, + ) + + expect(response).to have_http_status(:ok) + expect(response.body).not_to include(test_billboard.processed_html) + end + + it "does not return the test billboard for non-admin users" do + sign_out admin + sign_in create(:user) + + get article_billboard_path( + username: article.username, + slug: article.slug, + placement_area: "post_sidebar", + bb_test_placement_area: "post_sidebar", + bb_test_id: test_billboard.id, + ) + + expect(response).to have_http_status(:ok) + expect(response.body).not_to include(test_billboard.processed_html) + end + end end end