Create feed impression and click events (#20043)

* getting started: model setup

* basic controller action & spec

* somewhat working click events

* pick up on impressions

* there was an attempt at batching without duplicates

* (better) bulk upsert service

* fix specs?

* touch up feed events JS

* end-to-end specs??

* workers wip

* fix failing user delete spec
This commit is contained in:
PJ 2023-09-11 13:32:36 +01:00 committed by GitHub
parent 37d8fc866a
commit 3f7d1fbc1a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
28 changed files with 857 additions and 7 deletions

View file

@ -369,6 +369,9 @@ function insertArticles(articles) {
);
var lastElement = singleArticles[singleArticles.length - 1];
insertAfter(newNode, lastElement);
if (window.observeFeedElements && articles.length > 0) {
window.observeFeedElements();
}
if (nextPage > 0) {
fetching = false;
}

View file

@ -379,9 +379,15 @@ function buildArticleHTML(article, currentUserId = null) {
</a>
`;
let feedContentAttribute = '';
if (article.class_name === 'Article') {
feedContentAttribute = `data-feed-content-id="${article.id}"`;
}
return `<article class="crayons-story"
data-article-path="${article.path}"
id="article-${article.id}"
${feedContentAttribute}
data-content-user-id="${article.user_id}">\
${navigationLink}\
<div role="presentation">\

View file

@ -0,0 +1,26 @@
class FeedEventsController < ApplicationMetalController
include ActionController::Head
FEED_EVENT_ALLOWED_PARAMS = %i[
article_id
article_position
category
context_type
].freeze
def create
if session_current_user_id
FeedEvents::BulkUpsert.call(feed_events_params)
end
head :ok
end
private
def feed_events_params
@feed_events_params ||= params[:feed_events].map do |event|
event.slice(*FEED_EVENT_ALLOWED_PARAMS).merge(user_id: session_current_user_id)
end
end
end

View file

@ -29,6 +29,7 @@ export const Article = ({
return <PodcastArticle article={article} />;
}
const isArticle = article.class_name === 'Article';
const clickableClassList = [
'crayons-story',
'crayons-story__top',
@ -53,6 +54,7 @@ export const Article = ({
isFeatured ? ' crayons-story--featured' : ''
}`}
id={isFeatured ? 'featured-story-marker' : `article-${article.id}`}
data-feed-content-id={isArticle ? article.id : null}
data-content-user-id={article.user_id}
>
<a
@ -110,7 +112,7 @@ export const Article = ({
<ContentTitle article={article} />
<TagList tags={article.tag_list} flare_tag={article.flare_tag} />
{article.class_name === 'Article' && (
{isArticle && (
// eslint-disable-next-line no-underscore-dangle
<SearchSnippet highlightText={article.highlight} />
)}

View file

@ -54,6 +54,7 @@ Object {
<article
class="crayons-story cursor-pointer crayons-story--featured"
data-content-user-id="23289"
data-feed-content-id="62407"
id="featured-story-marker"
>
<a
@ -322,6 +323,7 @@ Object {
<article
class="crayons-story cursor-pointer crayons-story--featured"
data-content-user-id="23289"
data-feed-content-id="62407"
id="featured-story-marker"
>
<a

View file

@ -0,0 +1,181 @@
const MAX_BATCH_SIZE = 20; // Maybe adjust?
const AUTOSEND_PERIOD = 5 * 1000;
const VISIBLE_THRESHOLD = 0.25;
const tracker = {
queue: [],
processInterval: null,
observer: new IntersectionObserver(trackFeedImpressions, {
root: null,
rootMargin: '0px',
threshold: VISIBLE_THRESHOLD,
}),
beaconEnabled: true,
nextFeedPosition: null,
};
window.observeFeedElements = observeFeedElements;
/**
* Sets up the feed events tracker.
* Called every time posts are inserted into the feed.
*/
export function observeFeedElements() {
const feedContainer = document.getElementById('index-container');
// Default container for Preact-rendered home feed
const feedItemsRoot = document.getElementById('rendered-article-feed');
if (!(feedContainer && feedItemsRoot)) return;
const { feedCategoryClick, feedCategoryImpression, feedContextType } =
feedContainer.dataset;
// Reset all relevant state
tracker.categoryClick = feedCategoryClick;
tracker.categoryImpression = feedCategoryImpression;
tracker.contextType = feedContextType;
tracker.processInterval ||= setInterval(submitEventsBatch, AUTOSEND_PERIOD);
tracker.observer.disconnect();
tracker.nextFeedPosition = 1;
findAndTrackFeedItems(feedItemsRoot);
ensureQueueIsClearedBeforeExit();
}
/**
* Given how often it may be called, and the need to assign the correct positions
* in the feed, we take a more efficient approach to finding feed items than
* querying the entire DOM.
* This manual recursion (and a good chunk of `useListNavigation.js` would be
* unnecessary if `initScrolling.js` is updated to *not* create a waterfall of elements.
* @param {HTMLElement} root The (current) element with feed items as children.
*/
function findAndTrackFeedItems(root) {
Array.from(root.children).forEach((/** @type HTMLElement */ element) => {
if (element.classList.contains('paged-stories')) {
// This was inserted by `initScrolling, and will contain feed items within.
findAndTrackFeedItems(element);
} else if (element.dataset?.feedContentId) {
element.dataset.feedPosition = tracker.nextFeedPosition;
element.addEventListener('click', trackFeedClickListener, true);
tracker.observer.observe(element);
tracker.nextFeedPosition += 1;
}
});
}
/**
* Attempts to send any pending queued events before state is lost - e.g. when
* navigating to a different page, or (on mobile) switching to a different app
* (which can eventually cause the browser tab to be discarded in the background
* at the operating system's discretion).
* Both the unload event and the page visibility API are used as each one covers
* some gaps that the other does not.
*/
function ensureQueueIsClearedBeforeExit() {
document.addEventListener('visibilitychange', () => {
if (document.visibilityState == 'hidden') submitEventsBatch();
});
window.addEventListener('beforeunload', submitEventsBatch);
}
/**
* Collects feed impressions, counted as at least a quarter of the article card
* coming into view. This is typically enough to at least see the title and/or
* a significant portion of the cover image.
* @param {IntersectionObserverEntry[]} entries
*/
function trackFeedImpressions(entries) {
entries.forEach((entry) => {
// At least a quarter of the card is in view; not quite enough to read the
// title for many articles, but it'll do
if (entry.isIntersecting && entry.intersectionRatio >= VISIBLE_THRESHOLD) {
queueMicrotask(() => {
const post = entry.target;
if (!post.dataset.impressionRecorded) {
queueEvent(post, tracker.categoryImpression);
post.dataset.impressionRecorded = true;
}
});
}
});
}
/**
* Sends click events to the server immediately along with any currently-batched
* events.
* These may not necessarily be clicks that open the article (e.g. the user may
* have clicked on the author's profile image).
* TODO: Only track link-opening clicks instead?
* @param {MouseEvent} event
*/
function trackFeedClickListener(event) {
const post = event.currentTarget;
if (!post.dataset.clickRecorded) {
queueEvent(post, tracker.categoryClick);
post.dataset.clickRecorded = true;
submitEventsBatch();
}
}
function queueEvent(post, category) {
const { feedContentId, feedPosition } = post.dataset;
tracker.queue.push({
article_id: feedContentId,
article_position: feedPosition,
category,
context_type: tracker.contextType,
});
if (tracker.queue.length >= MAX_BATCH_SIZE) {
submitEventsBatch();
}
}
/**
* Sends a batch of feed events to the server.
* Note: requests made with `navigator.sendBeacon` have greater guarantees to
* actually complete than regular fetch requests with the `keepalive` property
* set. However, the former is a bit tedious to implement and is often
* inadvertently blocked by users (e.g. via extensions like uBlock). So a fallback
* to `fetch` is included to cover that.
*/
function submitEventsBatch() {
if (tracker.queue.length === 0) return;
const tokenMeta = document.querySelector("meta[name='csrf-token']");
const authenticity_token = tokenMeta?.getAttribute('content');
if (tracker.beaconEnabled) {
// The Beacon API doesn't actually let you set headers, so we set the content
// type and CSRF token within the body itself (the browser will recognise the
// former, and Rails will recognise the latter)
const data = new Blob(
[JSON.stringify({ authenticity_token, feed_events: tracker.queue })],
{ type: 'application/json' },
);
// `sendBeacon` returns true if sending a beacon worked, and false otherwise.
tracker.beaconEnabled = navigator.sendBeacon('/feed_events', data);
} else {
fallbackRequest(authenticity_token);
}
tracker.queue = [];
}
function fallbackRequest(authenticity_token) {
window
.fetch('/feed_events', {
method: 'POST',
headers: {
'X-CSRF-Token': authenticity_token,
'Content-Type': 'application/json',
},
body: JSON.stringify({ feed_events: tracker.queue }),
credentials: 'same-origin',
keepalive: true,
})
.catch((error) => console.error(error));
}

View file

@ -5,6 +5,7 @@ import {
observeBillboards,
initializeBillboardVisibility,
} from '../packs/billboardAfterRenderActions';
import { observeFeedElements } from '../packs/feedEvents';
import { setupBillboardDropdown } from '@utilities/billboardDropdown';
import { trackCreateAccountClicks } from '@utilities/ahoy/trackEvents';
@ -101,6 +102,7 @@ if (!document.getElementById('featured-story-marker')) {
initializeBillboardVisibility();
observeBillboards();
setupBillboardDropdown();
observeFeedElements();
};
renderFeed(feedTimeFrame, callback);
@ -123,6 +125,7 @@ if (!document.getElementById('featured-story-marker')) {
initializeBillboardVisibility();
observeBillboards();
setupBillboardDropdown();
observeFeedElements();
};
renderFeed(changedFeedTimeFrame, callback);

View file

@ -1,4 +1,4 @@
import { h, render } from 'preact';
import { h, Fragment, render } from 'preact';
import PropTypes from 'prop-types';
import { Article, LoadingArticle } from '../articles';
import { Feed } from '../articles/Feed';
@ -150,7 +150,7 @@ export const renderFeed = async (timeFrame, afterRender) => {
}
return (
<div>
<Fragment>
{feedConstruct(
pinnedItem,
imageItem,
@ -159,7 +159,7 @@ export const renderFeed = async (timeFrame, afterRender) => {
bookmarkClick,
currentUserId,
)}
</div>
</Fragment>
);
};

View file

@ -125,6 +125,7 @@ class Article < ApplicationRecord
has_many :context_notifications, as: :context, inverse_of: :context, dependent: :delete_all
has_many :context_notifications_published, -> { where(context_notifications_published: { action: "Published" }) },
as: :context, inverse_of: :context, class_name: "ContextNotification"
has_many :feed_events, dependent: :delete_all
has_many :notification_subscriptions, as: :notifiable, inverse_of: :notifiable, dependent: :delete_all
has_many :notifications, as: :notifiable, inverse_of: :notifiable, dependent: :delete_all
has_many :page_views, dependent: :delete_all

31
app/models/feed_event.rb Normal file
View file

@ -0,0 +1,31 @@
class FeedEvent < ApplicationRecord
# These are "optional" mostly so that we can perform validated bulk inserts
# without triggering article/user validation.
# Since there are database-level constraints, it's fine to skip the automatic
# Rails-side association validation (which causes an N+1 query).
belongs_to :article, optional: true
belongs_to :user, optional: true
enum category: {
impression: 0,
click: 1,
reaction: 2,
comment: 3
}
CONTEXT_TYPE_HOME = "home".freeze
CONTEXT_TYPE_SEARCH = "search".freeze
CONTEXT_TYPE_TAG = "tag".freeze
VALID_CONTEXT_TYPES = [
CONTEXT_TYPE_HOME,
CONTEXT_TYPE_SEARCH,
CONTEXT_TYPE_TAG,
].freeze
DEFAULT_TIMEBOX = 5.minutes.freeze
validates :article_position, numericality: { only_integer: true, greater_than: 0 }
validates :context_type, inclusion: { in: VALID_CONTEXT_TYPES }, presence: true
# Since we have disabled association validation, this is handy to filter basic bad data
validates :article_id, presence: true, numericality: { only_integer: true }
validates :user_id, numericality: { only_integer: true }, allow_nil: true
end

View file

@ -73,6 +73,7 @@ class User < ApplicationRecord
has_many :billboard_events, dependent: :nullify
has_many :email_authorizations, dependent: :delete_all
has_many :email_messages, class_name: "Ahoy::Message", dependent: :destroy
has_many :feed_events, dependent: :nullify
has_many :field_test_memberships, class_name: "FieldTest::Membership", as: :participant, dependent: :destroy
# Consider that we might be able to use dependent: :delete_all as the GithubRepo busts the user cache
has_many :github_repos, dependent: :destroy

View file

@ -0,0 +1,119 @@
module FeedEvents
# Inserts a collection of feed events into the database.
#
# If there are duplicate events in the collection (i.e. having the same user,
# article, and category) only one will be inserted.
#
# If a timebox is provided and there is a duplicate event (using the criteria
# for uniqueness above) that was created within it, any matching new event will
# not be created.
#
# This avoids inflating metrics (whether by accident or deliberately).
class BulkUpsert
ATTRIBUTES_FOR_INSERT = %i[article_id user_id article_position category context_type].freeze
def self.call(...)
new(...).call
end
# @param feed_events_data [Array<Hash|FeedEvent>] A list of feed events attributes to upsert
# @param timebox [ActiveSupport::Duration] A time window (in minutes) within which feed events must be unique
def initialize(feed_events_data, timebox: FeedEvent::DEFAULT_TIMEBOX)
@feed_events_data = feed_events_data
@timebox = timebox
end
def call
return if valid_events.blank?
if valid_events.size == 1
create_single_event!
return
end
# It's *possible* to construct a single SQL query that does what we want
# here, (i.e. find existing records, filter out duplicates, and insert the
# resulting set) but it is more complicated than it sounds.
# For instance, using Postgres' upserting feature (`ON CONFLICT DO...`) is
# not an option because we *don't* actually want the records to be unique
# in general, just within the provided timebox, so there is no constraint
# to trigger a conflict.
# Instead we take a Rails-ish `find_or_create_by` approach: first try to
# find matching record(s), then figure out which new record(s) to insert
# application-side. Making two queries does potentially allow duplicates
# (through race conditions), but that's an acceptable margin of error and
# would be the case anyway with a single query (without locking the table)
if timebox.present?
find_existing_events_within_timebox do |event|
track_recent_event(event)
end
end
records_to_insert = valid_events.filter_map do |event|
unless recent_event?(event)
track_recent_event(event)
ATTRIBUTES_FOR_INSERT.index_with { |attr| event[attr] }
end
end
FeedEvent.insert_all(records_to_insert) if records_to_insert.present?
end
private
attr_reader :feed_events_data, :timebox
def valid_events
@valid_events ||= feed_events_data.filter_map do |event_data|
event = FeedEvent.new(event_data)
event if event.valid?
rescue ArgumentError
# Enums raise ArgumentError if assigned with invalid value
end
end
def create_single_event!
event = valid_events.first
FeedEvent
.where.not("created_at > ?", timebox.ago.utc) # Only proceed if
.create_with(event.slice(:article_position, :context_type))
.find_or_create_by(event.slice(:article_id, :user_id, :category))
end
def recent_events
@recent_events ||= Hash.new do |users, user_id|
users[user_id] = Hash.new do |categories, category|
categories[category] = {} # articles
end
end
end
def recent_event?(event)
recent_events[event.user_id][event.category][event.article_id]
end
def track_recent_event(event)
recent_events[event.user_id][event.category][event.article_id] = true
end
def find_existing_events_within_timebox(&block)
values = valid_events.reduce([]) do |acc, event|
acc << event.article_id
# Postgres is...iffy about comparing NULL (NULL !== NULL), and chokes if it is present in a tuple comparison.
# Coalescing to 0 is fine because primary keys auto-increment from 1.
acc << (event.user_id || 0)
acc << FeedEvent.categories[event[:category]]
end
values_clause = Array.new(valid_events.length) { "(?, ?, ?)" }.join(", ")
FeedEvent
.where("created_at > ?", timebox.ago.utc)
.where(
"(article_id, COALESCE(user_id, 0), category) IN (#{values_clause})",
*values,
)
.each(&block)
end
end
end

View file

@ -1,4 +1,4 @@
<div class="crayons-story <% if featured == true %>crayons-story--featured<% end %>" data-content-user-id="<%= story.user_id %>">
<div class="crayons-story <% if featured == true %>crayons-story--featured<% end %>" data-feed-content-id="<%= story.id %>" data-content-user-id="<%= story.user_id %>">
<a href="<%= story.path %>" aria-labelledby="article-link-<%= story.id %>" class="crayons-story__hidden-navigation-link"><%= story.title %></a>
<% if featured == true %>
<div id="featured-story-marker" data-featured-article="articles-<%= story.id %>"></div>

View file

@ -33,6 +33,10 @@
data-params="<%= params.merge(sort_by: "hotness_score", sort_direction: "desc").to_json(only: %i[tag username q sort_by sort_direction]) %>" data-which="<%= @list_of %>"
data-tag=""
data-feed="<%= params[:timeframe] || "base-feed" %>"
data-feed-context-type="<%= FeedEvent::CONTEXT_TYPE_HOME %>"
<% FeedEvent.categories.keys.each do |category| %>
data-feed-category-<%= category %>="<%= category %>"
<% end %>
data-articles-since="<%= Timeframe.datetime_iso8601(params[:timeframe]) %>">
<%= render "articles/sidebar" %>
@ -104,5 +108,5 @@
<%= render "articles/sidebar_additional" %>
</div>
<%= javascript_packs_with_chunks_tag "storiesList", "followButtons", "feedPreviewCards", "heroBannerClose", "localizeArticleDates", defer: true %>
<%= javascript_packs_with_chunks_tag "storiesList", "followButtons", "feedPreviewCards", "heroBannerClose", "localizeArticleDates", "feedEvents", defer: true %>
<% end %>

View file

@ -142,6 +142,7 @@ Rails.application.routes.draw do
resources :tag_adjustments, only: %i[create destroy]
resources :rating_votes, only: [:create]
resources :page_views, only: %i[create update]
resources :feed_events, only: %i[create]
resources :credits, only: %i[index new create] do
get "purchase", on: :collection, to: "credits#new"
end

View file

@ -0,0 +1,126 @@
describe('Home page feed events', () => {
beforeEach(() => {
cy.testSetup();
});
const scrollBackAndForth = () => {
// The delay is necessary for Cypress to give `IntersectionObserver` time to actually work.
// Also, we scroll twice so we can confirm we don't send extraneous events.
cy.scrollTo('bottom', { duration: 700 });
cy.scrollTo('top', { duration: 700 });
};
context('when a user is not logged in', () => {
beforeEach(() => {
const spy = cy.spy().as('feedEventsSpy');
cy.intercept('/feed_events', spy);
cy.clock(new Date(), ['setInterval']);
cy.visit('/');
cy.tick(500); // Allow regular page load timers to run
});
it('does not send feed events', () => {
scrollBackAndForth();
// Again for good measure
scrollBackAndForth();
// More than autosubmit threshold
cy.tick(7000);
cy.get('@feedEventsSpy').should('not.have.been.called');
});
});
context('when a user is logged in', () => {
beforeEach(() => {
cy.fixture('users/articleEditorV1User.json').as('user');
cy.intercept('/feed_events').as('feedEventsSubmission');
cy.get('@user').then((user) => cy.loginUser(user));
});
const visitAndWaitForLoad = (url, { expectedPostCount }) => {
cy.visit(url);
cy.get('[data-feed-position]').should('have.length', expectedPostCount);
};
const findAndDetermineImpressions = (callback) => {
cy.get('[data-feed-content-id]').then((posts) => {
const ids = posts
.map(function () {
return this.dataset.feedContentId;
})
.get();
const expectedEvents = ids.map((id, index) => ({
article_id: `${id}`,
article_position: `${index + 1}`,
category: 'impression',
context_type: 'home',
}));
callback(expectedEvents);
});
};
it('submits click events immediately', () => {
visitAndWaitForLoad('/', { expectedPostCount: 11 });
// The first/featured article is `test-article-slug`
cy.get('#featured-story-marker').within((post) => {
const id = post.data('feedContentId');
cy.findByRole('heading').click();
// Less than auto-submit threshold of 5 seconds.
cy.wait('@feedEventsSubmission', { timeout: 1000 })
.its('request.body.feed_events')
.should('deep.contain', {
// For some reason Cypress leaves the integers in the request body as strings.
article_id: `${id}`,
article_position: '1',
category: 'click',
context_type: 'home',
});
});
});
it('auto-submits incomplete batches of events after a few seconds', () => {
visitAndWaitForLoad('/', { expectedPostCount: 11 });
// There are 11 articles currently in the seeder! If this line is failing
// and a seeded article has recently been added or removed, just bump
// this number.
findAndDetermineImpressions((events) => {
scrollBackAndForth();
cy.wait('@feedEventsSubmission')
.its('request.body.feed_events')
.should('have.deep.members', events);
});
});
it('submits impression events in batches of 20', () => {
const createArticles = [...Array(15)].map((_, index) =>
cy.createArticle({
title: `Feed post ${index}`,
content: `Content for feed post ${index}`,
}),
);
cy.all(createArticles).then(() => {
visitAndWaitForLoad('/', { expectedPostCount: 26 });
findAndDetermineImpressions((events) => {
const firstBatch = events.slice(0, 20);
const secondBatch = events.slice(20);
scrollBackAndForth();
cy.wait('@feedEventsSubmission')
.its('request.body.feed_events')
.should('have.deep.members', firstBatch);
cy.wait('@feedEventsSubmission')
.its('request.body.feed_events')
.should('have.deep.members', secondBatch);
});
});
});
});
});

View file

@ -26,6 +26,26 @@ import { getInterceptsForLingeringUserRequests } from '../util/networkUtils';
// -- This will overwrite an existing command --
// Cypress.Commands.overwrite("visit", (originalFn, url, options) => { ... })
/**
* Use this function to wait for multiple async/then-able commands to complete.
* Useful for making multiple requests.
*/
Cypress.Commands.add(
'all',
/**
* @param {Cypress.Chainable<undefined>[]} commands Functions that return
* a thenable
*/
(commands) =>
commands.reduce(
(allResults, command) =>
allResults.then((results) =>
command.then((result) => [...results, result]),
),
cy.wrap([]),
),
);
/**
* Use this function to sign a user out without lingering network calls causing unintended side-effects.
*/

View file

@ -0,0 +1,15 @@
class CreateFeedEvents < ActiveRecord::Migration[7.0]
def change
create_table :feed_events do |t|
t.integer :article_position
t.integer :category, null: false
t.string :context_type, null: false
t.integer :counts_for, null: false, default: 1
t.references :article, null: false, foreign_key: { on_delete: :cascade }
t.references :user, foreign_key: { on_delete: :nullify }
t.timestamps
end
end
end

View file

@ -0,0 +1,12 @@
class AddCreatedAtAndCompositeIndicesToFeedEvents < ActiveRecord::Migration[7.0]
disable_ddl_transaction!
def change
add_index :feed_events, :created_at, algorithm: :concurrently
add_index :feed_events,
%i[article_id user_id category],
name: "index_feed_events_on_article_user_and_category",
algorithm: :concurrently
end
end

View file

@ -0,0 +1,8 @@
class AddFeedCountersToArticle < ActiveRecord::Migration[7.0]
def change
safety_assured do
add_column :articles, :feed_impressions_count, :integer, default: 0
add_column :articles, :feed_clicks_count, :integer, default: 0
end
end
end

View file

@ -104,6 +104,8 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_31_160030) do
t.float "experience_level_rating", default: 5.0
t.float "experience_level_rating_distribution", default: 5.0
t.boolean "featured", default: false
t.integer "feed_clicks_count", default: 0
t.integer "feed_impressions_count", default: 0
t.string "feed_source_url"
t.integer "hotness_score", default: 0
t.string "language"
@ -501,6 +503,21 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_31_160030) do
t.index ["user_id"], name: "index_email_authorizations_on_user_id"
end
create_table "feed_events", force: :cascade do |t|
t.bigint "article_id", null: false
t.integer "article_position"
t.integer "category", null: false
t.string "context_type", null: false
t.integer "counts_for", default: 1, null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.bigint "user_id"
t.index ["article_id", "user_id", "category"], name: "index_feed_events_on_article_user_and_category"
t.index ["article_id"], name: "index_feed_events_on_article_id"
t.index ["created_at"], name: "index_feed_events_on_created_at"
t.index ["user_id"], name: "index_feed_events_on_user_id"
end
create_table "feedback_messages", force: :cascade do |t|
t.bigint "affected_id"
t.string "category"
@ -1391,6 +1408,8 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_31_160030) do
add_foreign_key "display_ad_events", "users", on_delete: :cascade
add_foreign_key "display_ads", "organizations", on_delete: :cascade
add_foreign_key "email_authorizations", "users", on_delete: :cascade
add_foreign_key "feed_events", "articles", on_delete: :cascade
add_foreign_key "feed_events", "users", on_delete: :nullify
add_foreign_key "feedback_messages", "users", column: "affected_id", on_delete: :nullify
add_foreign_key "feedback_messages", "users", column: "offender_id", on_delete: :nullify
add_foreign_key "feedback_messages", "users", column: "reporter_id", on_delete: :nullify

