Wp-calypso: Earn: Only a maximum of twenty payment plans are displayed.

Created on 9 Sep 2020  路  7Comments  路  Source: Automattic/wp-calypso

Steps to reproduce

I have a site with 75 Payment plans on it, but no more than twenty will show on the Payments Plan page, or when going to add the Payments block to a page.

When adding a new plan, the bottom-most one disappears. Deleting a plan makes another another re-appear, so it seems this is an issue with how the page and block are handling pagination.

In this gif, you can see how it adds the new plan and things seem fine, but on a refresh the last item goes missing.

Screen Capture on 2020-09-09 at 13-30-54

Context / Source

p1599663244336400-slack-earn

-->

cc @kwight

Earn [Pri] Normal [Type] Bug

Most helpful comment

Let's stick to pagination; I seem to remember we had lots of problems with the InifiniteScroll component, and had always intended to go back and replace it with pagination in comments.

When a new payment plan is created or an old one is deleted, we append to or remove from the existing list on the frontend, instead of retrieving a new list altogether.

I don't think that too bad, it's a good first step, and if users really complain about confusion, we can look at the extra cost of regular API calls on changes.

I also saw this error when refreshing the page.

Hm, not encouraging 馃檭 Let's see if it pops up for anyone again. (Was there anything in the console you remember? Although that kind of error strikes me as unexpected API results.)

All 7 comments

I doubt this is an issue with missing plans at all, but rather the UI not being able to handle paging beyond the plans coming back from the initial API call.

I took a look at this and @kwight is right. The API currently just returns the most recent 20 results.

However, there seems to be multiple problems with this experience:

  1. The API returns the most recent 20 results without the ability to paginate. The jetpack_api function is called in wp-content/lib/memberships/class.membership-product.php, which retrieves the data using a certain URL, but I haven't been able to find the logic that serves the URL: https://public-api.wordpress.com/rest/v1.1/sites/{blog_id}/posts?type=jp_mem_plan. We'll probably need to change that to be paginated, or return the full list. Can anyone direct me to the logic that serves this, and what would be a solid approach to search for it? I've had trouble trying to find it.

  2. The frontend needs a pagination experience or a warning that tells the user only the most recent 20 payment plans are shown. @sixhours is there an existing design you can recommend for this page specifically?

  3. When a new payment plan is created or an old one is deleted, we append to or remove from the existing list on the frontend, instead of retrieving a new list altogether. Depending on the user experience we want, this could be a confusing to the user, if they create multiple payment plans in a short timeframe, and suddenly see n number of payment plans, and then refresh the page and only see 20. Refetching will also ensure that the client always has the most up-to-date information (in case the user is logged in at two places at the same time). Furthermore, the experience noted in the issue about creating new payment plans, or removing old ones will only happen if the user creates/deletes _and_ refreshes the page (or navigates away and the back to this page)

  4. I also saw this error when refreshing the page. It may be transient, but I want to document it in case anyone else sees it. Rebuilding my containers, sandbox, and reconnecting my network and sandbox didn't help. What did help resolve this issue was navigating from the homepage all over again.
    Screen Shot 2020-09-14 at 11 43 37 PM

All in all -- I think we can break this out into multiple issues once we have a plan of attack. @kwight do you have any preferences on the experience or general thoughts?

The frontend needs a pagination experience or a warning that tells the user only the most recent 20 payment plans are shown. @sixhours is there an existing design you can recommend for this page specifically?

Usually we'd use the InfiniteScroll component to keep showing plans after the first load; but we also have a Pagination component if InfiniteScroll is too complicated/doesn't work for whatever reason. You can see it in action on the /comments screen on a site with lots of comments.

Let's stick to pagination; I seem to remember we had lots of problems with the InifiniteScroll component, and had always intended to go back and replace it with pagination in comments.

When a new payment plan is created or an old one is deleted, we append to or remove from the existing list on the frontend, instead of retrieving a new list altogether.

I don't think that too bad, it's a good first step, and if users really complain about confusion, we can look at the extra cost of regular API calls on changes.

I also saw this error when refreshing the page.

Hm, not encouraging 馃檭 Let's see if it pops up for anyone again. (Was there anything in the console you remember? Although that kind of error strikes me as unexpected API results.)

Made a lot of progress using the existing Pagination component and existing pagination patterns -- left off at getting the total count and incorporating the parameter in the query. Will resume here tomorrow!

I noticed today while looking at #45712 that I couldn't replicate the original issue: I've got an account with 24 plans, and I get all 24 back from the /memberships/products endpoint (and all 24 displayed in the Payment Plans list UI). It turns out Memberships_Product::get_product_list() treats Jetpack and Simple sites differently: JP sites get a standard /posts API call (resulting in twenty plans returned), while the get_posts() call for Simple uses posts_per_page => 100.

@krymson24 So that's something that will need to be addressed for consistency's sake: that post_per_page param should be removed, so that both Simple and Jetpack sites are returning the same number of plans to Calypso, and I think we'll need to add a param to get_product_list() (and therefore the endpoint calling it) for page (the page number of twenty posts to fetch).

See discussion here D49781-code#1012195. Ultimately, building out pagination for this solution became a larger lift than anticipated. Therefore, to address this problem in the fastest and most reasonable way possible, we decided to display the first 100 payment plans in this view for Atomic sites and Simple sites.

Was this page helpful?
0 / 5 - 0 ratings