Merge pull request #5648 from artsyjian/auction-results-track-current…

…-page-number-in-filter-state

src/v2/Apps/Artist/Routes/AuctionResults/ArtistAuctionResultItem.tsx

@@ -34,7 +34,6 @@ export interface Props extends SystemContextProps {
   mediator?: Mediator
   lastChild: boolean
   filtersAtDefault: boolean
-  paginated: boolean
 }
 
 const FullWidthBorderBox = styled(BorderBox)`
@@ -196,8 +195,7 @@ const LargeAuctionItem: SFC<Props> = props => {
               props.user,
               props.mediator,
               "lg",
-              props.filtersAtDefault,
-              props.paginated
+              props.filtersAtDefault
             )}
           </Flex>
           <Flex width="10%" justifyContent="flex-end">
@@ -241,8 +239,7 @@ const ExtraSmallAuctionItem: SFC<Props> = props => {
           props.user,
           props.mediator,
           "xs",
-          props.filtersAtDefault,
-          props.paginated
+          props.filtersAtDefault
         )}
         <Sans size="2" weight="medium" color="black60">
           {title}
@@ -326,16 +323,12 @@ const renderPricing = (
   user,
   mediator,
   size,
-  filtersAtDefault,
-  paginated
+  filtersAtDefault
 ) => {
   const textSize = size === "xs" ? "2" : "3t"
 
-  // If user is logged in we show prices. Otherwise we show prices only for the default view - on page 1 and filters not changed.
-  // Ideally we get current page number from filter context 'page' property but somehow it is always '1'.
-  // So we resort to pagination detection. If user has paginated at all, prices will be hidden. even if user comes back to page 1.
-  // TODO: Fix filter context so its 'page' property has the current page number, then change this code.
-  if (user || (filtersAtDefault && !paginated)) {
+  // If user is logged in we show prices. Otherwise we show prices only when filters at default.
+  if (user || filtersAtDefault) {
     const textAlign = size === "xs" ? "left" : "right"
     const dateOfSale = DateTime.fromISO(saleDate)
     const now = DateTime.local()
@@ -436,12 +429,11 @@ const renderRealizedPrice = (
   user,
   mediator,
   size,
-  filtersAtDefault,
-  paginated
+  filtersAtDefault
 ) => {
   const justifyContent = size === "xs" ? "flex-start" : "flex-end"
-  // Show prices if user is logged in. Otherwise, show prices only on default view - filters at default and no pagination has happened.
-  if (user || (filtersAtDefault && !paginated)) {
+  // Show prices if user is logged in. Otherwise, show prices only when filters at default.
+  if (user || filtersAtDefault) {
     return (
       <Flex justifyContent={justifyContent}>
         {estimatedPrice && (
@@ -551,8 +543,7 @@ const renderLargeCollapse = (props, user, mediator, filtersAtDefault) => {
               user,
               mediator,
               "lg",
-              filtersAtDefault,
-              props.paginated
+              filtersAtDefault
             )}
           </Col>
         </Row>
@@ -638,8 +629,7 @@ const renderSmallCollapse = (props, user, mediator, filtersAtDefault) => {
               user,
               mediator,
               "xs",
-              filtersAtDefault,
-              props.paginated
+              filtersAtDefault
             )}
           </Col>
         </Row>

src/v2/Apps/Artist/Routes/AuctionResults/ArtistAuctionResults.tsx

@@ -6,7 +6,6 @@ import { RelayRefetchProp, createRefetchContainer, graphql } from "react-relay"
 import useDeepCompareEffect from "use-deep-compare-effect"
 import { AuctionResultItemFragmentContainer as AuctionResultItem } from "./ArtistAuctionResultItem"
 import { TableSidebar } from "./Components/TableSidebar"
-
 import { ContextModule } from "@artsy/cohesion"
 import { Box, Spacer } from "@artsy/palette"
 import { AnalyticsSchema } from "v2/Artsy"
@@ -40,56 +39,18 @@ const AuctionResultsContainer: React.FC<AuctionResultsProps> = ({
   relay,
 }) => {
   const filterContext = useAuctionResultsFilterContext()
-
-  const {
-    sort,
-    organizations,
-    categories,
-    sizes,
-    createdAfterYear,
-    createdBeforeYear,
-    allowEmptyCreatedDates,
-  } = filterContext.filters
-
-  // Detect whether user has paginated at all.
-  const [paginated, togglePaginated] = useState(false)
+  const { pageInfo } = artist.auctionResultsConnection
+  const { hasNextPage, endCursor } = pageInfo
 
   const loadNext = () => {
-    const { hasNextPage, endCursor } = pageInfo
-
+    const nextPageNum = filterContext.filters.pageAndCursor.page + 1
     if (hasNextPage) {
-      loadAfter(endCursor)
+      loadPage(endCursor, nextPageNum)
     }
   }
 
-  const loadAfter = cursor => {
-    setIsLoading(true)
-    togglePaginated(true)
-
-    relay.refetch(
-      {
-        first: PAGE_SIZE,
-        after: cursor,
-        artistID: artist.slug,
-        before: null,
-        last: null,
-        organizations,
-        categories,
-        sizes,
-        sort,
-        createdBeforeYear,
-        createdAfterYear,
-        allowEmptyCreatedDates,
-      },
-      null,
-      error => {
-        setIsLoading(false)
-
-        if (error) {
-          logger.error(error)
-        }
-      }
-    )
+  const loadPage = (cursor, pageNum) => {
+    filterContext.setFilter("pageAndCursor", { page: pageNum, cursor: cursor })
   }
 
   const [isLoading, setIsLoading] = useState(false)
@@ -110,6 +71,7 @@ const AuctionResultsContainer: React.FC<AuctionResultsProps> = ({
       ([filterKey, currentFilter]) => {
         const previousFilter = previousFilters[filterKey]
         const filtersHaveUpdated = !isEqual(currentFilter, previousFilter)
+
         if (filtersHaveUpdated) {
           fetchResults()
 
@@ -134,7 +96,7 @@ const AuctionResultsContainer: React.FC<AuctionResultsProps> = ({
     const relayParams = {
       first: PAGE_SIZE,
       artistID: artist.slug,
-      after: null,
+      after: filterContext.filters.pageAndCursor.cursor,
       before: null,
       last: null,
     }
@@ -153,7 +115,6 @@ const AuctionResultsContainer: React.FC<AuctionResultsProps> = ({
     })
   }
 
-  const { pageInfo } = artist.auctionResultsConnection
   const auctionResultsLength = artist.auctionResultsConnection.edges.length
 
   const resultList = (
@@ -166,7 +127,6 @@ const AuctionResultsContainer: React.FC<AuctionResultsProps> = ({
               auctionResult={node}
               lastChild={index === auctionResultsLength - 1}
               filtersAtDefault={filtersAtDefault}
-              paginated={paginated}
             />
           </React.Fragment>
         )
@@ -211,8 +171,8 @@ const AuctionResultsContainer: React.FC<AuctionResultsProps> = ({
             <Pagination
               hasNextPage={pageInfo.hasNextPage}
               pageCursors={artist.auctionResultsConnection.pageCursors}
-              onClick={loadAfter}
-              onNext={loadNext}
+              onClick={(_cursor, page) => loadPage(_cursor, page)}
+              onNext={() => loadNext()}
               scrollTo="#jumpto-ArtistHeader"
             />
           </Box>

src/v2/Apps/Artist/Routes/AuctionResults/AuctionResultsFilterContext.tsx

@@ -4,7 +4,7 @@ export interface AuctionResultsFilters {
   organizations?: string[]
   categories?: string[]
   sizes?: string[]
-  page?: number
+  pageAndCursor?: { page: number; cursor: string }
   sort?: string
   createdAfterYear?: number
   createdBeforeYear?: number
@@ -26,7 +26,7 @@ export const initialAuctionResultsFilterState: AuctionResultsFilters = {
   organizations: [],
   categories: [],
   sizes: [],
-  page: 1,
+  pageAndCursor: { page: 1, cursor: null },
   sort: "DATE_DESC",
   allowEmptyCreatedDates: true,
 }
@@ -157,9 +157,8 @@ const AuctionResultsFilterReducer = (
      */
     case "SET": {
       const { name, value } = action.payload
-
       const filterState: AuctionResultsFilters = {
-        page: 1,
+        pageAndCursor: { page: 1, cursor: null },
       }
 
       arrayFilterTypes.forEach(filter => {
@@ -171,17 +170,23 @@ const AuctionResultsFilterReducer = (
       // primitive filter types
       const primitiveFilterTypes: Array<keyof AuctionResultsFilters> = [
         "sort",
-        "page",
+        "pageAndCursor",
         "createdAfterYear",
         "createdBeforeYear",
         "allowEmptyCreatedDates",
       ]
+
       primitiveFilterTypes.forEach(filter => {
         if (name === filter) {
           filterState[name as any] = value
         }
       })
 
+      // do not allow a real cursor to be set for page 1. to agree with initial filter state.
+      if (filterState.pageAndCursor.page === 1) {
+        filterState.pageAndCursor.cursor = null
+      }
+
       if (name === "createdBeforeYear" && value) {
         if (!state.createdAfterYear) {
           filterState.createdAfterYear = state.earliestCreatedYear
@@ -213,7 +218,7 @@ const AuctionResultsFilterReducer = (
       const { name } = action.payload as { name: keyof AuctionResultsFilters }
 
       const filterState: AuctionResultsFilters = {
-        page: 1,
+        pageAndCursor: { page: 1, cursor: null },
       }
 
       const filters: Array<keyof AuctionResultsFilters> = ["sort"]

src/v2/Apps/Artist/Routes/AuctionResults/__tests__/AuctionResults.jest.tsx

@@ -108,28 +108,25 @@ describe("AuctionResults", () => {
         )
       })
       describe("pagination", () => {
-        it("triggers relay refetch with after", () => {
+        it("triggers relay refetch with after, and re-shows sign up to see price", done => {
           const pagination = wrapper.find("Pagination")
 
           pagination
             .find("button")
             .at(1)
             .simulate("click")
-          expect(refetchSpy).toHaveBeenCalledTimes(1)
-          expect(refetchSpy.mock.calls[0][0]).toEqual(
-            expect.objectContaining({
-              ...defaultRelayParams,
-              after: "YXJyYXljb25uZWN0aW9uOjk=",
-            })
-          )
-        })
 
-        it("re-shows sign up to see price", () => {
-          const pagination = wrapper.find("Pagination")
-          pagination
-            .find("button")
-            .at(2)
-            .simulate("click")
+          setTimeout(() => {
+            expect(refetchSpy).toHaveBeenCalledTimes(1)
+            expect(refetchSpy.mock.calls[0][0]).toEqual(
+              expect.objectContaining({
+                ...defaultRelayParams,
+                after: "YXJyYXljb25uZWN0aW9uOjk=",
+              })
+            )
+            done()
+          })
+
           wrapper.update()
           const html = wrapper.html()
           expect(html).toContain("Sign up to see price")
@@ -177,7 +174,7 @@ describe("AuctionResults", () => {
                 changed: { categories: ["Work on Paper"] },
                 current: {
                   categories: ["Work on Paper"],
-                  page: 1,
+                  pageAndCursor: { page: 1, cursor: null },
                   sort: "DATE_DESC",
                   organizations: [],
                   sizes: [],
@@ -280,7 +277,7 @@ describe("AuctionResults", () => {
                 changed: { sizes: ["MEDIUM"] },
                 current: {
                   sizes: ["MEDIUM"],
-                  page: 1,
+                  pageAndCursor: { page: 1, cursor: null },
                   sort: "DATE_DESC",
                   organizations: [],
                   categories: [],

src/v2/Apps/Artist/Routes/AuctionResults/__tests__/AuctionResultsFilterContext.jest.tsx

@@ -43,9 +43,12 @@ describe("AuctionResultsFilterContext", () => {
     it("#setFilter", done => {
       getWrapper()
       act(() => {
-        context.setFilter("page", 10)
+        context.setFilter("pageAndCursor", { page: 10, cursor: null })
         setTimeout(() => {
-          expect(context.filters.page).toEqual(10)
+          expect(context.filters.pageAndCursor).toEqual({
+            page: 10,
+            cursor: null,
+          })
           done()
         })
       })
@@ -54,14 +57,17 @@ describe("AuctionResultsFilterContext", () => {
     it("#setFilter resets pagination", done => {
       getWrapper({
         filters: {
-          page: 10,
+          pageAndCursor: { page: 10, cursor: null },
         },
       })
       act(() => {
-        expect(context.filters.page).toEqual(10)
+        expect(context.filters.pageAndCursor.page).toEqual(10)
         context.setFilter("sort", "relevant")
         setTimeout(() => {
-          expect(context.filters.page).toEqual(1)
+          expect(context.filters.pageAndCursor).toEqual({
+            page: 1,
+            cursor: null,
+          })
           done()
         })
       })
@@ -155,13 +161,16 @@ describe("AuctionResultsFilterContext", () => {
     it("#unsetFilter", done => {
       getWrapper()
       act(() => {
-        context.setFilter("page", 10)
+        context.setFilter("pageAndCursor", { page: 10, cursor: null })
         setTimeout(() => {
-          expect(context.filters.page).toEqual(10)
+          expect(context.filters.pageAndCursor.page).toEqual(10)
           act(() => {
-            context.unsetFilter("page")
+            context.unsetFilter("pageAndCursor")
             setTimeout(() => {
-              expect(context.filters.page).toEqual(1)
+              expect(context.filters.pageAndCursor).toEqual({
+                page: 1,
+                cursor: null,
+              })
               done()
             })
           })
@@ -172,15 +181,18 @@ describe("AuctionResultsFilterContext", () => {
     it("#unsetFilter resets pagination", done => {
       getWrapper({
         filters: {
-          page: 10,
+          pageAndCursor: { page: 10, cursor: null },
           sort: "relevant",
         },
       })
       act(() => {
-        expect(context.filters.page).toEqual(10)
+        expect(context.filters.pageAndCursor.page).toEqual(10)
         context.unsetFilter("sort")
         setTimeout(() => {
-          expect(context.filters.page).toEqual(1)
+          expect(context.filters.pageAndCursor).toEqual({
+            page: 1,
+            cursor: null,
+          })
           done()
         })
       })