View file

@ -0,0 +1,9 @@
FactoryBot.define do
factory :feed_event do
sequence(:article_position)
category { :impression }
context_type { FeedEvent::CONTEXT_TYPE_HOME }
article
user
end
end

View file

@ -25,6 +25,7 @@ RSpec.describe Article do
it { is_expected.to have_many(:comments).dependent(:nullify) }
it { is_expected.to have_many(:context_notifications).dependent(:delete_all) }
it { is_expected.to have_many(:feed_events).dependent(:delete_all) }
it { is_expected.to have_many(:mentions).dependent(:delete_all) }
it { is_expected.to have_many(:notification_subscriptions).dependent(:delete_all) }
it { is_expected.to have_many(:notifications).dependent(:delete_all) }
@ -1420,6 +1421,7 @@ RSpec.describe Article do
end
end
end
describe "#detect_language" do
let(:detected_language) { :kl } # kl for Klingon

View file

@ -0,0 +1,14 @@
require "rails_helper"
RSpec.describe FeedEvent do
describe "validations" do
it { is_expected.to belong_to(:article).optional }
it { is_expected.to validate_numericality_of(:article_id).only_integer }
it { is_expected.to belong_to(:user).optional }
it { is_expected.to validate_numericality_of(:user_id).only_integer.allow_nil }
it { is_expected.to define_enum_for(:category).with_values(%i[impression click reaction comment]) }
it { is_expected.to validate_numericality_of(:article_position).is_greater_than(0).only_integer }
it { is_expected.to validate_inclusion_of(:context_type).in_array(%w[home search tag]) }
end
end

