woocommerce / pinterest-for-woocommerce

A native Pinterest integration for WooCommerce. Development is managed by Ventures.
https://woo.com/products/pinterest-for-woocommerce/
GNU General Public License v3.0
22 stars 9 forks source link

Fix pagination on feed_issues endpoint #1027

Open brezocordero opened 2 weeks ago

brezocordero commented 2 weeks ago

Changes proposed in this Pull Request:

Closes #936

Fix the pagination on the Feed Issues table by slicing the results based on per_page and paged parameters.

Screenshots:

Detailed test instructions:

  1. Requires a site connected to Pinterest and a synched catalog with issues.
  2. Go to wp-admin/Marketing/Pinterest
  3. If your catalog has more than 25 issues, check the pagination works
  4. If your catalog has less than 25 issues, add the parameters to the query and check the number of issues displayed is correct. (Ex: on a catalog with 18 issues page=wc-admin&path=%2Fpinterest%2Fcatalog&paged=2&per_page=10 should display 8 issues)
Screenshot 2024-06-21 at 7 17 16 PM

Additional details:

Changelog entry

Fix - Pagination on Feed Issues table.

message-dimke commented 2 weeks ago

Hey, @brezocordero !

The issue is a bit deeper than @ecgan described within his #936 ticket.

With the Pinterest API v3 endpoint we were downloading a file with all the feed item issues and parsing it. We could find a total number of records and build a proper pagination. However, this is not possible anymore. Pinterest v5 API removed the file and replaced it with an API call https://developers.pinterest.com/docs/api/v5/#operation/items_issues/list.

The fix you did does a proper paging for 250 first records, which is the maximum amount of records available in one call. When the solution may fit most of our users, there might be others, who have more records. With the current Pinterest v5 API implementation, it is not possible to know the total amount of items and build the paging. We could build a Prev/Next kind of paging.

Another issue with paging is that we would like to have error type of records go before all the warnings ones. That is why you could see sorting made before the output. And that is the reason I am fetching 250 records with the API call.

brezocordero commented 2 weeks ago

Another issue with paging is that we would like to have error type of records go before all the warnings ones. That is why you could see sorting made before the output. And that is the reason I am fetching 250 records with the API call.

it is not possible to know the total amount of items and build the paging.

Thanks for the explanation @message-dimke . I noticed the limitations when looking at the code. It is not easy to have pagination + errors first with those constraints.

When the solution may fit most of our users, there might be others, who have more records.

Do you know if they could see all the issues on Pinterest > Catalog and Product Groups > Diagnostics?

If they can, one option would be to display a message for those users, letting them know that we only display up to 250 issues and that they can see them all on Pinterest > Catalog and Product Groups > Diagnostics. What do you think?

message-dimke commented 1 week ago

Hi, @brezocordero !

I spoke with @budzanowski about this, and we think your idea of having a message and a link is good. However, I can not yet confirm that Pinterest > Catalog and Product Groups > Diagnostics has the required records. My Pinterent account got disapproved, and I need it recovered first. It may take some time.