diff --git a/app/javascript/packs/feedEvents.js b/app/javascript/packs/feedEvents.js index 59829b572..d8e549175 100644 --- a/app/javascript/packs/feedEvents.js +++ b/app/javascript/packs/feedEvents.js @@ -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; diff --git a/app/models/feed_event.rb b/app/models/feed_event.rb index d95e647c9..1de9ebe5e 100644 --- a/app/models/feed_event.rb +++ b/app/models/feed_event.rb @@ -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 diff --git a/app/services/reaction_handler.rb b/app/services/reaction_handler.rb index d7445c46f..79696c1c8 100644 --- a/app/services/reaction_handler.rb +++ b/app/services/reaction_handler.rb @@ -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 diff --git a/cypress/e2e/seededFlows/homeFeedFlows/events.spec.js b/cypress/e2e/seededFlows/homeFeedFlows/events.spec.js index 1aff243e7..40705ddcd 100644 --- a/cypress/e2e/seededFlows/homeFeedFlows/events.spec.js +++ b/cypress/e2e/seededFlows/homeFeedFlows/events.spec.js @@ -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') diff --git a/spec/models/feed_event_spec.rb b/spec/models/feed_event_spec.rb index 713e97437..66f1f7008 100644 --- a/spec/models/feed_event_spec.rb +++ b/spec/models/feed_event_spec.rb @@ -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 diff --git a/spec/services/reaction_handler_spec.rb b/spec/services/reaction_handler_spec.rb index 782f4a5de..dcd2af1b5 100644 --- a/spec/services/reaction_handler_spec.rb +++ b/spec/services/reaction_handler_spec.rb @@ -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