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

feat: add signed as filter param to filterArtworksConnection #15111

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
304 changes: 172 additions & 132 deletions data/schema.graphql

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ import { MaterialsFilter } from "Components/ArtworkFilter/ArtworkFilters/Materia
import { MediumFilter } from "Components/ArtworkFilter/ArtworkFilters/MediumFilter"
import { PartnersFilter } from "Components/ArtworkFilter/ArtworkFilters/PartnersFilter"
import { PriceRangeFilter } from "Components/ArtworkFilter/ArtworkFilters/PriceRangeFilter"
import { SignedFilter } from "Components/ArtworkFilter/ArtworkFilters/SignedFilter"
import { SizeFilter } from "Components/ArtworkFilter/ArtworkFilters/SizeFilter"
import { TimePeriodFilter } from "Components/ArtworkFilter/ArtworkFilters/TimePeriodFilter"
import { WaysToBuyFilter } from "Components/ArtworkFilter/ArtworkFilters/WaysToBuyFilter"
import { useFeatureFlag } from "System/Hooks/useFeatureFlag"
import { useSystemContext } from "System/Hooks/useSystemContext"

type ArtistArtworkFiltersProps = {}
Expand All @@ -22,6 +24,9 @@ export const ArtistArtworkFilters: React.FC<
React.PropsWithChildren<ArtistArtworkFiltersProps>
> = props => {
const { user } = useSystemContext()
const enableShowOnlySignedArtworksFilter = useFeatureFlag(
"onyx_only_signed_artworks_filter",
)

return (
<Join separator={<Spacer y={4} />}>
Expand All @@ -40,6 +45,7 @@ export const ArtistArtworkFilters: React.FC<
<TimePeriodFilter />
<ColorFilter />
<PartnersFilter />
{enableShowOnlySignedArtworksFilter && <SignedFilter />}
</Join>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ import { MaterialsFilter } from "Components/ArtworkFilter/ArtworkFilters/Materia
import { MediumFilter } from "Components/ArtworkFilter/ArtworkFilters/MediumFilter"
import { PartnersFilter } from "Components/ArtworkFilter/ArtworkFilters/PartnersFilter"
import { PriceRangeFilter } from "Components/ArtworkFilter/ArtworkFilters/PriceRangeFilter"
import { SignedFilter } from "Components/ArtworkFilter/ArtworkFilters/SignedFilter"
import { SizeFilter } from "Components/ArtworkFilter/ArtworkFilters/SizeFilter"
import { TimePeriodFilter } from "Components/ArtworkFilter/ArtworkFilters/TimePeriodFilter"
import { WaysToBuyFilter } from "Components/ArtworkFilter/ArtworkFilters/WaysToBuyFilter"
import { updateUrl } from "Components/ArtworkFilter/Utils/urlBuilder"
import { useFeatureFlag } from "System/Hooks/useFeatureFlag"
import { useRouter } from "System/Hooks/useRouter"
import { useSystemContext } from "System/Hooks/useSystemContext"
import { __internal__useMatchMedia } from "Utils/Hooks/useMatchMedia"
Expand All @@ -41,6 +43,10 @@ interface CollectionArtworksFilterProps {
export const CollectionArtworksFilter: React.FC<
React.PropsWithChildren<CollectionArtworksFilterProps>
> = props => {
const enableShowOnlySignedArtworksFilter = useFeatureFlag(
"onyx_only_signed_artworks_filter",
)

const { relay, collection, aggregations, counts } = props
const { slug, query } = collection
const isArtistCollection = query?.artistIDs?.length === 1
Expand All @@ -64,6 +70,7 @@ export const CollectionArtworksFilter: React.FC<
<TimePeriodFilter expanded />
<ColorFilter expanded />
<PartnersFilter expanded />
{enableShowOnlySignedArtworksFilter && <SignedFilter expanded />}
</Join>
)

Expand Down
6 changes: 6 additions & 0 deletions src/Apps/Search/Components/SearchResultsArtworksFilters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,16 @@ import { MaterialsFilter } from "Components/ArtworkFilter/ArtworkFilters/Materia
import { MediumFilter } from "Components/ArtworkFilter/ArtworkFilters/MediumFilter"
import { PartnersFilter } from "Components/ArtworkFilter/ArtworkFilters/PartnersFilter"
import { PriceRangeFilter } from "Components/ArtworkFilter/ArtworkFilters/PriceRangeFilter"
import { SignedFilter } from "Components/ArtworkFilter/ArtworkFilters/SignedFilter"
import { SizeFilter } from "Components/ArtworkFilter/ArtworkFilters/SizeFilter"
import { TimePeriodFilter } from "Components/ArtworkFilter/ArtworkFilters/TimePeriodFilter"
import { WaysToBuyFilter } from "Components/ArtworkFilter/ArtworkFilters/WaysToBuyFilter"
import { useFeatureFlag } from "System/Hooks/useFeatureFlag"

export const SearchResultsArtworksFilters = () => {
const enableShowOnlySignedArtworksFilter = useFeatureFlag(
"onyx_only_signed_artworks_filter",
)
return (
<Join separator={<Spacer y={4} />}>
<ArtistsFilter expanded />
Expand All @@ -29,6 +34,7 @@ export const SearchResultsArtworksFilters = () => {
<TimePeriodFilter expanded />
<ColorFilter expanded />
<PartnersFilter expanded />
{enableShowOnlySignedArtworksFilter && <SignedFilter expanded />}
</Join>
)
}
3 changes: 2 additions & 1 deletion src/Components/Alert/AlertProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,12 @@ export const AlertProvider: FC<React.PropsWithChildren<AlertProviderProps>> = ({
if (isEditMode || isAlertArtworksView) return
const criteria = getAllowedSearchCriteria(initialCriteria ?? {})

// `forSale` is allowed as a filter criterion,
// `forSale` and `signed` are allowed as a filter criterion,
// but NOT as an alert criterion, so we remove it.
// (Alerts, by definition, stipulate forSale=true
// when they are created in Gravity.)
delete criteria.forSale
delete criteria.signed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Same thought as here)


dispatch({ type: "SET_CRITERIA", payload: criteria })
}, [initialCriteria, isEditMode, isAlertArtworksView])
Expand Down
11 changes: 11 additions & 0 deletions src/Components/ArtworkFilter/ArtworkFilterContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export enum FilterParamName {
medium = "medium",
partnerIDs = "partnerIDs",
priceRange = "priceRange",
signed = "signed",
sizes = "sizes",
sort = "sort",
timePeriod = "majorPeriods",
Expand Down Expand Up @@ -143,6 +144,7 @@ export enum SelectedFiltersCountsLabels {
medium = "medium",
partnerIDs = "partnerIDs",
priceRange = "priceRange",
signed = "signed",
sizes = "sizes",
sort = "sort",
timePeriod = "majorPeriods",
Expand Down Expand Up @@ -486,6 +488,7 @@ const artworkFilterReducer = (
"includeArtworksByFollowedArtists",
"inquireableOnly",
"offerable",
"signed",
]
booleanFilterTypes.forEach(filter => {
if (name === filter) {
Expand Down Expand Up @@ -537,6 +540,7 @@ const artworkFilterReducer = (
"inquireableOnly",
"offerable",
"partnerID",
"signed",
]
filters.forEach(filter => {
if (name === filter) {
Expand Down Expand Up @@ -648,6 +652,13 @@ export const getSelectedFiltersCounts = (
break
}

case paramName === FilterParamName.signed: {
if (paramValue) {
counts[paramName] = 1
}
break
}

default: {
counts[paramName] = 1
}
Expand Down
1 change: 1 addition & 0 deletions src/Components/ArtworkFilter/ArtworkFilterTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export interface ArtworkFilters extends MultiSelectArtworkFilters {
page?: number
partnerID?: string
priceRange?: string
signed?: boolean
sort?: string
term?: string
width?: string
Expand Down
44 changes: 44 additions & 0 deletions src/Components/ArtworkFilter/ArtworkFilters/SignedFilter.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { Checkbox } from "@artsy/palette"
import {
SelectedFiltersCountsLabels,
useArtworkFilterContext,
useCurrentlySelectedFilters,
} from "Components/ArtworkFilter/ArtworkFilterContext"
import { FilterExpandable } from "Components/ArtworkFilter/ArtworkFilters/FilterExpandable"
import { useFilterLabelCountByKey } from "Components/ArtworkFilter/Utils/useFilterLabelCountByKey"

interface SignedFilterProps {
expanded?: boolean
}

export const SignedFilter: React.FC<SignedFilterProps> = ({ expanded }) => {
const { unsetFilter, setFilter } = useArtworkFilterContext()
const currentSelectedFilters = useCurrentlySelectedFilters()

const filtersCount = useFilterLabelCountByKey(
SelectedFiltersCountsLabels.signed,
)
const label = `Signed${filtersCount}`

const isSelected = currentSelectedFilters?.signed

const handleOnSelect = (selected: boolean) => {
if (selected) {
setFilter("signed", true)
} else {
unsetFilter("signed")
}
}

return (
<FilterExpandable label={label} expanded={isSelected || expanded}>
<Checkbox
selected={!!currentSelectedFilters?.signed}
onSelect={handleOnSelect}
my={1}
>
Only signed works
</Checkbox>
</FilterExpandable>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import { screen } from "@testing-library/react"
import userEvent from "@testing-library/user-event"
import { useArtworkFilterContext } from "Components/ArtworkFilter/ArtworkFilterContext"
import { SignedFilter } from "Components/ArtworkFilter/ArtworkFilters/SignedFilter"
import {
createArtworkFilterTestRenderer,
currentArtworkFilterContext,
} from "Components/ArtworkFilter/ArtworkFilters/__tests__/Utils"
import { __internal__useMatchMedia } from "Utils/Hooks/useMatchMedia"
import { useEffect } from "react"

jest.mock("Utils/Hooks/useMatchMedia")

const render = createArtworkFilterTestRenderer()

describe(SignedFilter, () => {
it("renders a signed toggle", () => {
render(<SignedFilter expanded />)
expect(screen.getByText("Only signed works")).toBeInTheDocument()
})

it("updates context on filter change", () => {
render(<SignedFilter expanded />)
expect(currentArtworkFilterContext().filters?.signed).toBeFalsy()

userEvent.click(screen.getAllByRole("checkbox")[0])
expect(currentArtworkFilterContext().filters?.signed).toBeTruthy()

userEvent.click(screen.getAllByRole("checkbox")[0])
expect(currentArtworkFilterContext().filters?.signed).toBeNull()
})

it("clears local input state after Clear All", () => {
render(<SignedFilter expanded />)
userEvent.click(screen.getAllByRole("checkbox")[0])
expect(currentArtworkFilterContext().filters?.signed).toBeTruthy()

userEvent.click(screen.getByText("Clear all"))

expect(currentArtworkFilterContext().filters?.signed).toBeUndefined()
})

describe("mobile-specific behavior", () => {
beforeEach(() => {
// Simulate the media query that checks for xs size and returns true
;(__internal__useMatchMedia as jest.Mock).mockImplementation(() => true)

/*
* A test component that simulates the usage of
* the SignedFilter inside ArtworkFilterMobileOverlay
*/
const MobileVersionOfSignedFilter = () => {
const filterContext = useArtworkFilterContext()

// biome-ignore lint/correctness/useExhaustiveDependencies: <explanation>
useEffect(() => {
// on mount, initialize the staged filters
filterContext.setShouldStageFilterChanges?.(true)
if (filterContext.filters) {
filterContext.setStagedFilters?.(filterContext.filters)
}
}, [])

return <SignedFilter expanded />
}
render(<MobileVersionOfSignedFilter />)
})

afterEach(() => {
jest.clearAllMocks()
})

it("stages the filter change instead of applying", () => {
expect(currentArtworkFilterContext().filters?.signed).toBeFalsy()

userEvent.click(screen.getAllByRole("checkbox")[0])

expect(currentArtworkFilterContext().filters?.signed).toBeFalsy()
expect(currentArtworkFilterContext().stagedFilters?.signed).toBeTruthy()
})

it("displays a filter count inline", () => {
expect(screen.getByText("Signed")).toBeInTheDocument()
expect(screen.queryByText("Signed • 1")).not.toBeInTheDocument()

userEvent.click(screen.getAllByRole("checkbox")[0])

expect(screen.getByText("Signed • 1")).toBeInTheDocument()
})
})

describe("the `expanded` prop", () => {
it("hides the filter controls when not set", () => {
render(<SignedFilter />)
expect(screen.queryAllByRole("checkbox").length).toBe(0)
})

it("hides the filter controls when `false`", () => {
render(<SignedFilter expanded={false} />)
expect(screen.queryAllByRole("checkbox").length).toBe(0)
})

it("shows the filter controls when `true`", () => {
render(<SignedFilter expanded />)
expect(screen.queryAllByRole("checkbox").length).not.toBe(0)
})
})
})
6 changes: 6 additions & 0 deletions src/Components/ArtworkFilter/ArtworkFilters/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { Join, Spacer } from "@artsy/palette"
import { AvailabilityFilter } from "Components/ArtworkFilter/ArtworkFilters/AvailabilityFilter"
import { KeywordFilter } from "Components/ArtworkFilter/ArtworkFilters/KeywordFilter"
import { SignedFilter } from "Components/ArtworkFilter/ArtworkFilters/SignedFilter"
import { useFeatureFlag } from "System/Hooks/useFeatureFlag"
import type * as React from "react"
import { ArtistNationalityFilter } from "./ArtistNationalityFilter"
import { ArtistsFilter } from "./ArtistsFilter"
Expand All @@ -23,6 +25,9 @@ interface ArtworkFiltersProps {
export const ArtworkFilters: React.FC<
React.PropsWithChildren<ArtworkFiltersProps>
> = props => {
const enableShowOnlySignedArtworksFilter = useFeatureFlag(
"onyx_only_signed_artworks_filter",
)
const { user } = props

return (
Expand All @@ -41,6 +46,7 @@ export const ArtworkFilters: React.FC<
<TimePeriodFilter expanded />
<ColorFilter expanded />
<PartnersFilter expanded />
{enableShowOnlySignedArtworksFilter && <SignedFilter expanded />}
</Join>
)
}
2 changes: 2 additions & 0 deletions src/Components/ArtworkFilter/Utils/allowedFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const BOOLEAN_INPUT_ARGS = [
"keywordMatchExact",
"marketable",
"offerable",
"signed",
]

/**
Expand Down Expand Up @@ -87,6 +88,7 @@ const SUPPORTED_INPUT_ARGS = [
"periods",
"priceRange",
"saleID",
"signed",
"size",
"sizes",
"sort",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,24 @@ describe("extractPillsFromDefaultCriteria", () => {
},
])
})

it("should support signed", () => {
const result = extractPillsFromDefaultCriteria({
signed: {
displayValue: "Signed",
value: true,
},
})

expect(result).toEqual([
{
isDefault: true,
displayValue: "Signed",
value: "true",
field: "signed",
},
])
})
})

describe("excludeDefaultCriteria", () => {
Expand Down
8 changes: 8 additions & 0 deletions src/Components/SavedSearchAlert/Utils/extractPills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,14 @@ export const extractPillsFromCriteria = ({
}
break
}
case "signed": {
result = {
field: paramName,
value: paramValue,
displayValue: "Signed",
}
break
}
default: {
result = null
}
Expand Down
1 change: 1 addition & 0 deletions src/Components/SavedSearchAlert/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const allowedSearchCriteriaKeys = [
"offerable",
"partnerIDs",
"priceRange",
"signed",
"sizes",
"width",
]
Expand Down
Loading