From b8041f0a6017e40768015c39bf81c79c6369d730 Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Wed, 21 Sep 2022 10:25:51 +0200 Subject: [PATCH] Added clicks to activity feed (#15439) closes https://github.com/TryGhost/Team/issues/1933 - Added click_events to activity feed - Added support for parsing click_events in the frontend - Moved url parsing (transform ready) to model layer of LinkRedirect - Moved `getEventTimeline` method to the top of the event repository - Added description field to parsed events in the frontend (because we need a second line) - Fixed: member email not returned in comment_event --- .../app/components/member/activity-feed.hbs | 5 + .../components/members-activity/table-row.hbs | 5 + ghost/admin/app/helpers/parse-member-event.js | 39 ++++++ .../output/mappers/activity-feed-events.js | 51 +++++++ .../serializers/output/mappers/comments.js | 11 +- .../core/core/server/models/link-redirect.js | 19 +++ .../LinkRedirectRepository.js | 8 +- .../server/services/link-redirection/index.js | 3 +- .../core/core/server/services/members/api.js | 1 + .../admin/__snapshots__/members.test.js.snap | 2 +- ghost/members-api/lib/MembersAPI.js | 2 + ghost/members-api/lib/repositories/event.js | 124 +++++++++++------- 12 files changed, 216 insertions(+), 54 deletions(-) diff --git a/ghost/admin/app/components/member/activity-feed.hbs b/ghost/admin/app/components/member/activity-feed.hbs index 34d206fc97..18039af7ee 100644 --- a/ghost/admin/app/components/member/activity-feed.hbs +++ b/ghost/admin/app/components/member/activity-feed.hbs @@ -37,6 +37,11 @@ {{/if}} + {{#if event.description}} +
+ {{event.description}} +
+ {{/if}}
diff --git a/ghost/admin/app/components/members-activity/table-row.hbs b/ghost/admin/app/components/members-activity/table-row.hbs index 41221a01cf..70a99f9514 100644 --- a/ghost/admin/app/components/members-activity/table-row.hbs +++ b/ghost/admin/app/components/members-activity/table-row.hbs @@ -32,6 +32,11 @@ {{/if}} + {{#if event.description}} +
+ {{event.description}} +
+ {{/if}}
diff --git a/ghost/admin/app/helpers/parse-member-event.js b/ghost/admin/app/helpers/parse-member-event.js index 949655ccca..7b85cb1559 100644 --- a/ghost/admin/app/helpers/parse-member-event.js +++ b/ghost/admin/app/helpers/parse-member-event.js @@ -11,6 +11,7 @@ export default class ParseMemberEventHelper extends Helper { const icon = this.getIcon(event); const action = this.getAction(event, hasMultipleNewsletters); const info = this.getInfo(event); + const description = this.getDescription(event); const join = this.getJoin(event); const object = this.getObject(event); @@ -28,6 +29,7 @@ export default class ParseMemberEventHelper extends Helper { join, object, info, + description, url, timestamp }; @@ -81,6 +83,10 @@ export default class ParseMemberEventHelper extends Helper { icon = 'comment'; } + if (event.type === 'click_event') { + icon = 'click'; + } + return 'event-' + icon + (this.feature.get('memberAttribution') ? '--feature-attribution' : ''); } @@ -148,6 +154,10 @@ export default class ParseMemberEventHelper extends Helper { } return 'commented'; } + + if (event.type === 'click_event') { + return 'clicked email'; + } } /** @@ -191,6 +201,12 @@ export default class ParseMemberEventHelper extends Helper { } } + if (event.type === 'click_event') { + if (event.data.post) { + return event.data.post.title; + } + } + return ''; } @@ -207,6 +223,23 @@ export default class ParseMemberEventHelper extends Helper { return; } + getDescription(event) { + if (event.type === 'click_event') { + // Clean URL + try { + const parsedURL = new URL(event.data.link.to); + + // Remove protocol, querystring and hash + // + strip trailing / + return 'URL: ' + parsedURL.host + (parsedURL.pathname === '/' ? '' : parsedURL.pathname); + } catch (e) { + // Invalid URL + } + return 'URL: ' + event.data.link.to; + } + return; + } + /** * Make the object clickable */ @@ -217,6 +250,12 @@ export default class ParseMemberEventHelper extends Helper { } } + if (event.type === 'click_event') { + if (event.data.post) { + return event.data.post.url; + } + } + if (event.type === 'signup_event' || event.type === 'subscription_event') { if (event.data.attribution && event.data.attribution.url) { return event.data.attribution.url; diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/activity-feed-events.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/activity-feed-events.js index 668a873526..051c63e3f8 100644 --- a/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/activity-feed-events.js +++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/activity-feed-events.js @@ -1,4 +1,6 @@ const mapComment = require('./comments'); +const url = require('../utils/url'); +const _ = require('lodash'); const commentEventMapper = (json, frame) => { return { @@ -7,10 +9,59 @@ const commentEventMapper = (json, frame) => { }; }; +const clickEventMapper = (json, frame) => { + const memberFields = [ + 'id', + 'uuid', + 'name', + 'email', + 'avatar_image' + ]; + + const linkFields = [ + 'from', + 'to' + ]; + + const postFields = [ + 'id', + 'uuid', + 'title', + 'url' + ]; + + const data = json.data; + const response = {}; + + if (data.link && data.link.post) { + // We could use the post mapper here, but we need less field + don't need al the async behavior support + url.forPost(data.link.post.id, data.link.post, frame); + response.post = _.pick(data.link.post, postFields); + } + + if (data.link) { + response.link = _.pick(data.link, linkFields); + } + + if (data.member) { + response.member = _.pick(data.member, memberFields); + } else { + response.member = null; + } + + return { + ...json, + data: response + }; +}; + const activityFeedMapper = (event, frame) => { if (event.type === 'comment_event') { return commentEventMapper(event, frame); } + if (event.type === 'click_event') { + return clickEventMapper(event, frame); + } return event; }; diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js index 8e73ac2704..d67a3dcbbe 100644 --- a/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js +++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js @@ -18,6 +18,15 @@ const memberFields = [ 'avatar_image' ]; +const memberFieldsAdmin = [ + 'id', + 'uuid', + 'name', + 'email', + 'expertise', + 'avatar_image' +]; + const postFields = [ 'id', 'uuid', @@ -36,7 +45,7 @@ const commentMapper = (model, frame) => { const response = _.pick(jsonModel, commentFields); if (jsonModel.member) { - response.member = _.pick(jsonModel.member, memberFields); + response.member = _.pick(jsonModel.member, utils.isMembersAPI(frame) ? memberFields : memberFieldsAdmin); } else { response.member = null; } diff --git a/ghost/core/core/server/models/link-redirect.js b/ghost/core/core/server/models/link-redirect.js index 3b798bb6e8..eef3cd97bd 100644 --- a/ghost/core/core/server/models/link-redirect.js +++ b/ghost/core/core/server/models/link-redirect.js @@ -1,10 +1,29 @@ const ghostBookshelf = require('./base'); +const urlUtils = require('../../shared/url-utils'); const LinkRedirect = ghostBookshelf.Model.extend({ tableName: 'link_redirects', post() { return this.belongsTo('Post', 'post_id'); + }, + + formatOnWrite(attrs) { + if (attrs.to) { + attrs.to = urlUtils.absoluteToTransformReady(attrs.to); + } + + return attrs; + }, + + parse() { + const attrs = ghostBookshelf.Model.prototype.parse.apply(this, arguments); + + if (attrs.to) { + attrs.to = urlUtils.transformReadyToAbsolute(attrs.to); + } + + return attrs; } }, { }); diff --git a/ghost/core/core/server/services/link-redirection/LinkRedirectRepository.js b/ghost/core/core/server/services/link-redirection/LinkRedirectRepository.js index 89d65a5b13..10287fa095 100644 --- a/ghost/core/core/server/services/link-redirection/LinkRedirectRepository.js +++ b/ghost/core/core/server/services/link-redirection/LinkRedirectRepository.js @@ -4,17 +4,13 @@ const ObjectID = require('bson-objectid').default; module.exports = class LinkRedirectRepository { /** @type {Object} */ #LinkRedirect; - /** @type {Object} */ - #urlUtils; /** * @param {object} deps * @param {object} deps.LinkRedirect Bookshelf Model - * @param {object} deps.urlUtils */ constructor(deps) { this.#LinkRedirect = deps.LinkRedirect; - this.#urlUtils = deps.urlUtils; } /** @@ -25,7 +21,7 @@ module.exports = class LinkRedirectRepository { const model = await this.#LinkRedirect.add({ // Only store the parthname (no support for variable query strings) from: linkRedirect.from.pathname, - to: this.#urlUtils.absoluteToTransformReady(linkRedirect.to.href) + to: linkRedirect.to.href }, {}); linkRedirect.link_id = ObjectID.createFromHexString(model.id); @@ -47,7 +43,7 @@ module.exports = class LinkRedirectRepository { return new LinkRedirect({ id: linkRedirect.id, from: url, - to: new URL(this.#urlUtils.transformReadyToAbsolute(linkRedirect.get('to'))) + to: new URL(linkRedirect.get('to')) }); } } diff --git a/ghost/core/core/server/services/link-redirection/index.js b/ghost/core/core/server/services/link-redirection/index.js index 0d243a8ac2..7eba3c1134 100644 --- a/ghost/core/core/server/services/link-redirection/index.js +++ b/ghost/core/core/server/services/link-redirection/index.js @@ -14,8 +14,7 @@ class LinkRedirectsServiceWrapper { const {LinkRedirectsService} = require('@tryghost/link-redirects'); const linkRedirectRepository = new LinkRedirectRepository({ - LinkRedirect: models.LinkRedirect, - urlUtils + LinkRedirect: models.LinkRedirect }); // Expose the service diff --git a/ghost/core/core/server/services/members/api.js b/ghost/core/core/server/services/members/api.js index 93f71ec9dd..37294e2626 100644 --- a/ghost/core/core/server/services/members/api.js +++ b/ghost/core/core/server/services/members/api.js @@ -187,6 +187,7 @@ function createApiInstance(config) { MemberAnalyticEvent: models.MemberAnalyticEvent, MemberCreatedEvent: models.MemberCreatedEvent, SubscriptionCreatedEvent: models.SubscriptionCreatedEvent, + MemberLinkClickEvent: models.MemberLinkClickEvent, OfferRedemption: models.OfferRedemption, Offer: models.Offer, StripeProduct: models.StripeProduct, diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/members.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/members.test.js.snap index ab58f972d5..a24e700ece 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/members.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/members.test.js.snap @@ -3805,7 +3805,7 @@ exports[`Members API Returns comments in activity feed 2: [headers] 1`] = ` Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "1093", + "content-length": "1147", "content-type": "application/json; charset=utf-8", "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "vary": "Origin, Accept-Encoding", diff --git a/ghost/members-api/lib/MembersAPI.js b/ghost/members-api/lib/MembersAPI.js index 901425c725..2a09513a1b 100644 --- a/ghost/members-api/lib/MembersAPI.js +++ b/ghost/members-api/lib/MembersAPI.js @@ -51,6 +51,7 @@ module.exports = function MembersAPI({ MemberAnalyticEvent, MemberCreatedEvent, SubscriptionCreatedEvent, + MemberLinkClickEvent, Offer, OfferRedemption, StripeProduct, @@ -110,6 +111,7 @@ module.exports = function MembersAPI({ MemberLoginEvent, MemberCreatedEvent, SubscriptionCreatedEvent, + MemberLinkClickEvent, Comment, labsService, memberAttributionService diff --git a/ghost/members-api/lib/repositories/event.js b/ghost/members-api/lib/repositories/event.js index 45203c4af5..15a2d8e6fe 100644 --- a/ghost/members-api/lib/repositories/event.js +++ b/ghost/members-api/lib/repositories/event.js @@ -11,6 +11,7 @@ module.exports = class EventRepository { MemberCreatedEvent, SubscriptionCreatedEvent, MemberPaidSubscriptionEvent, + MemberLinkClickEvent, Comment, labsService, memberAttributionService @@ -25,9 +26,59 @@ module.exports = class EventRepository { this._labsService = labsService; this._MemberCreatedEvent = MemberCreatedEvent; this._SubscriptionCreatedEvent = SubscriptionCreatedEvent; + this._MemberLinkClickEvent = MemberLinkClickEvent; this._memberAttributionService = memberAttributionService; } + async getEventTimeline(options = {}) { + if (!options.limit) { + options.limit = 10; + } + + // Changing this order might need a change in the query functions + // because of the different underlying models. + options.order = 'created_at desc'; + + // Create a list of all events that can be queried + const pageActions = [ + {type: 'newsletter_event', action: 'getNewsletterSubscriptionEvents'}, + {type: 'subscription_event', action: 'getSubscriptionEvents'}, + {type: 'login_event', action: 'getLoginEvents'}, + {type: 'payment_event', action: 'getPaymentEvents'}, + {type: 'signup_event', action: 'getSignupEvents'}, + {type: 'comment_event', action: 'getCommentEvents'} + ]; + + if (this._labsService.isSet('emailClicks')) { + pageActions.push({type: 'click_event', action: 'getClickEvents'}); + } + + if (this._EmailRecipient) { + pageActions.push({type: 'email_delivered_event', action: 'getEmailDeliveredEvents'}); + pageActions.push({type: 'email_opened_event', action: 'getEmailOpenedEvents'}); + pageActions.push({type: 'email_failed_event', action: 'getEmailFailedEvents'}); + } + + let filters = this.getNQLSubset(options.filter); + + //Filter events to query + const filteredPages = filters.type ? pageActions.filter(page => nql(filters.type).queryJSON(page)) : pageActions; + + //Start the promises + const pages = filteredPages.map(page => this[page.action](options, filters)); + + const allEventPages = await Promise.all(pages); + + const allEvents = allEventPages.reduce((accumulator, page) => accumulator.concat(page.data), []); + + return allEvents.sort((a, b) => { + return new Date(b.data.created_at) - new Date(a.data.created_at); + }).reduce((memo, event) => { + //disable the event filtering + return memo.concat(event); + }, []).slice(0, options.limit); + } + async registerPayment(data) { await this._MemberPaymentEvent.add({ ...data, @@ -276,6 +327,35 @@ module.exports = class EventRepository { }; } + async getClickEvents(options = {}, filters = {}) { + options = { + ...options, + withRelated: ['member', 'link', 'link.post'], + filter: [] + }; + if (filters['data.created_at']) { + options.filter.push(filters['data.created_at'].replace(/data.created_at:/g, 'created_at:')); + } + if (filters['data.member_id']) { + options.filter.push(filters['data.member_id'].replace(/data.member_id:/g, 'member_id:')); + } + options.filter = options.filter.join('+'); + + const {data: models, meta} = await this._MemberLinkClickEvent.findPage(options); + + const data = models.map((model) => { + return { + type: 'click_event', + data: model.toJSON(options) + }; + }); + + return { + data, + meta + }; + } + async getEmailDeliveredEvents(options = {}, filters = {}) { options = { ...options, @@ -444,50 +524,6 @@ module.exports = class EventRepository { return result; } - async getEventTimeline(options = {}) { - if (!options.limit) { - options.limit = 10; - } - - // Changing this order might need a change in the query functions - // because of the different underlying models. - options.order = 'created_at desc'; - - // Create a list of all events that can be queried - const pageActions = [ - {type: 'newsletter_event', action: 'getNewsletterSubscriptionEvents'}, - {type: 'subscription_event', action: 'getSubscriptionEvents'}, - {type: 'login_event', action: 'getLoginEvents'}, - {type: 'payment_event', action: 'getPaymentEvents'}, - {type: 'signup_event', action: 'getSignupEvents'}, - {type: 'comment_event', action: 'getCommentEvents'} - ]; - if (this._EmailRecipient) { - pageActions.push({type: 'email_delivered_event', action: 'getEmailDeliveredEvents'}); - pageActions.push({type: 'email_opened_event', action: 'getEmailOpenedEvents'}); - pageActions.push({type: 'email_failed_event', action: 'getEmailFailedEvents'}); - } - - let filters = this.getNQLSubset(options.filter); - - //Filter events to query - const filteredPages = filters.type ? pageActions.filter(page => nql(filters.type).queryJSON(page)) : pageActions; - - //Start the promises - const pages = filteredPages.map(page => this[page.action](options, filters)); - - const allEventPages = await Promise.all(pages); - - const allEvents = allEventPages.reduce((accumulator, page) => accumulator.concat(page.data), []); - - return allEvents.sort((a, b) => { - return new Date(b.data.created_at) - new Date(a.data.created_at); - }).reduce((memo, event) => { - //disable the event filtering - return memo.concat(event); - }, []).slice(0, options.limit); - } - async getSubscriptions() { const results = await this._MemberSubscribeEvent.findAll({ aggregateSubscriptionDeltas: true