Skip to content

Commit

Permalink
feat(home view): allow section declaration to specify a minimum requi…
Browse files Browse the repository at this point in the history
…red Eigen version (#6297)

* chore(deps): update node types to match current 18.15.0

In order to use Array.at() in the next commit, I need to update our
Node types to match the actual version of Node we are using.

I also had to tweak one file to conform to the updated types.

* feat: add a utility for parsing Eigen version numbers

* feat: add a predicate that asserts a minimum version number

* feat: allow section declaration to specify a minimum Eigen version

* feat: enforce the minimum Eigen version

But only if there is an actual Eigen version to consider.

Otherwise we are presumably coming from a different client
altogether, and would not want to gate this section.

* refactor: fix filename casing

* test: fix desktop UA string

* refactor: assume UA is string, not array

* test: add an Android case for good measure
  • Loading branch information
anandaroop authored Dec 11, 2024
1 parent 9cab472 commit 812914b
Show file tree
Hide file tree
Showing 9 changed files with 262 additions and 6 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
"@types/invariant": "2.2.29",
"@types/jest": "^29.5.14",
"@types/lodash": "4.14.86",
"@types/node": "14.17.0",
"@types/node": "18.15.0",
"@types/node-fetch": "2.1.7",
"@types/qs": "6.5.1",
"@types/request": "2.0.8",
Expand Down
124 changes: 124 additions & 0 deletions src/lib/__tests__/semanticVersioning.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
import {
SemanticVersionNumber,
getEigenVersionNumber,
isAtLeastVersion,
} from "../semanticVersioning"

describe("getEigenVersionNumber", () => {
describe("with a typical Eigen user agent string", () => {
it("parses the version number", () => {
expect(
getEigenVersionNumber(
"unknown iOS/18.1.1 Artsy-Mobile/8.59.0 Eigen/2024.12.10.06/8.59.0"
)
).toEqual({
major: 8,
minor: 59,
patch: 0,
})
})
})

describe("with a __DEV__ Eigen user agent string", () => {
it("parses the version number from iOS", () => {
expect(
getEigenVersionNumber(
"Artsy-Mobile ios null/null Artsy-Mobile/8.59.0 Eigen/null/8.59.0"
)
).toEqual({
major: 8,
minor: 59,
patch: 0,
})
})

it("parses the version number from Android", () => {
expect(
getEigenVersionNumber(
"Artsy-Mobile android null/null Artsy-Mobile/8.59.0 Eigen/null/8.59.0"
)
).toEqual({
major: 8,
minor: 59,
patch: 0,
})
})
})
describe("with a typical mobile browser user agent string", () => {
it("returns null", () => {
expect(
getEigenVersionNumber(
"Mozilla/5.0 (iPhone; CPU iPhone OS 18_1_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.1.1 Mobile/15E148 Safari/604.1"
)
).toBeNull()
})
})

describe("with a typical desktop browser user agent string", () => {
it("returns null", () => {
expect(
getEigenVersionNumber(
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.1.1 Safari/605.1.15"
)
).toBeNull()
})
})

describe("with an empty user agent string", () => {
it("returns null", () => {
expect(getEigenVersionNumber("")).toBeNull()
})
})

describe("with a gibberish user agent string", () => {
it("returns null", () => {
expect(
getEigenVersionNumber("It’s 2024 so I'm just going to say: Moo Deng")
).toBeNull()
})
})
})

describe("isAtLeastVersion", () => {
const candidate: SemanticVersionNumber = {
major: 42,
minor: 42,
patch: 42,
}

it("returns true if the candidate version is identical to the minimum version", () => {
expect(
isAtLeastVersion(candidate, { major: 42, minor: 42, patch: 42 })
).toBe(true)
})

it("tests the candidate major version", () => {
expect(
isAtLeastVersion(candidate, { major: 41, minor: 42, patch: 42 })
).toBe(true)

expect(
isAtLeastVersion(candidate, { major: 43, minor: 42, patch: 42 })
).toBe(false)
})

it("tests the candidate minor version", () => {
expect(
isAtLeastVersion(candidate, { major: 42, minor: 41, patch: 42 })
).toBe(true)

expect(
isAtLeastVersion(candidate, { major: 42, minor: 43, patch: 42 })
).toBe(false)
})

it("tests the candidate patch version", () => {
expect(
isAtLeastVersion(candidate, { major: 42, minor: 42, patch: 41 })
).toBe(true)

expect(
isAtLeastVersion(candidate, { major: 42, minor: 42, patch: 43 })
).toBe(false)
})
})
3 changes: 2 additions & 1 deletion src/lib/fetchPersistedQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ export const fetchPersistedQuery: RequestHandler = (req, res, next) => {
} else {
const message = `Unable to serve persisted query with ID ${documentID}`
error(message)
return res.status(404).send(message).end()
res.status(404).send(message).end()
return
}
}
next()
Expand Down
36 changes: 36 additions & 0 deletions src/lib/semanticVersioning.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
export type SemanticVersionNumber = {
major: number
minor: number
patch: number
}

export function getEigenVersionNumber(
userAgent: string
): SemanticVersionNumber | null {
if (!userAgent) return null
if (!userAgent.includes("Artsy-Mobile")) return null

const parts = userAgent.split("/")
const version = parts.at(-1)

if (!version) return null

const [major, minor, patch] = version.split(".").map(Number)

return { major, minor, patch }
}

export function isAtLeastVersion(
version: SemanticVersionNumber,
atLeast: SemanticVersionNumber
): boolean {
const { major, minor, patch } = atLeast

if (version.major > major) return true
if (version.major < major) return false

if (version.minor > minor) return true
if (version.minor < minor) return false

return version.patch >= patch
}
Original file line number Diff line number Diff line change
Expand Up @@ -121,4 +121,81 @@ describe("isSectionDisplayable", () => {
).toBe(false)
})
})