View file

@ -82,6 +82,7 @@ RSpec.describe User do
it { is_expected.to have_many(:billboard_events).dependent(:nullify) }
it { is_expected.to have_many(:email_authorizations).dependent(:delete_all) }
it { is_expected.to have_many(:email_messages).class_name("Ahoy::Message").dependent(:destroy) }
it { is_expected.to have_many(:feed_events).dependent(:nullify) }
it { is_expected.to have_many(:field_test_memberships).class_name("FieldTest::Membership").dependent(:destroy) }
it { is_expected.to have_many(:github_repos).dependent(:destroy) }
it { is_expected.to have_many(:html_variants).dependent(:destroy) }

View file

@ -0,0 +1,81 @@
require "rails_helper"
RSpec.describe "FeedEvents" do
let(:user) { create(:user) }
let(:article) { create(:article) }
let(:event_params) do
{
article_id: article.id,
article_position: 4,
category: :click,
context_type: FeedEvent::CONTEXT_TYPE_HOME
}
end
describe "POST /feed_events" do
context "when user is signed in" do
before do
sign_in user
end
it "creates a feed click event" do
expect { post "/feed_events", params: { feed_events: [event_params] } }.to change(FeedEvent, :count).by(1)
expect(response).to be_successful
expect(article.feed_events.first).to have_attributes(
user_id: user.id,
article_position: 4,
category: "click",
context_type: "home",
)
end
it "creates a feed impression event" do
params = event_params.merge(category: :impression)
expect { post "/feed_events", params: { feed_events: [params] } }.to change(FeedEvent, :count).by(1)
expect(response).to be_successful
expect(article.feed_events.first.category).to eq("impression")
end
it "creates multiple events in a batch" do
second_article = create(:article)
events = [
{
article_id: article.id,
article_position: 20,
category: :impression,
context_type: FeedEvent::CONTEXT_TYPE_HOME
},
{
article_id: article.id,
article_position: 20,
category: :click,
context_type: FeedEvent::CONTEXT_TYPE_HOME
},
{
article_id: second_article.id,
article_position: 7,
category: :impression,
context_type: FeedEvent::CONTEXT_TYPE_TAG
},
{
article_id: second_article.id,
article_position: 7,
category: :click,
context_type: FeedEvent::CONTEXT_TYPE_TAG
},
]
expect { post "/feed_events", params: { feed_events: events } }.to change(FeedEvent, :count).by(4)
expect(response).to be_successful
end
end
context "when user is not signed in" do
it "silently does not create an event" do
expect { post "/feed_events", params: { feed_events: [event_params] } }.not_to change(FeedEvent, :count)
expect(response).to be_successful
end
end
end
end

