Merge pull request #5115 from artsy/staging

Deploy

.babelrc

@@ -18,6 +18,8 @@
   "plugins": [
     "@babel/plugin-proposal-class-properties",
     "@babel/plugin-transform-runtime",
+    "@babel/plugin-proposal-optional-chaining",
+    "@babel/plugin-proposal-nullish-coalescing-operator",
     [
       "babel-plugin-styled-components",
       {

package.json

@@ -74,9 +74,9 @@
   "dependencies": {
     "@artsy/express-reloadable": "1.4.7",
     "@artsy/gemup": "0.0.3",
-    "@artsy/palette": "7.1.0",
+    "@artsy/palette": "7.1.1",
     "@artsy/passport": "1.1.11",
-    "@artsy/reaction": "25.18.0",
+    "@artsy/reaction": "25.18.1",
     "@artsy/stitch": "6.1.6",
     "@babel/core": "7.6.0",
     "@babel/node": "7.6.1",
@@ -142,6 +142,7 @@
     "embedly-view-helpers": "artsy/embedly-view-helpers",
     "es7-object-polyfill": "0.0.7",
     "express": "4.16.4",
+    "express-http-context": "1.2.3",
     "express-ipfilter": "0.2.1",
     "express-request-id": "1.4.0",
     "factor-bundle": "2.5.0",
@@ -242,6 +243,8 @@
   },
   "devDependencies": {
     "@artsy/antigravity": "0.2.0",
+    "@babel/plugin-proposal-nullish-coalescing-operator": "7.8.3",
+    "@babel/plugin-proposal-optional-chaining": "7.8.3",
     "@babel/plugin-transform-modules-commonjs": "7.6.0",
     "@graphql-inspector/core": "1.14.0",
     "@loadable/babel-plugin": "5.12.0",

src/desktop/apps/experimental-app-shell/apps/artwork/artworkClient.tsx

@@ -11,9 +11,12 @@ export const artworkClient = () => {
   const ViewInRoomView = require("desktop/components/view_in_room/view.coffee")
   const $ = require("jquery")
   const mediator = require("desktop/lib/mediator.coffee")
+  const pageType = window.location.pathname.split("/")[1]
 
-  const artworkSlug = location.pathname.replace(/\/artwork\//, "")
-  recordArtworkView(artworkSlug, sd.CURRENT_USER)
+  if (pageType === "artwork") {
+    const artworkSlug = location.pathname.replace(/\/artwork\//, "")
+    recordArtworkView(artworkSlug, sd.CURRENT_USER)
+  }
 
   const openInquireableModal = (
     artworkId: string,

src/desktop/apps/experimental-app-shell/client.tsx

@@ -17,20 +17,12 @@ buildClientApp({
   } as any,
 })
   .then(({ ClientApp }) => {
-    const pageType = window.location.pathname.split("/")[1] as
-      | "artist"
-      | "artwork"
-
     /**
      * Mount route-specific client code here
      */
     const mountClientAppSupport = () => {
-      switch (pageType) {
-        case "artist":
-          return artistClient()
-        case "artwork":
-          return artworkClient()
-      }
+      artistClient()
+      artworkClient()
     }
 
     loadableReady(() => {

src/desktop/apps/experimental-app-shell/server.tsx

@@ -3,8 +3,8 @@ import express, { Request } from "express"
 import { buildServerApp } from "@artsy/reaction/dist/Artsy/Router/server"
 import { getAppRoutes } from "reaction/Apps/getAppRoutes"
 import { stitch } from "@artsy/stitch"
-import { buildServerAppContext } from "desktop/lib/buildServerAppContext"
 
+import { buildServerAppContext } from "desktop/lib/buildServerAppContext"
 import { handleArtworkImageDownload } from "./apps/artwork/artworkMiddleware"
 import { artistMiddleware } from "./apps/artist/artistMiddleware"
 import { bidderRegistrationMiddleware } from "./apps/auction/bidderRegistrationMiddleware"

src/desktop/assets/main_layout.styl

@@ -8,6 +8,7 @@
 @require '../components/clock'
 @require '../components/recently_viewed_artworks'
 @require '../components/inquiry_questionnaire/stylesheets'
+@require '../components/view_in_room'
 
 // TODO: Remove once Buyers Premium modal has been rebuilt in Reaction
 .artwork-auction-buyers-premium-modal

src/desktop/components/split_test/middleware.coffee

@@ -1,6 +1,7 @@
 SplitTest = require './server_split_test.coffee'
 runningTests = require './running_tests'
 qs = require 'qs'
+{ setSplitTest } = require './splitTestContext'
 
 module.exports = (req, res, next) ->
   for key, configuration of runningTests
@@ -15,4 +16,11 @@ module.exports = (req, res, next) ->
       test.set v
       res.locals.sd[k.toUpperCase()] = v
 
+  # TODO: Remove when the client nav AB test ends.
+  if runningTests['client_navigation_v3']
+    res.locals.sd['EXPERIMENTAL_APP_SHELL'] = Boolean(res.locals.sd['CLIENT_NAVIGATION_V3'] is 'experiment')
+
+  # Store value in globally available location.
+  setSplitTest('EXPERIMENTAL_APP_SHELL', res.locals.sd['EXPERIMENTAL_APP_SHELL'])
+
   next()

src/desktop/components/split_test/running_tests.coffee

@@ -1,5 +1,7 @@
 # Centralizes configuration for currently running split tests
 #
+# USE: Append `?split_test[test_name]=experiment` query param in url.
+#
 # eg.
 # header_design:
 #   key: 'header_design'
@@ -24,4 +26,10 @@
 # this should export empty Object
 # module.exports = {}
 
-module.exports = {}
+module.exports = {
+  client_navigation_v3:
+    key: "client_navigation_v3"
+    outcomes:
+      control: 50
+      experiment: 50
+}

src/desktop/components/split_test/skipIfClientSideRoutingEnabled.ts

@@ -1,7 +1,9 @@
+import { getSplitTest } from "./splitTestContext"
+
 export const skipIfClientSideRoutingEnabled = (_req, _res, next) => {
   if (
+    getSplitTest("EXPERIMENTAL_APP_SHELL") ||
     process.env.EXPERIMENTAL_APP_SHELL
-    // && res.locals.sd.CLIENT_NAVIGATION_V2 === "experiment"
   ) {
     return next("route")
   } else {

src/desktop/components/split_test/splitTestContext.ts

@@ -0,0 +1,10 @@
+import httpContext from "express-http-context"
+
+export function getSplitTest(ENV_VAR) {
+  const envVar = httpContext.get(ENV_VAR)
+  return envVar
+}
+
+export function setSplitTest(ENV_VAR, value) {
+  httpContext.set(ENV_VAR, value)
+}

src/desktop/components/view_in_room/index.styl

@@ -32,7 +32,7 @@ view-in-room = {
     text-align center
     background-color gray-darker-color
     z-index 2
-    opacity 0.6
+    opacity 0
     transition opacity 0.25s
     text-decoration none
 

src/desktop/components/view_in_room/view.coffee

@@ -80,6 +80,8 @@ module.exports = class ViewInRoom extends Backbone.View
         artworkTransformCSS[key] = Math.abs artworkTransformCSS[key] * @roomScalingFactor()
       artworkTransformCSS['left'] = Math.floor ($(window).innerWidth() - artworkTransformCSS['width']) / 2
 
+    $('.js-view-in-room-close').css('opacity', 1)
+
     @$artwork
       .addClass 'is-transition'
       .css artworkTransformCSS
@@ -161,6 +163,7 @@ module.exports = class ViewInRoom extends Backbone.View
     })
 
   remove: ->
+    $('.js-view-in-room-close').remove()
     $(window).off 'resize.view-in-room'
     @transitionOut()
       .one $.support.transition.end, =>

src/desktop/index.js

@@ -1,5 +1,6 @@
 import { middleware as stitchMiddleware } from "@artsy/stitch/dist/internal/middleware"
 import * as globalReactModules from "desktop/components/react/stitch_components"
+import { getSplitTest } from "desktop/components/split_test/splitTestContext"
 
 const app = (module.exports = require("express")())
 
@@ -94,6 +95,9 @@ app.use(require("./apps/user"))
 // Used to test various SSR configurations
 app.use(require("./apps/ssr-experiments/server").app)
 
-if (process.env.EXPERIMENTAL_APP_SHELL) {
+if (
+  getSplitTest("EXPERIMENTAL_APP_SHELL") ||
+  process.env.EXPERIMENTAL_APP_SHELL
+) {
   app.use(require("./apps/experimental-app-shell/server").app)
 }

src/index.js

@@ -33,9 +33,13 @@ const path = require("path")
 const { setAliases } = require("require-control")
 
 // Force resolution of potentially `yarn link`'d modules to the local node_modules
-// folder. This gets around SSR issues involving single react context requirements.
-// This is server-side only. Client-side must be resolved via webpack.
+// folder. This gets around SSR issues involving single react context requirements,
+// amongst other things. This is server-side only. Client-side must be resolved
+// via webpack.
 setAliases({
+  "express-http-context": path.resolve(
+    path.join(__dirname, "../node_modules/express-http-context")
+  ),
   react: path.resolve(path.join(__dirname, "../node_modules/react")),
   "react-dom": path.resolve(path.join(__dirname, "../node_modules/react-dom")),
   "styled-components": path.resolve(

src/lib/setup.js

@@ -24,6 +24,7 @@ import siteAssociation from "artsy-eigen-web-association"
 import superSync from "backbone-super-sync"
 import { IpFilter as ipfilter } from "express-ipfilter"
 import timeout from "connect-timeout"
+import httpContext from "express-http-context"
 import "./setup_sharify"
 import cache from "./cache"
 import downcase from "./middleware/downcase"
@@ -208,6 +209,18 @@ export default function(app) {
   )
   app.use("/(.well-known/)?apple-app-site-association", siteAssociation)
 
+  /**
+   * Add support for request-scoped contexts; meaning, a variable set in the
+   * scope of a request can be accessed outside of the express chain. Think,
+   * A/B tests.
+   *
+   * NOTE: some popular middlewares (such as body-parser, express-jwt) may cause
+   * context to get lost. To workaround such issues, you are advised to use any
+   * third party middleware that does NOT need the context BEFORE you use this
+   * middleware.
+   */
+  app.use(httpContext.middleware)
+
   // Redirect requests before they even have to deal with Force routing
   app.use(downcase)
   app.use(hardcodedRedirects)
@@ -217,6 +230,7 @@ export default function(app) {
   app.use(escapedFragmentMiddleware)
   app.use(logger)
   app.use(unsupportedBrowserCheck)
+
   if (NODE_ENV !== "test") app.use(splitTestMiddleware)
   app.use(addIntercomUserHash)
   app.use(pageCacheMiddleware)