Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Review App] Close out new App Shell A/B test #5654

Closed
wants to merge 12 commits into from
Closed
2 changes: 0 additions & 2 deletions .storybook/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const {
CI,
CMS_URL,
ENABLE_SIGN_IN_WITH_APPLE,
EXPERIMENTAL_APP_SHELL,
FACEBOOK_APP_NAMESPACE,
PREDICTION_URL,
FORCE_CLOUDFRONT_URL,
Expand Down Expand Up @@ -56,7 +55,6 @@ const sharifyPath = sharify({
CDN_URL,
CMS_URL,
ENABLE_SIGN_IN_WITH_APPLE,
EXPERIMENTAL_APP_SHELL,
FACEBOOK_APP_NAMESPACE,
FORCE_CLOUDFRONT_URL,
GEMINI_CLOUDFRONT_URL,
Expand Down
1 change: 1 addition & 0 deletions cypress/integration/partnerProfile.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ describe("/:partner_id", () => {
})
it("shows partner articles", () => {
cy.visit("gagosian-gallery/articles")
cy.wait(2000)
cy.contains("Articles")
})

Expand Down
5 changes: 3 additions & 2 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
const sharedConfig = {
transform: {
"\\.(gql|graphql)$": "jest-transform-graphql",
"^.+\\.coffee$": "<rootDir>/node_modules/jest-coffee-preprocessor/index.js",
".(ts|tsx|js|jsx)": "babel-jest",
"\\.graphql$": "jest-raw-loader",
},
cacheDirectory: ".cache/jest",
coverageDirectory: "./coverage/",
collectCoverage: true,
coverageReporters: ["lcov", "text-summary"],
Expand All @@ -18,7 +19,7 @@ module.exports = {
*/
{
...sharedConfig,
modulePathIgnorePatterns: ["v2"],
modulePathIgnorePatterns: ["v2", "data"],
testRegex: ".*\\.jest\\.(ts|tsx|js|jsx)$",
setupFiles: ["<rootDir>/test.config.js"],
roots: ["<rootDir>/src"],
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@
"fork-ts-checker-notifier-webpack-plugin": "0.4.0",
"fork-ts-checker-webpack-plugin": "0.4.10",
"friendly-errors-webpack-plugin": "1.6.1",
"graphql-tag": "^2.10.3",
"graphql-tools": "4.0.3",
"hulk-editor": "craigspaeth/hulk",
"husky": "3.0.5",
Expand All @@ -337,8 +338,8 @@
"jest": "24.9.0",
"jest-coffee-preprocessor": "1.0.0",
"jest-junit": "6.4.0",
"jest-raw-loader": "1.0.1",
"jest-styled-components": "7.0.0-2",
"jest-transform-graphql": "^2.1.0",
"jsdom": "11.6.2",
"jsdom-global": "3.0.2",
"json-loader": "0.5.7",
Expand Down
28 changes: 10 additions & 18 deletions src/desktop/analytics/inquiry_questionnaire.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,15 @@ const analytics = window.analytics
function getTrackingOptions() {
var trackingOptions = {}

// FIXME: Remove once A/B test completes
if (sd.CLIENT_NAVIGATION_V5 === "experiment") {
const referrer = window.analytics.__artsyReferrer
// Grab referrer from our trackingMiddleware in Reaction, since we're in a
// single-page-app context and the value will need to be refreshed on route
// change. See: https://github.com/artsy/reaction/blob/master/src/Artsy/Analytics/trackingMiddleware.ts
if (referrer) {
trackingOptions = {
page: {
referrer,
},
}
const referrer = window.analytics.__artsyReferrer
// Grab referrer from our trackingMiddleware in Reaction, since we're in a
// single-page-app context and the value will need to be refreshed on route
// change. See: https://github.com/artsy/reaction/blob/master/src/Artsy/Analytics/trackingMiddleware.ts
if (referrer) {
trackingOptions = {
page: {
referrer,
},
}
}

Expand Down Expand Up @@ -57,12 +54,7 @@ const analytics = window.analytics
// _per session_, and therefore can't rely on `once`, as subsequent
// inquiries would then not get tracked as there's no "hard jumps"
// between pages. See: https://github.com/artsy/force/pull/5232
// FIXME: Remove once A/B test completes
if (window.sd.CLIENT_NAVIGATION_V5 === "experiment") {
analyticsHooks.on(namespace(name), handler)
} else {
analyticsHooks.once(namespace(name), handler)
}
analyticsHooks.on(namespace(name), handler)
}

// DOM events
Expand Down
21 changes: 9 additions & 12 deletions src/desktop/analytics/main_layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,15 @@ class PageTimeTracker {
this.timer = setTimeout(() => {
let trackingOptions = {}

// FIXME: Remove once A/B test completes
if (sd.CLIENT_NAVIGATION_V5 === "experiment") {
const referrer = window.analytics.__artsyReferrer
// Grab referrer from our trackingMiddleware in Reaction, since we're in a
// single-page-app context and the value will need to be refreshed on route
// change. See: https://github.com/artsy/reaction/blob/master/src/Artsy/Analytics/trackingMiddleware.ts
if (referrer) {
trackingOptions = {
page: {
referrer,
},
}
const referrer = window.analytics.__artsyReferrer
// Grab referrer from our trackingMiddleware in Reaction, since we're in a
// single-page-app context and the value will need to be refreshed on route
// change. See: https://github.com/artsy/reaction/blob/master/src/Artsy/Analytics/trackingMiddleware.ts
if (referrer) {
trackingOptions = {
page: {
referrer,
},
}
}

Expand Down
35 changes: 0 additions & 35 deletions src/desktop/apps/artist/client.tsx

This file was deleted.

68 changes: 0 additions & 68 deletions src/desktop/apps/artist/server.tsx

This file was deleted.

5 changes: 5 additions & 0 deletions src/desktop/apps/artsy-v2/apps/artist/artistClient.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { setupArtistSignUpModal } from "desktop/components/artistSignupModal/artistSignupModal"

export const artistClient = () => {
setupArtistSignUpModal()
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export const artworkClient = () => {
const Artwork = require("desktop/models/artwork.coffee")
const ArtworkInquiry = require("desktop/models/artwork_inquiry.coffee")
const openInquiryQuestionnaireFor = require("desktop/components/inquiry_questionnaire/index.coffee")
const openAuctionBuyerPremium = require("desktop/apps/artwork/components/buyers_premium/index.coffee")
const openAuctionBuyerPremium = require("desktop/components/artworkBuyersPremium/index.coffee")
const ViewInRoomView = require("desktop/components/view_in_room/view.coffee")
const $ = require("jquery")
const mediator = require("desktop/lib/mediator.coffee")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,7 @@ export const handleArtworkImageDownload = async (req, res, next) => {
if (req.user) {
imageRequest.set("X-ACCESS-TOKEN", req.user.get("accessToken"))
}
req
.pipe(
imageRequest,
{ end: false }
)
.pipe(res)
req.pipe(imageRequest, { end: false }).pipe(res)
} else {
const error: any = new Error("Not authorized to download this image.")
error.status = 403
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ export const searchMiddleware = async (req, res, next) => {

const layout = await stitch({
basePath: __dirname,
layout:
"../../../../components/main_layout/templates/experimental_app_shell.jade",
layout: "../../../../components/main_layout/templates/artsy_v2.jade",
blocks: {
loadingComponent: _props => {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ const mediator = require("desktop/lib/mediator.coffee")
buildClientApp({
routes: getAppRoutes(),
context: {
EXPERIMENTAL_APP_SHELL: true,
user: sd.CURRENT_USER,
mediator,
} as any,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,6 @@ app.get("/artwork/:artworkID/download/:filename", handleArtworkImageDownload)
*/
app.get(
"/*",
(_req, res, next) => {
const isExperiment = res.locals.sd.CLIENT_NAVIGATION_V5 === "experiment"

if (!isExperiment) {
return next("route")
}
return next()
},

userRequiredMiddleware,

/**
Expand Down Expand Up @@ -58,9 +49,7 @@ app.get(
bodyHTML,
headTags,
} = await buildServerApp({
context: buildServerAppContext(req, res, {
EXPERIMENTAL_APP_SHELL: true,
}),
context: buildServerAppContext(req, res),
routes: getAppRoutes(),
url: req.url,
userAgent: req.header("User-Agent"),
Expand All @@ -77,8 +66,7 @@ app.get(

const layout = await stitch({
basePath: __dirname,
layout:
"../../components/main_layout/templates/experimental_app_shell.jade",
layout: "../../components/main_layout/templates/artsy_v2.jade",
blocks: {
body: bodyHTML,
head: () => <>{headTags}</>,
Expand Down
Loading