describe("with a section that requires a minimum Eigen version", () => {
it("returns false if the user's Eigen version is below the minimum", () => {
const section: Partial<HomeViewSection> = {
requiresAuthentication: false,
minimumEigenVersion: { major: 9, minor: 0, patch: 0 },
}

const context: Partial<ResolverContext> = {
userAgent:
"unknown iOS/18.1.1 Artsy-Mobile/8.59.0 Eigen/2024.12.10.06/8.59.0",
}

expect(
isSectionDisplayable(
section as HomeViewSection,
context as ResolverContext
)
).toBe(false)
})

it("returns true if the user's Eigen version is equal to the minimum", () => {
const section: Partial<HomeViewSection> = {
requiresAuthentication: false,
minimumEigenVersion: { major: 8, minor: 59, patch: 0 },
}

const context: Partial<ResolverContext> = {
userAgent:
"unknown iOS/18.1.1 Artsy-Mobile/8.59.0 Eigen/2024.12.10.06/8.59.0",
}

expect(
isSectionDisplayable(
section as HomeViewSection,
context as ResolverContext
)
).toBe(true)
})

it("returns true if the user's Eigen version is above the minimum", () => {
const section: Partial<HomeViewSection> = {
requiresAuthentication: false,
minimumEigenVersion: { major: 8, minor: 0, patch: 0 },
}

const context: Partial<ResolverContext> = {
userAgent:
"unknown iOS/18.1.1 Artsy-Mobile/8.59.0 Eigen/2024.12.10.06/8.59.0",
}

expect(
isSectionDisplayable(
section as HomeViewSection,
context as ResolverContext
)
).toBe(true)
})

it("returns true if an Eigen version is not recognized", () => {
const section: Partial<HomeViewSection> = {
requiresAuthentication: false,
minimumEigenVersion: { major: 8, minor: 0, patch: 0 },
}

const context: Partial<ResolverContext> = {
userAgent: "Hi it's me, Moo Deng, again",
}

expect(
isSectionDisplayable(
section as HomeViewSection,
context as ResolverContext
)
).toBe(true)
})
})
})
14 changes: 14 additions & 0 deletions src/schema/v2/homeView/helpers/isSectionDisplayable.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { ResolverContext } from "types/graphql"
import { HomeViewSection } from "../sections"
import { isFeatureFlagEnabled } from "lib/featureFlags"
import { getEigenVersionNumber, isAtLeastVersion } from "lib/semanticVersioning"

/**
* Determine if an individual section can be displayed, considering the current
Expand All @@ -27,6 +28,19 @@ export function isSectionDisplayable(
})
}

// minimum Eigen version
if (isDisplayable && section.minimumEigenVersion) {
const actualEigenVersion = getEigenVersionNumber(
context.userAgent as string
)
if (actualEigenVersion) {
isDisplayable = isAtLeastVersion(
actualEigenVersion,
section.minimumEigenVersion
)
}
}

// section's display pre-check
if (typeof section.shouldBeDisplayed === "function") {
isDisplayable = isDisplayable && section?.shouldBeDisplayed(context)
Expand Down
2 changes: 2 additions & 0 deletions src/schema/v2/homeView/sections/InfiniteDiscovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ export const InfiniteDiscovery: HomeViewSection = {
featureFlag: "diamond_home-view-infinite-discovery",
id: "home-view-section-infinite-discovery",
requiresAuthentication: true,
// TODO: update this to match the first release that can support Infinite Discovery
minimumEigenVersion: { major: 8, minor: 59, patch: 0 },
ownerType: OwnerType.infiniteDiscovery,
type: HomeViewSectionTypeNames.HomeViewSectionCard,

Expand Down
2 changes: 2 additions & 0 deletions src/schema/v2/homeView/sections/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { Tasks } from "./Tasks"
import { TrendingArtists } from "./TrendingArtists"
import { ViewingRooms } from "./ViewingRooms"
import { InfiniteDiscovery } from "./InfiniteDiscovery"
import { SemanticVersionNumber } from "lib/semanticVersioning"

type MaybeResolved<T> =
| T
Expand All @@ -46,6 +47,7 @@ export type HomeViewSection = {
}
ownerType?: OwnerType
requiresAuthentication: boolean
minimumEigenVersion?: SemanticVersionNumber
shouldBeDisplayed?: (context: ResolverContext) => boolean
resolver?: GraphQLFieldResolver<any, ResolverContext>
type: keyof typeof HomeViewSectionTypeNames
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2263,10 +2263,10 @@
dependencies:
undici-types "~6.19.2"

"@types/node@14.17.0":
version "14.17.0"
resolved "https://registry.yarnpkg.com/@types/node/-/node-14.17.0.tgz#3ba770047723b3eeb8dc9fca02cce8a7fb6378da"
integrity sha512-w8VZUN/f7SSbvVReb9SWp6cJFevxb4/nkG65yLAya//98WgocKm5PLDAtSs5CtJJJM+kHmJjO/6mmYW4MHShZA==
"@types/node@18.15.0":
version "18.15.0"
resolved "https://registry.yarnpkg.com/@types/node/-/node-18.15.0.tgz#286a65e3fdffd691e170541e6ecb0410b16a38be"
integrity sha512-z6nr0TTEOBGkzLGmbypWOGnpSpSIBorEhC4L+4HeQ2iezKCi4f77kyslRwvHeNitymGQ+oFyIWGP96l/DPSV9w==

"@types/node@^10.12.18":
version "10.17.60"
Expand Down

0 comments on commit 812914b

Please sign in to comment.