-
Notifications
You must be signed in to change notification settings - Fork 22
Free Listings + Paid Ads: Fetch the number of syncable products for the product feed status section #1706
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
Free Listings + Paid Ads: Fetch the number of syncable products for the product feed status section #1706
Changes from all commits
ca6582c
74ce676
2afa78f
8511198
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,62 @@ | ||||
/** | ||||
* External dependencies | ||||
*/ | ||||
import { useEffect, useCallback } from '@wordpress/element'; | ||||
|
||||
/** | ||||
* Internal dependencies | ||||
*/ | ||||
import { API_NAMESPACE } from '.~/data/constants'; | ||||
import useApiFetchCallback from './useApiFetchCallback'; | ||||
import useCountdown from './useCountdown'; | ||||
|
||||
/** | ||||
* @typedef {Object} SyncableProductsCalculation | ||||
* @property {number|null} count The number of syncable products. `null` if it's still in the calculation. | ||||
* @property {()=>Promise} request The function requesting the start of a calculation. | ||||
* @property {()=>Promise} retrieve The function retrieving the result of a requested calculation by polling with 5-second timer. | ||||
*/ | ||||
|
||||
const getOptions = { | ||||
path: `${ API_NAMESPACE }/mc/syncable-products-count`, | ||||
}; | ||||
const postOptions = { | ||||
...getOptions, | ||||
method: 'POST', | ||||
}; | ||||
|
||||
/** | ||||
* A hook for communicating with the calculation of the syncable products and for returning the result. | ||||
* | ||||
* If a shop has a large number of products, requesting the result with a single API may encounter | ||||
* a timeout or out-of-memory problem. Therefore, we use an API to schedule a batch for calculating, | ||||
* and another one to poll the result. | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as https://github.com/woocommerce/google-listings-and-ads/pull/1706/files#r985802794 I think the verb is pull |
||||
* | ||||
* @return {SyncableProductsCalculation} Payload containing the result of calculation and the functions for requesting and retrieving a calculation. | ||||
*/ | ||||
export default function useSyncableProductsCalculation() { | ||||
const { second, callCount, startCountdown } = useCountdown(); | ||||
const [ fetch, { data } ] = useApiFetchCallback( getOptions ); | ||||
const [ request ] = useApiFetchCallback( postOptions ); | ||||
|
||||
// The number 0 is a valid value. | ||||
const count = data?.count ?? null; | ||||
|
||||
const retrieve = useCallback( () => { | ||||
const promise = fetch(); | ||||
promise.finally( () => startCountdown( 5 ) ); | ||||
return promise; | ||||
}, [ fetch, startCountdown ] ); | ||||
|
||||
useEffect( () => { | ||||
if ( second === 0 && callCount > 0 && count === null ) { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ Why we need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It's up to the hook's consumer to decide when to call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||
retrieve(); | ||||
} | ||||
}, [ second, callCount, count, retrieve ] ); | ||||
|
||||
return { | ||||
request, | ||||
retrieve, | ||||
count, | ||||
}; | ||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure but I guess instead of
by polling
you meanby pulling
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just my 2 cents, to me polling is usually used when a request is periodically repeated (which seems to be the case here). Either one could be correct though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the client side has no way of knowing when the needed data will be ready, it queries the data by asking periodically. The implementation here matches the scenario, so the term polling is used.