View file

@ -0,0 +1,162 @@
require "rails_helper"
RSpec.describe FeedEvents::BulkUpsert, type: :service do
let(:user) { create(:user) }
let(:second_user) { create(:user) }
def feed_event(**attributes)
build(:feed_event, **attributes).attributes
end
context "when there are no duplicates in the list or database" do
let(:articles) { create_list(:article, 5) }
let(:feed_events_data) do
articles
.map
.with_index do |article, index|
{
article: article,
user: user,
article_position: index + 1,
category: :impression,
context_type: FeedEvent::CONTEXT_TYPE_HOME
}
end
end
it "inserts all the feed events" do
expect { described_class.call(feed_events_data) }.to change(FeedEvent, :count).by(5)
expect(FeedEvent.pluck(:article_id, :user_id, :category)).to match_array(
articles.map { |article| [article.id, user.id, "impression"] },
)
end
end
context "when there are duplicate events within the list" do
let(:article) { create(:article) }
let(:feed_events_data) do
Array.new(5) do
feed_event(article: article, user: user, category: :click)
end
end
it "does not insert extra duplicate events" do
expect { described_class.call(feed_events_data) }.to change(FeedEvent, :count).by(1)
expect(FeedEvent.pluck(:article_id, :user_id, :category)).to contain_exactly(
[article.id, user.id, "click"],
)
end
end
context "when there are invalid events in the list" do
let(:article) { create(:article) }
let(:feed_events_data) do
[
feed_event(article: nil, user: user, category: :click),
feed_event(article: article, user: user).merge(category: :not_a_real_category),
feed_event(article: article, user: user, category: :impression),
feed_event(article: article, user: nil, category: :impression),
feed_event(article: article, user: second_user, category: :impression, context_type: "blahblahblah"),
feed_event(article: article, user: second_user, category: :click, article_position: -2),
]
end
it "filters them out" do
expect { described_class.call(feed_events_data, timebox: nil) }.to change(FeedEvent, :count).by(2)
expect(FeedEvent.pluck(:article_id, :user_id, :category)).to contain_exactly(
[article.id, user.id, "impression"],
[article.id, nil, "impression"],
)
end
end
context "when there is only one valid item in the list" do
let(:article) { create(:article) }
let(:feed_event_data) do
[
feed_event(article: article, user: user, category: :click, context_type: "foobar"),
feed_event(article: article, user: second_user, category: :impression),
]
end
it "handles it appropriately" do
expect { described_class.call(feed_event_data) }.to change(FeedEvent, :count).by(1)
expect(FeedEvent.pluck(:article_id, :user_id, :category)).to contain_exactly(
[article.id, second_user.id, "impression"],
)
end
end
context "when there are no valid items in the list" do
let(:feed_event_data) do
[
feed_event(article: nil, user: user, category: :click),
feed_event(article: nil, user: second_user, category: :click),
]
end
it "does nothing and returns" do
allow(FeedEvent).to receive(:where).and_call_original
expect { described_class.call(feed_event_data) }.not_to change(FeedEvent, :count)
expect(FeedEvent).not_to have_received(:where)
end
end
context "when there are already existing events with the same article, user and category" do
let(:article) { create(:article) }
let(:second_article) { create(:article) }
let(:feed_events_data) do
[
feed_event(article: article, user: user, category: :impression),
feed_event(article: article, user: user, category: :click),
feed_event(article: article, user: second_user, category: :impression),
feed_event(article: second_article, user: user, category: :impression),
]
end
before do
create(:feed_event, article: article, user: user, category: :impression)
create(:feed_event, article: article, user: second_user, category: :impression)
end
it "ignores them if no timebox is provided" do
expect { described_class.call(feed_events_data, timebox: nil) }.to change(FeedEvent, :count).by(4)
expect(FeedEvent.pluck(:article_id, :user_id, :category)).to contain_exactly(
[article.id, user.id, "impression"],
[article.id, second_user.id, "impression"],
[article.id, user.id, "impression"],
[article.id, user.id, "click"],
[article.id, second_user.id, "impression"],
[second_article.id, user.id, "impression"],
)
end
it "does not create new events if the existing matching events were created within the provided timebox" do
Timecop.travel(7.minutes.from_now) do
expect { described_class.call(feed_events_data, timebox: 10.minutes) }.to change(FeedEvent, :count).by(2)
expect(FeedEvent.pluck(:article_id, :user_id, :category)).to contain_exactly(
[article.id, user.id, "impression"],
[article.id, second_user.id, "impression"],
[article.id, user.id, "click"],
[second_article.id, user.id, "impression"],
)
end
end
it "creates new events if the existing matching events were created outside the provided timebox" do
Timecop.travel(12.minutes.from_now) do
expect { described_class.call(feed_events_data, timebox: 10.minutes) }.to change(FeedEvent, :count).by(4)
expect(FeedEvent.pluck(:article_id, :user_id, :category)).to contain_exactly(
[article.id, user.id, "impression"],
[article.id, second_user.id, "impression"],
[article.id, user.id, "impression"],
[article.id, user.id, "click"],
[article.id, second_user.id, "impression"],
[second_article.id, user.id, "impression"],
)
end
end
end
end

View file

@ -84,8 +84,9 @@ RSpec.describe Users::Delete, type: :service do
affected_feedback_messages
audit_logs
banished_users
created_podcasts
billboard_events
created_podcasts
feed_events
offender_feedback_messages
page_views
rating_votes