From daa581d8f27965410b4a08385bc926069178a627 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Wed, 30 Mar 2022 10:10:39 -0400 Subject: [PATCH] REFACTOR: Abstract search link click logging (#16317) --- .../app/components/search-result-entry.js | 13 +++++++++ .../app/controllers/full-page-search.js | 14 ++++------ .../javascripts/discourse/app/lib/search.js | 11 ++++++++ .../components/search-result-entries.hbs | 8 +++++- .../app/templates/full-page-search.hbs | 1 + .../discourse/app/widgets/header.js | 10 ++----- .../tests/acceptance/search-full-test.js | 28 +++++++++++++++++-- .../tests/fixtures/search-fixtures.js | 4 +-- 8 files changed, 66 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/search-result-entry.js b/app/assets/javascripts/discourse/app/components/search-result-entry.js index 73e340671fa..9312174ed18 100644 --- a/app/assets/javascripts/discourse/app/components/search-result-entry.js +++ b/app/assets/javascripts/discourse/app/components/search-result-entry.js @@ -1,4 +1,6 @@ import Component from "@ember/component"; +import { action } from "@ember/object"; +import { logSearchLinkClick } from "discourse/lib/search"; export default Component.extend({ tagName: "div", @@ -6,4 +8,15 @@ export default Component.extend({ classNameBindings: ["bulkSelectEnabled"], attributeBindings: ["role"], role: "listitem", + + @action + logClick(topicId) { + if (this.searchLogId && topicId) { + logSearchLinkClick({ + searchLogId: this.searchLogId, + searchResultId: topicId, + searchResultType: "topic", + }); + } + }, }); diff --git a/app/assets/javascripts/discourse/app/controllers/full-page-search.js b/app/assets/javascripts/discourse/app/controllers/full-page-search.js index c364cb3f815..65922b664a2 100644 --- a/app/assets/javascripts/discourse/app/controllers/full-page-search.js +++ b/app/assets/javascripts/discourse/app/controllers/full-page-search.js @@ -3,6 +3,7 @@ import discourseComputed, { observes } from "discourse-common/utils/decorators"; import { getSearchKey, isValidSearchTerm, + logSearchLinkClick, searchContextDescription, translateResults, updateRecentSearches, @@ -470,15 +471,10 @@ export default Controller.extend({ logClick(topicId) { if (this.get("model.grouped_search_result.search_log_id") && topicId) { - ajax("/search/click", { - type: "POST", - data: { - search_log_id: this.get( - "model.grouped_search_result.search_log_id" - ), - search_result_id: topicId, - search_result_type: "topic", - }, + logSearchLinkClick({ + searchLogId: this.get("model.grouped_search_result.search_log_id"), + searchResultId: topicId, + searchResultType: "topic", }); } }, diff --git a/app/assets/javascripts/discourse/app/lib/search.js b/app/assets/javascripts/discourse/app/lib/search.js index 6df0545edea..40352647edb 100644 --- a/app/assets/javascripts/discourse/app/lib/search.js +++ b/app/assets/javascripts/discourse/app/lib/search.js @@ -244,3 +244,14 @@ export function updateRecentSearches(currentUser, term) { recentSearches.unshiftObject(term); currentUser.set("recent_searches", recentSearches); } + +export function logSearchLinkClick(params) { + ajax("/search/click", { + type: "POST", + data: { + search_log_id: params.searchLogId, + search_result_id: params.searchResultId, + search_result_type: params.searchResultType, + }, + }); +} diff --git a/app/assets/javascripts/discourse/app/templates/components/search-result-entries.hbs b/app/assets/javascripts/discourse/app/templates/components/search-result-entries.hbs index ea93c26dbdd..941ec29b38f 100644 --- a/app/assets/javascripts/discourse/app/templates/components/search-result-entries.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/search-result-entries.hbs @@ -1,5 +1,11 @@
{{#each posts as |post|}} - {{search-result-entry post=post bulkSelectEnabled=bulkSelectEnabled selected=selected highlightQuery=highlightQuery}} + {{search-result-entry + post=post + bulkSelectEnabled=bulkSelectEnabled + selected=selected + highlightQuery=highlightQuery + searchLogId=searchLogId + }} {{/each}}
diff --git a/app/assets/javascripts/discourse/app/templates/full-page-search.hbs b/app/assets/javascripts/discourse/app/templates/full-page-search.hbs index 4669c11f3e2..1fb8cb40768 100644 --- a/app/assets/javascripts/discourse/app/templates/full-page-search.hbs +++ b/app/assets/javascripts/discourse/app/templates/full-page-search.hbs @@ -131,6 +131,7 @@ bulkSelectEnabled=bulkSelectEnabled selected=selected highlightQuery=highlightQuery + searchLogId=model.grouped_search_result.search_log_id }} {{#conditional-loading-spinner condition=loading}} diff --git a/app/assets/javascripts/discourse/app/widgets/header.js b/app/assets/javascripts/discourse/app/widgets/header.js index 377e76304db..4885bbf1981 100644 --- a/app/assets/javascripts/discourse/app/widgets/header.js +++ b/app/assets/javascripts/discourse/app/widgets/header.js @@ -10,6 +10,7 @@ import { iconNode } from "discourse-common/lib/icon-library"; import { schedule } from "@ember/runloop"; import { scrollTop } from "discourse/mixins/scroll-top"; import { wantsNewWindow } from "discourse/lib/intercept-click"; +import { logSearchLinkClick } from "discourse/lib/search"; const _extraHeaderIcons = []; @@ -426,14 +427,7 @@ export default createWidget("header", { const { searchLogId, searchResultId, searchResultType } = attrs; if (searchLogId && searchResultId && searchResultType) { - ajax("/search/click", { - type: "POST", - data: { - search_log_id: searchLogId, - search_result_id: searchResultId, - search_result_type: searchResultType, - }, - }); + logSearchLinkClick({ searchLogId, searchResultId, searchResultType }); } } diff --git a/app/assets/javascripts/discourse/tests/acceptance/search-full-test.js b/app/assets/javascripts/discourse/tests/acceptance/search-full-test.js index 8331ebec021..a4b1ff73023 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/search-full-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/search-full-test.js @@ -6,7 +6,7 @@ import { selectDate, visible, } from "discourse/tests/helpers/qunit-helpers"; -import { click, fillIn, visit } from "@ember/test-helpers"; +import { click, currentURL, fillIn, visit } from "@ember/test-helpers"; import { test } from "qunit"; import { SEARCH_TYPE_CATS_TAGS, @@ -16,6 +16,7 @@ import { import selectKit from "discourse/tests/helpers/select-kit-helper"; let lastBody; +let searchResultClickTracked = false; acceptance("Search - Full Page", function (needs) { needs.user(); @@ -95,7 +96,16 @@ acceptance("Search - Full Page", function (needs) { server.put("/topics/bulk", (request) => { lastBody = helper.parsePostData(request.requestBody); - return helper.response({ topic_ids: [7] }); + return helper.response({ topic_ids: [130] }); + }); + + server.post("/search/click", () => { + searchResultClickTracked = true; + return helper.response({ success: "OK" }); + }); + + needs.hooks.afterEach(() => { + searchResultClickTracked = false; }); }); @@ -556,7 +566,7 @@ acceptance("Search - Full Page", function (needs) { await click(".bulk-select-visible .btn:nth-child(2)"); // select all await click(".bulk-select-btn"); // show bulk actions await click(".topic-bulk-actions-modal .btn:nth-child(2)"); // close topics - assert.equal(lastBody["topic_ids[]"], 7); + assert.equal(lastBody["topic_ids[]"], 130); }); test("adds visited class to visited topics", async function (assert) { @@ -570,4 +580,16 @@ acceptance("Search - Full Page", function (needs) { await click(".search-cta"); assert.equal(queryAll(".visited").length, 1); }); + + test("result link click tracking is invoked", async function (assert) { + await visit("/search"); + + await fillIn(".search-query", "discourse"); + await click(".search-cta"); + + await click("a.search-link:first-child"); + + assert.strictEqual(currentURL(), "/t/lorem-ipsum-dolor-sit-amet/130"); + assert.ok(searchResultClickTracked); + }); }); diff --git a/app/assets/javascripts/discourse/tests/fixtures/search-fixtures.js b/app/assets/javascripts/discourse/tests/fixtures/search-fixtures.js index bfb8a449991..1251166c8cd 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/search-fixtures.js +++ b/app/assets/javascripts/discourse/tests/fixtures/search-fixtures.js @@ -11,12 +11,12 @@ export default { blurb: "The first paragraph of this pinned topic will be visible as a welcome message to all new visitors on your homepage. It’s important! Edit this into a brief description of your community: Who is it for? What can they find here? Why should they come here? Where can they read more (links, resources, ...", post_number: 1, - topic_id: 7, + topic_id: 130, }, ], topics: [ { - id: 7, + id: 130, title: "Welcome to Discourse", fancy_title: "Welcome to Discourse", slug: "welcome-to-discourse",