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

Add pagination to Shopify integration #391

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
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
15 changes: 13 additions & 2 deletions inc/Integrations/Shopify/Queries/SearchProducts.graphql
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
query SearchProducts($search: String) {
products(first: 10, query: $search, sortKey: BEST_SELLING) {
query SearchProducts($search: String!, $limit: Int!, $cursor_next: String) {
products(
first: $limit
after: $cursor_next
query: $search
sortKey: BEST_SELLING
) {
pageInfo {
hasNextPage
hasPreviousPage
startCursor
endCursor
}
edges {
node {
id
Expand Down
30 changes: 30 additions & 0 deletions inc/Integrations/Shopify/ShopifyIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,19 @@ public static function get_queries( ShopifyDataSource $data_source ): array {
'search' => [
'type' => 'ui:search_input',
],
'limit' => [
'default_value' => 8,
'name' => 'Items per page',
'type' => 'ui:pagination_per_page',
],
'cursor_next' => [
'name' => 'Next page cursor',
'type' => 'ui:pagination_cursor_next',
],
'cursor_previous' => [
'name' => 'Previous page cursor',
'type' => 'ui:pagination_cursor_previous',
],
],
'output_schema' => [
'path' => '$.data.products.edges[*]',
Expand Down Expand Up @@ -113,6 +126,23 @@ public static function get_queries( ShopifyDataSource $data_source ): array {
],
],
],
'pagination_schema' => [
'cursor_next' => [
'name' => 'Next page cursor',
'path' => '$.data.products.pageInfo.endCursor',
'type' => 'string',
],
'cursor_previous' => [
'name' => 'Previous page cursor',
'path' => '$.data.products.pageInfo.startCursor',
'type' => 'string',
],
'has_next_page' => [
'name' => 'Has next page',
'path' => '$.data.products.pageInfo.hasNextPage',
'type' => 'boolean',
],
],
'graphql_query' => file_get_contents( __DIR__ . '/Queries/SearchProducts.graphql' ),
] ),
];
Expand Down
27 changes: 19 additions & 8 deletions inc/Validation/ConfigSchemas.php
Original file line number Diff line number Diff line change
Expand Up @@ -288,14 +288,15 @@ private static function generate_http_query_config_schema(): array {
'pagination_schema' => Types::nullable(
Types::object( [
// This field provides an integer representing the total number of
// items available in paginated results. This field must be defined
// in order present to enable pagination support of any type,
// including cursor-based pagination.
'total_items' => Types::object( [
'name' => Types::nullable( Types::string() ),
'path' => Types::json_path(),
'type' => Types::enum( 'integer' ),
] ),
// items available in paginated results. Either this field or
// `has_next_page` must be defined in order to enable
'total_items' => Types::nullable(
Types::object( [
'name' => Types::nullable( Types::string() ),
'path' => Types::json_path(),
'type' => Types::enum( 'integer' ),
] ),
),
// This field provides a pagination cursor for the next page of
// paginated results, or a null value if there is no next page. This
// field must be defined in order to enable cursor-based pagination.
Expand All @@ -316,6 +317,16 @@ private static function generate_http_query_config_schema(): array {
'type' => Types::enum( 'string' ),
] ),
),
// This field provides a boolean indicating if there is a next page of
// paginated results. This is helpful if the API does not provide a
// total number of items.
'has_next_page' => Types::nullable(
Types::object( [
'name' => Types::nullable( Types::string() ),
'path' => Types::json_path(),
'type' => Types::enum( 'boolean' ),
] )
),
] )
),
'preprocess_response' => Types::nullable( Types::callable() ),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ function getResultsWithId( results: RemoteDataResult[], instanceId: string ): Re
interface ItemListProps {
availableBindings: Record< string, RemoteDataBinding >;
blockName: string;
hasNextPage: boolean;
loading: boolean;
onSelect: ( data: RemoteDataQueryInput ) => void;
onSelectField?: ( data: FieldSelection, fieldValue: string ) => void;
Expand All @@ -35,6 +36,7 @@ interface ItemListProps {
remoteData?: RemoteData;
searchInput: string;
setPage: ( newPage: number ) => void;
setPerPage: ( newPerPage: number ) => void;
setSearchInput: ( newValue: string ) => void;
supportsSearch: boolean;
totalItems?: number;
Expand All @@ -45,6 +47,7 @@ export function ItemList( props: ItemListProps ) {
const {
availableBindings,
blockName,
hasNextPage,
loading,
onSelect,
onSelectField,
Expand All @@ -53,6 +56,7 @@ export function ItemList( props: ItemListProps ) {
remoteData,
searchInput,
setPage,
setPerPage,
setSearchInput,
supportsSearch,
totalItems,
Expand Down Expand Up @@ -121,6 +125,7 @@ export function ItemList( props: ItemListProps ) {

function onChangeView( newView: View ) {
setPage( newView.page ?? 1 );
setPerPage( newView.perPage ?? perPage ?? data.length );
setSearchInput( newView.search ?? '' );
setView( newView );
}
Expand Down Expand Up @@ -157,7 +162,7 @@ export function ItemList( props: ItemListProps ) {
onChangeView={ onChangeView }
paginationInfo={ {
totalItems: totalItems ?? data.length,
totalPages: totalPages ?? 1,
totalPages: totalPages ?? ( hasNextPage ? page + 1 : page - 1 ) ?? 1,
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't quite work. The part in parentheses will always return a non-null value, so the 1 is never reached

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. What do you think of using Math.max?

totalPages: totalPages ?? ( hasNextPage ? page + 1 : Math.max( 1, page ) ),

} }
search={ supportsSearch }
view={ view }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@
const { close, isOpen, open } = useModalState();
const {
data,
hasNextPage,
loading,
page,
searchInput,
setPage,
setPerPage,
setSearchInput,
supportsSearch,
totalItems,
Expand Down Expand Up @@ -72,13 +74,15 @@
<ItemList
availableBindings={ availableBindings }
blockName={ blockName }
hasNextPage={ hasNextPage }

Check failure on line 77 in src/blocks/remote-data-container/components/modals/DataViewsModal.tsx

View workflow job for this annotation

GitHub Actions / eslint, prettier, wp-scripts

Type 'boolean | undefined' is not assignable to type 'boolean'.
loading={ loading }
onSelect={ onSelect ? onSelectItem : close }
onSelectField={ onSelectField }
page={ page }
remoteData={ data }
searchInput={ searchInput }
setPage={ setPage }
setPerPage={ setPerPage }
setSearchInput={ setSearchInput }
supportsSearch={ supportsSearch }
totalItems={ totalItems }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from '@/blocks/remote-data-container/config/constants';

interface UsePaginationVariables {
hasNextPage?: boolean;
onFetch: ( remoteData: RemoteData ) => void;
page: number;
paginationQueryInput: RemoteDataQueryInput;
Expand Down Expand Up @@ -88,6 +89,7 @@ export function usePaginationVariables( {
supportsCursorPagination || supportsPagePagination || supportsOffsetPagination;
const totalItems = paginationData?.totalItems;
const totalPages = totalItems && perPage ? Math.ceil( totalItems / perPage ) : undefined;
const hasNextPage = paginationData?.hasNextPage;
Copy link
Member

Choose a reason for hiding this comment

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

can you update the supports*Pagination variables above to take into account the statement we made in the schema? "either total_items or has_next_page must be defined"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something like this?

	const supportsPagination =
		supportsCursorPagination ||
		supportsPagePagination ||
		supportsOffsetPagination ||
		totalItems ||
		hasNextPage;


function onFetch( remoteData: RemoteData ): void {
if ( ! supportsPagination ) {
Expand Down Expand Up @@ -120,6 +122,7 @@ export function usePaginationVariables( {
}

return {
hasNextPage,
onFetch,
page,
paginationQueryInput,
Expand Down
6 changes: 4 additions & 2 deletions src/blocks/remote-data-container/hooks/useRemoteData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ async function fetchRemoteData( requestData: RemoteDataApiRequest ): Promise< Re
pagination: body.pagination && {
cursorNext: body.pagination.cursor_next,
cursorPrevious: body.pagination.cursor_previous,
hasNextPage: body.pagination.has_next_page,
totalItems: body.pagination.total_items,
},
queryInput: body.query_input,
Expand All @@ -51,7 +52,7 @@ interface UseRemoteData {
data?: RemoteData;
error?: Error;
fetch: ( queryInput: RemoteDataQueryInput ) => Promise< void >;
hasNextPage: boolean;
hasNextPage?: boolean;
hasPreviousPage: boolean;
loading: boolean;
page: number;
Expand Down Expand Up @@ -126,6 +127,7 @@ export function useRemoteData( {
const inputVariables = query.inputs;

const {
hasNextPage,
onFetch: onFetchForPagination,
page,
perPage,
Expand Down Expand Up @@ -237,7 +239,7 @@ export function useRemoteData( {
data: resolvedData,
error,
fetch,
hasNextPage: totalPages ? page < totalPages : supportsPagination,
hasNextPage: hasNextPage ?? ( totalPages ? page < totalPages : supportsPagination ),
hasPreviousPage: page > 1,
loading,
page,
Expand Down
6 changes: 4 additions & 2 deletions types/remote-data.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ interface InnerBlockContext {
interface RemoteDataPagination {
cursorNext?: string;
cursorPrevious?: string;
totalItems: number;
hasNextPage?: boolean;
totalItems?: number;
}

interface RemoteDataResultFields {
Expand Down Expand Up @@ -87,7 +88,8 @@ interface RemoteDataApiResponseBody {
pagination?: {
cursor_next?: string;
cursor_previous?: string;
total_items: number;
has_next_page?: boolean;
total_items?: number;
};
query_input: RemoteDataQueryInput;
result_id: string;
Expand Down
Loading