From 6a34cf63eb9c26097c5cf3494a67d36b51888580 Mon Sep 17 00:00:00 2001 From: Eve Essex Date: Thu, 8 Oct 2020 17:36:32 -0400 Subject: [PATCH 01/20] deprecate analytics/articles, move legacy tracking events to related components --- src/desktop/analytics/articles.js | 55 ------------------- src/desktop/assets/analytics.ts | 1 - .../article/client/super_article.coffee | 33 +++++++++++ .../components/article/client/view.coffee | 16 ++++++ 4 files changed, 49 insertions(+), 56 deletions(-) delete mode 100644 src/desktop/analytics/articles.js diff --git a/src/desktop/analytics/articles.js b/src/desktop/analytics/articles.js deleted file mode 100644 index 574d439b866..00000000000 --- a/src/desktop/analytics/articles.js +++ /dev/null @@ -1,55 +0,0 @@ -// -// Analytics for all things articles. This includes tests for share buttons -// and potentionally other alternative layout options or more. -// - -if ( - location.pathname.match("/article/") || - location.pathname.match("/2016-year-in-art") -) { - $(document.body) - .on("click", ".article-social a", function () { - const articleId = $(this).closest(".article-container").data("id") - analytics.track("Article Share", { - article_id: articleId, - context_type: sd.ARTICLE ? "article_fixed" : "magazine_fixed", - service: $(this).attr("data-service"), - }) - }) - .on("click", ".article-share-fixed > a", function () { - const articleId = $(this).closest(".article-container").data("id") - analytics.track("Article Share", { - article_id: articleId, - context_type: "article_sticky", - service: $(this).attr("data-service"), - }) - }) - .on("click", ".article-sa-primary-logo a", function () { - analytics.track("Clicked primary partner logo", { - destination_path: $(this)[0].href, - impression_type: "sa_primary_logo", - context_type: "article_fixed", - }) - }) - .on("click", ".article-sa-secondary-logo a", function () { - analytics.track("Clicked secondary partner logo", { - destination_path: $(this)[0].href, - impression_type: "sa_secondary_logo", - context_type: "article_fixed", - }) - }) - .on("click", ".article-sa-cta-container a", function () { - analytics.track("Clicked partner cta link", { - destination_path: $(this)[0].href, - impression_type: "sa_partner_cta", - context_type: "article_fixed", - }) - }) - .on("click", ".article-sa-footer-blurb a", function () { - analytics.track("Clicked partner cta link in footer blurb", { - destination_path: $(this)[0].href, - impression_type: "sa_partner_cta", - context_type: "article_fixed", - }) - }) -} diff --git a/src/desktop/assets/analytics.ts b/src/desktop/assets/analytics.ts index 676370fb903..312a14920c1 100644 --- a/src/desktop/assets/analytics.ts +++ b/src/desktop/assets/analytics.ts @@ -44,7 +44,6 @@ $(() => onAnalyticsReady() setupSplitTests() - require("../analytics/articles.js") require("../analytics/artists.js") require("../analytics/artwork.js") require("../analytics/consignments.js") diff --git a/src/desktop/components/article/client/super_article.coffee b/src/desktop/components/article/client/super_article.coffee index 04d6c7c209d..ab7680d5f4f 100644 --- a/src/desktop/components/article/client/super_article.coffee +++ b/src/desktop/components/article/client/super_article.coffee @@ -4,6 +4,11 @@ initCarousel = require '../../merry_go_round/horizontal_nav_mgr.coffee' sd = require('sharify').data module.exports = class SuperArticleView extends Backbone.View + events: + 'click .article-sa-primary-logo a': 'trackPrimaryLogoClick' + 'click .article-sa-secondary-logo a': 'trackSecondaryLogoClick' + 'click .article-sa-cta-container a': 'trackCtaClick' + 'click .article-sa-footer-blurb a': 'trackFooterBlurbClick' initialize: (options) -> { @article } = options @@ -79,3 +84,31 @@ module.exports = class SuperArticleView extends Backbone.View else @$superArticleNavToc.css 'height', '100vh' @$body.addClass 'is-open' + + trackPrimaryLogoClick: (e) -> + window.analytics.track("Clicked primary partner logo", { + destination_path: e.currentTarget.href, + impression_type: "sa_primary_logo", + context_type: "article_fixed", + }) + + trackSecondaryLogoClick: (e) -> + window.analytics.track("Clicked secondary partner logo", { + destination_path: e.currentTarget.href, + impression_type: "sa_secondary_logo", + context_type: "article_fixed", + }) + + trackCtaClick: (e) -> + window.analytics.track("Clicked partner cta link", { + destination_path: e.currentTarget.href, + impression_type: "sa_partner_cta", + context_type: "article_fixed", + }) + + trackFooterBlurbClick: (e) -> + window.analytics.track("Clicked partner cta link in footer blurb", { + destination_path: e.currentTarget.href, + impression_type: "sa_partner_cta", + context_type: "article_fixed", + }) \ No newline at end of file diff --git a/src/desktop/components/article/client/view.coffee b/src/desktop/components/article/client/view.coffee index f708ef062af..f0101764771 100644 --- a/src/desktop/components/article/client/view.coffee +++ b/src/desktop/components/article/client/view.coffee @@ -20,6 +20,8 @@ module.exports = class ArticleView extends Backbone.View .articles-section-left-chevron': 'toggleSectionCarousel' 'click .article-video-play-button': 'playVideo' 'click .article-section-image-set__remaining, .article-section-image-set__image-container': 'toggleModal' + 'click .article-social a': 'trackArticleSocialShare' + 'click .article-share-fixed a': 'trackArticleFixedShare' initialize: (options) -> @user = CurrentUser.orNull() @@ -269,3 +271,17 @@ module.exports = class ArticleView extends Backbone.View else $('.article-social').show() , { offset: 'bottom-in-view' } + + trackArticleSocialShare: (e) => + window.analytics.track("Article Share", { + article_id: @article.id, + context_type: "article_fixed" + service: e.currentTarget.getAttribute('data-service'), + }) + + trackArticleFixedShare: (e) => + window.analytics.track("Article Share", { + article_id: @article.id, + context_type: "article_sticky", + service: e.currentTarget.getAttribute('data-service'), + }) From 4002c655a5abeed772177fe725d3ad2f698391b0 Mon Sep 17 00:00:00 2001 From: Eve Essex Date: Thu, 8 Oct 2020 17:56:24 -0400 Subject: [PATCH 02/20] deprecate unused embedded-inquiry track events --- src/desktop/analytics/embedded_inquiry.js | 23 ----------------------- src/desktop/assets/analytics.ts | 1 - 2 files changed, 24 deletions(-) delete mode 100644 src/desktop/analytics/embedded_inquiry.js diff --git a/src/desktop/analytics/embedded_inquiry.js b/src/desktop/analytics/embedded_inquiry.js deleted file mode 100644 index f7276918690..00000000000 --- a/src/desktop/analytics/embedded_inquiry.js +++ /dev/null @@ -1,23 +0,0 @@ -// Events ported from apps/artwork/components/contact.coffee -// & components/contact/inquiry.coffee - -;(function() { - "use strict" - - // DOM events - var $document = $(document) - - $document.on("click", ".js-send-embedded-inquiry", function() { - analytics.track('Clicked "Contact Gallery" button', { - id: $(this).data("artwork-id"), - prequalify: - sd.ARTWORK && sd.ARTWORK.partner && sd.ARTWORK.partner.pre_qualify, - }) - }) - - $document.on("mouseover", ".js-send-embedded-inquiry", function() { - analytics.track("Hovered over contact form 'Send' button", { - id: $(this).data("artwork-id"), - }) - }) -})() diff --git a/src/desktop/assets/analytics.ts b/src/desktop/assets/analytics.ts index 4562badee42..1a547732116 100644 --- a/src/desktop/assets/analytics.ts +++ b/src/desktop/assets/analytics.ts @@ -48,7 +48,6 @@ $(() => require("../analytics/artwork.js") require("../analytics/consignments.js") require("../analytics/contact.js") - require("../analytics/embedded_inquiry.js") require("../analytics/galleries.js") require("../analytics/inquiry_questionnaire.js") require("../analytics/recently_viewed_artworks.js") From 90987d68f6f470416c1679beda9a8897b3eef857 Mon Sep 17 00:00:00 2001 From: Eve Essex Date: Mon, 12 Oct 2020 11:07:23 -0400 Subject: [PATCH 03/20] fix backbone/jquery flake in mocha tests --- .../apps/fair/test/client/for_you.test.js | 77 ++++++++----------- .../apps/fair/test/client/main_page.test.js | 45 +++++------ .../components/article/test/view.test.js | 45 ++++++----- 3 files changed, 75 insertions(+), 92 deletions(-) diff --git a/src/mobile/apps/fair/test/client/for_you.test.js b/src/mobile/apps/fair/test/client/for_you.test.js index 58cd27d54a1..5bc663b8e7b 100644 --- a/src/mobile/apps/fair/test/client/for_you.test.js +++ b/src/mobile/apps/fair/test/client/for_you.test.js @@ -1,8 +1,3 @@ -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const _ = require("underscore") const Backbone = require("backbone") const CurrentUser = require("../../../../models/current_user") @@ -14,69 +9,63 @@ const sinon = require("sinon") const benv = require("benv") const { resolve } = require("path") -describe("For You View", function () { - beforeEach(function (done) { - this.user = new CurrentUser(fabricate("user", { id: "current-user-id" })) - return benv.setup(() => { +describe("For You View", () => { + let view + let user + let fair + beforeEach(done => { + const profile = new Profile(fabricate("fair_profile")) + user = new CurrentUser(fabricate("user", { id: "current-user-id" })) + benv.setup(() => { benv.expose({ $: benv.require("jquery") }) Backbone.$ = $ $.onInfiniteScroll = function () {} - sinon.stub(CurrentUser, "orNull").returns(this.user) + sinon.stub(CurrentUser, "orNull").returns(user) sinon.stub(Backbone, "sync") - return benv.render( + benv.render( resolve(__dirname, "../../templates/for_you.jade"), { - fair: (this.fair = new Fair(fabricate("fair"))), - profile: (this.profile = new Profile(fabricate("fair_profile"))), + fair: (fair = new Fair(fabricate("fair"))), + profile, sd: {}, }, () => { const { ForYouView } = require("../../client/for_you") - this.view = new ForYouView({ + view = new ForYouView({ el: $("#fair-for-you"), - fair: this.fair, - profile: this.profile, + fair: fair, + profile, }) - return done() + done() } ) }) }) - afterEach(function () { + afterEach(() => { benv.teardown() Backbone.sync.restore() - return CurrentUser.orNull.restore() + CurrentUser.orNull.restore() }) - describe("init code", () => - it("creates a for you fair page view", function () { - return this.view.$("#fair-page-title").text().should.containEql("Guide") - })) - - xdescribe("logged out / not many follows", () => - xit("renders exhibitors to follow", function () {})) + describe("init code", () => { + it("creates a for you fair page view", () => { + view.$("#fair-page-title").text().should.containEql("Guide") + }) + }) - return describe("logged in", function () { - describe("#updateAndShowTitle", () => - it("renders the user's name in the heading", function () { - return this.view - .$("#fair-page-title") - .text() - .should.containEql(this.user.get("name")) - })) + describe("logged in", () => { + describe("#updateAndShowTitle", () => { + it("renders the user's name in the heading", () => { + view.$("#fair-page-title").text().should.containEql(user.get("name")) + }) + }) - describe("#fetchFollowingExhibitors", () => + describe("#fetchFollowingExhibitors", () => { it("renders fair exhibitor booths of partner profiles the current user follows", function () { Backbone.sync.args[0][2].url.should.containEql("me/follow/profiles") - return Backbone.sync.args[0][2].data.fair_id.should.equal( - this.fair.get("id") - ) - })) - //Backbone.sync.args[0][2].success({results: [fabricate('partner_profile')]}) - //Backbone.sync.args[0][2].success([]) - - return xdescribe("#fetchFollowingArtists", () => - it("renders links to booths of artists the current user follows", function () {})) + Backbone.sync.args[0][2].data.fair_id.should.equal(fair.get("id")) + }) + }) }) }) diff --git a/src/mobile/apps/fair/test/client/main_page.test.js b/src/mobile/apps/fair/test/client/main_page.test.js index 1feccb0e5f0..d4176e8a474 100644 --- a/src/mobile/apps/fair/test/client/main_page.test.js +++ b/src/mobile/apps/fair/test/client/main_page.test.js @@ -1,8 +1,3 @@ -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const _ = require("underscore") const Backbone = require("backbone") const Fair = require("../../../../models/fair") @@ -12,15 +7,15 @@ const sinon = require("sinon") const benv = require("benv") const { resolve } = require("path") -describe("Main page client-side code", function () { - beforeEach(function (done) { - return benv.setup(() => { +describe("Main page client-side code", () => { + let view + beforeEach(done => { + benv.setup(() => { benv.expose({ $: benv.require("jquery") }) - Backbone.$ = $ $.onInfiniteScroll = function () {} sinon.stub(Backbone.history, "start") sinon.stub(Backbone, "sync") - return benv.render( + benv.render( resolve(__dirname, "../../templates/main_page.jade"), { fair: new Fair(fabricate("fair")), @@ -47,33 +42,34 @@ describe("Main page client-side code", function () { "imageNavItemTemplate", ] ) - this.view = new FairMainPageView({ + Backbone.$ = $ + view = new FairMainPageView({ el: $("body"), model: new Fair(fabricate("fair")), }) - return done() + done() } ) }) }) - afterEach(function () { + afterEach(() => { benv.teardown() Backbone.sync.restore() - return Backbone.history.start.restore() + Backbone.history.start.restore() }) - return describe("FairMainPageView", () => - describe("#renderFeaturedLinks", function () { - it("fetches the fairs sets", function () { - this.view.renderFeaturedLinks() - return _.last(Backbone.sync.args)[2].url.should.containEql( + describe("FairMainPageView", () => { + describe("#renderFeaturedLinks", () => { + it("fetches the fairs sets", () => { + view.renderFeaturedLinks() + _.last(Backbone.sync.args)[2].url.should.containEql( "/sets?owner_type=Fair" ) }) - return it("renders the featured links", function () { - this.view.renderFeaturedLinks() + it("renders the featured links", () => { + view.renderFeaturedLinks() _.last(Backbone.sync.args)[2].success([ fabricate("set", { key: "editorial", display_on_martsy: true }), ]) @@ -83,9 +79,8 @@ describe("Main page client-side code", function () { display_on_martsy: true, }), ]) - return this.view.$el - .html() - .should.containEql("Featured link for this awesome page") + view.$el.html().should.containEql("Featured link for this awesome page") }) - })) + }) + }) }) diff --git a/src/mobile/components/article/test/view.test.js b/src/mobile/components/article/test/view.test.js index d0f6d40a7ae..99a054f5771 100644 --- a/src/mobile/components/article/test/view.test.js +++ b/src/mobile/components/article/test/view.test.js @@ -1,8 +1,3 @@ -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const _ = require("underscore") const benv = require("benv") const sinon = require("sinon") @@ -13,16 +8,16 @@ const sd = require("sharify").data const { resolve } = require("path") const { fabricate } = require("@artsy/antigravity") -describe("ArticleView", function () { - beforeEach(function (done) { - return benv.setup(() => { +describe("ArticleView", () => { + let article + let ArticleView + beforeEach(done => { + benv.setup(() => { benv.expose({ $: benv.require("jquery"), Element: window.Element, }) - // benv bug that doesn't attach globals to window - Backbone.$ = $ - this.article = fabricate("article", { + article = fabricate("article", { sections: [ { type: "video", @@ -37,10 +32,10 @@ describe("ArticleView", function () { section_ids: [], }) - return benv.render( + benv.render( resolve(__dirname, "../templates/index.jade"), { - article: new Article(this.article), + article: new Article(article), asset() {}, sd: {}, resize() {}, @@ -51,28 +46,32 @@ describe("ArticleView", function () { relatedArticles: new Articles(), }, () => { - this.ArticleView = benv.requireWithJadeify( + Backbone.$ = $ + ArticleView = benv.requireWithJadeify( resolve(__dirname, "../client/view"), [] ) sinon.stub(Backbone, "sync") - return done() + done() } ) }) }) - afterEach(function () { + afterEach(() => { benv.teardown() - return Backbone.sync.restore() + Backbone.sync.restore() }) - return describe("#clickPlay", () => - it("replaces iFrame with an autoplay attribute", function () { - this.view = new this.ArticleView({ el: $("body") }) - $(".article-video-play-button").click() - return $(".article-section-video iframe") + describe("#clickPlay", () => { + let view + it("replaces iFrame with an autoplay attribute", () => { + view = new ArticleView({ el: $("body") }) + view.$(".article-video-play-button").click() + view + .$(".article-section-video iframe") .attr("src") .should.containEql("autoplay=1") - })) + }) + }) }) From 3c99922eabe75abb2e1956b12ba806b52ad8ba71 Mon Sep 17 00:00:00 2001 From: Eve Essex Date: Mon, 12 Oct 2020 13:23:02 -0400 Subject: [PATCH 04/20] sync metaphysics schema --- data/schema.graphql | 1 - 1 file changed, 1 deletion(-) diff --git a/data/schema.graphql b/data/schema.graphql index 177ed388fa4..bdd5b8b6c08 100644 --- a/data/schema.graphql +++ b/data/schema.graphql @@ -7487,7 +7487,6 @@ input MyCollectionUpdateArtworkInput { depth: String editionNumber: String editionSize: String - externalImageUrls: [String] height: String medium: String metric: String From bf422a7774d9ed7399b6d48f8634b3d56a725e7f Mon Sep 17 00:00:00 2001 From: Eve Essex Date: Mon, 12 Oct 2020 13:24:25 -0400 Subject: [PATCH 05/20] reinstate sync schema command --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 05a71c35b71..fa11a3e6253 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "start:prod": "yarn assets && yarn build:server && NODE_ENV=production yarn start", "start": "scripts/start.sh", "storybook": "concurrently --raw --kill-others 'yarn relay --watch' 'start-storybook --quiet -s ./public -p 9001'", + "sync-schema": "scripts/sync-schema-pull.sh", "test:acceptance": "yarn acceptance src/test/acceptance/*.js", "test:smoke": "scripts/smoke_test.sh", "test": "scripts/test.sh", From 90dad17d5accd1fd65092d5a41b455f698b9cf75 Mon Sep 17 00:00:00 2001 From: Eve Essex Date: Mon, 12 Oct 2020 14:01:41 -0400 Subject: [PATCH 06/20] add data attrs for integrity to bid buttons --- .../Components/ArtworkSidebar/ArtworkSidebarBidAction.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/v2/Apps/Artwork/Components/ArtworkSidebar/ArtworkSidebarBidAction.tsx b/src/v2/Apps/Artwork/Components/ArtworkSidebar/ArtworkSidebarBidAction.tsx index 6f68e3783d6..2ebae9ab72e 100644 --- a/src/v2/Apps/Artwork/Components/ArtworkSidebar/ArtworkSidebarBidAction.tsx +++ b/src/v2/Apps/Artwork/Components/ArtworkSidebar/ArtworkSidebarBidAction.tsx @@ -33,7 +33,7 @@ const RegisterToBidButton: React.FC<{ onClick: () => void }> = ({ onClick, }) => { return ( - ) @@ -279,6 +279,7 @@ export class ArtworkSidebarBidAction extends React.Component<