Record feed events for public reactions (#20079)

* create feed event on feed reaction

* move feed event creation to feed event model

* tiny whoops

* fix failing specs

* slightly less flaky e2e spec (maybe)

* tiny fixes (couldn't resist)
This commit is contained in:
PJ 2023-09-12 13:36:10 +01:00 committed by GitHub
parent bf4c34636e
commit 95ca9d37c5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 178 additions and 33 deletions

View file

@ -56,7 +56,8 @@ function findAndTrackFeedItems(root) {
findAndTrackFeedItems(element);
} else if (element.dataset?.feedContentId) {
element.dataset.feedPosition = tracker.nextFeedPosition;
element.addEventListener('click', trackFeedClickListener, true);
// Also captures right-click opens
element.addEventListener('mousedown', trackFeedClickListener, true);
tracker.observer.observe(element);
tracker.nextFeedPosition += 1;

View file

@ -28,4 +28,18 @@ class FeedEvent < ApplicationRecord
# 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
def self.record_journey_for(user, article:, category:)
return unless %i[reaction comment].include?(category)
last_click = where(user: user, category: :click).last
return unless last_click&.article_id == article.id
create_with(last_click.slice(:article_position, :context_type))
.find_or_create_by(
category: category,
user: user,
article: article,
)
end
end

View file

@ -89,6 +89,7 @@ class ReactionHandler
rate_limit_reaction_creation
sink_articles(reaction)
send_notifications(reaction)
record_feed_event(reaction)
update_last_reacted_at(reaction)
end
@ -159,6 +160,13 @@ class ReactionHandler
Notification.send_reaction_notification_without_delay(reaction, reaction.reactable.organization)
end
def record_feed_event(reaction)
return unless (reaction.visible_to_public? || reaction.category == "readinglist") &&
reaction.reactable_type == "Article"
FeedEvent.record_journey_for(reaction.user, article: reaction.reactable, category: :reaction)
end
def rate_article(reaction)
user_experience_level = current_user.setting.experience_level
return unless user_experience_level

View file

@ -3,11 +3,12 @@ describe('Home page feed events', () => {
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 });
// Cypress full-page scrolling is so janky that it sometimes does not trigger
// IntersectionObserver, so we are doing a bit of a manual scroll
const scrollAll = (posts) => {
posts.each((_, post) => {
cy.wrap(post).scrollIntoView({ duration: 50 });
});
};
context('when a user is not logged in', () => {
@ -21,9 +22,9 @@ describe('Home page feed events', () => {
});
it('does not send feed events', () => {
scrollBackAndForth();
// Again for good measure
scrollBackAndForth();
cy.scrollTo('bottom', { duration: 600 });
cy.scrollTo('top', { duration: 600 });
cy.scrollTo('bottom', { duration: 600 });
// More than autosubmit threshold
cy.tick(7000);
@ -40,25 +41,27 @@ describe('Home page feed events', () => {
const visitAndWaitForLoad = (url, { expectedPostCount }) => {
cy.visit(url);
cy.get('[data-feed-position]').should('have.length', expectedPostCount);
return cy
.get('[data-feed-position]')
.should('have.length', expectedPostCount)
.then((posts) => {
posts.ea;
});
};
const findAndDetermineImpressions = (callback) => {
cy.get('[data-feed-content-id]').then((posts) => {
const ids = posts
.map(function () {
return this.dataset.feedContentId;
})
.get();
const determineImpressions = (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);
});
return ids.map((id, index) => ({
article_id: `${id}`,
article_position: `${index + 1}`,
category: 'impression',
context_type: 'home',
}));
};
it('submits click events immediately', () => {
@ -83,14 +86,13 @@ describe('Home page feed events', () => {
});
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();
visitAndWaitForLoad('/', { expectedPostCount: 11 }).then((posts) => {
const events = determineImpressions(posts);
scrollAll(posts);
cy.wait('@feedEventsSubmission')
.its('request.body.feed_events')
.should('have.deep.members', events);
@ -105,12 +107,12 @@ describe('Home page feed events', () => {
}),
);
cy.all(createArticles).then(() => {
visitAndWaitForLoad('/', { expectedPostCount: 26 });
visitAndWaitForLoad('/', { expectedPostCount: 26 }).then((posts) => {
const events = determineImpressions(posts);
findAndDetermineImpressions((events) => {
const firstBatch = events.slice(0, 20);
const secondBatch = events.slice(20);
scrollBackAndForth();
scrollAll(posts);
cy.wait('@feedEventsSubmission')
.its('request.body.feed_events')

View file

@ -11,4 +11,56 @@ RSpec.describe FeedEvent do
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
describe ".record_journey_for" do
subject(:record_journey) { described_class.record_journey_for(user, article: article, category: category) }
let(:article) { create(:article) }
let(:user) { create(:user) }
let(:category) { :reaction }
it "records a feed event if the user's last click was on the specified article" do
click = create(:feed_event, user: user, article: article, category: :click)
expect { record_journey }.to change(described_class, :count).by(1)
expect(user.feed_events.last).to have_attributes(
category: "reaction",
article_id: article.id,
user_id: user.id,
context_type: click.context_type,
article_position: click.article_position,
)
end
it "does not record a feed event if the user has no feed events for the specified article" do
expect { record_journey }.not_to change(described_class, :count)
expect(user.feed_events).to be_empty
end
it "does not record a feed event if the user did not click through from the feed" do
impression = create(:feed_event, user: user, article: article, category: :impression)
expect { record_journey }.not_to change(described_class, :count)
expect(user.feed_events).to contain_exactly(impression)
end
it "does not record a feed event if the user's last click was not on the specified article" do
click = create(:feed_event, user: user, article: article, category: :click)
other_click = create(:feed_event, user: user, category: :click)
expect { record_journey }.not_to change(described_class, :count)
expect(user.feed_events).to contain_exactly(click, other_click)
end
context "when the interaction is not a comment or reaction" do
let(:category) { :impression }
it "does not record a feed event" do
click = create(:feed_event, user: user, article: article, category: :click)
expect { record_journey }.not_to change(described_class, :count)
expect(user.feed_events).to contain_exactly(click)
end
end
end
end

View file

@ -53,6 +53,25 @@ RSpec.describe ReactionHandler, type: :service do
expect(result).to be_success
end
end
it "records a feed event for articles reached through a feed" do
create(:feed_event, category: :click, article: article, user: user)
expect { result }.to change(FeedEvent, :count).by(1)
expect(user.feed_events.last).to have_attributes(
category: "reaction",
article_id: article.id,
user_id: user.id,
)
end
it "does not record a feed event for articles that were not reached through a feed" do
# activity by a different user!
create(:feed_event, category: :click, article: article, user: moderator)
expect { result }.not_to change(FeedEvent, :count)
expect(user.feed_events).to be_empty
end
end
context "when the article is written for an organization" do
@ -70,6 +89,18 @@ RSpec.describe ReactionHandler, type: :service do
end
end
context "when the reaction is not a public reaction or bookmark" do
let(:user) { moderator }
let(:category) { "vomit" }
it "does not record a feed event" do
click = create(:feed_event, category: :click, article: article, user: moderator)
expect { result }.not_to change(FeedEvent, :count)
expect(moderator.feed_events).to contain_exactly(click)
end
end
context "when there's an existing/matching reaction by user" do
let!(:existing) { article.reactions.create! user: user, category: "like" }
@ -90,7 +121,7 @@ RSpec.describe ReactionHandler, type: :service do
end
end
context "when the reaction is not in a notifiable category" do
context "when the reaction is a bookmark" do
let(:category) { "readinglist" }
it "does not send a notification to the author" do
@ -99,6 +130,17 @@ RSpec.describe ReactionHandler, type: :service do
expect(result.action).to eq("create")
end
end
it "records a feed event if reached through a feed" do
create(:feed_event, category: :click, article: article, user: user)
expect { result }.to change(FeedEvent, :count).by(1)
expect(user.feed_events.last).to have_attributes(
category: "reaction",
article_id: article.id,
user_id: user.id,
)
end
end
context "when there's an existing, contradictory mod reaction" do
@ -140,6 +182,13 @@ RSpec.describe ReactionHandler, type: :service do
expect(result.action).to eq("create")
expect(Reaction.ids).not_to include(contradictory_mod.id)
end
it "does not record a feed event" do
click = create(:feed_event, category: :click, article: comment.commentable, user: moderator)
expect { result }.not_to change(FeedEvent, :count)
expect(moderator.feed_events).to contain_exactly(click)
end
end
end
@ -163,6 +212,25 @@ RSpec.describe ReactionHandler, type: :service do
expect(result).to be_success
end
end
it "records a feed event for articles reached through a feed" do
create(:feed_event, category: :click, article: article, user: user)
expect { result }.to change(FeedEvent, :count).by(1)
expect(user.feed_events.last).to have_attributes(
category: "reaction",
article_id: article.id,
user_id: user.id,
)
end
it "does not record a feed event for articles that were not reached through a feed" do
# activity by a different user!
create(:feed_event, category: :click, article: article, user: moderator)
expect { result }.not_to change(FeedEvent, :count)
expect(user.feed_events).to be_empty
end
end
context "when the reaction is not in a notifiable category" do