the-events-calendar / ql-events

The Events Calendar binding to WPGraphQL
15 stars 7 forks source link

0.3.1 only returns one paged set of events results with gatsby-source-wordpress #61

Open james-wfaa opened 8 months ago

james-wfaa commented 8 months ago

When used with gatsby-source-wordpress, version 0.3.0 returns all events on a site during the sourcing phase.

Version 0.3.1 only returns one request's worth of events. gatsby-source-wordpress has a schema / perPage setting that controls "The number of nodes to fetch per page during node sourcing" https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-source-wordpress/docs/plugin-options.md#schemaperpage

Whatever this value is set to (default is 100, but on my sites I use 20 or 40 due to constraints of my wordpress server), that's how many events are returned when running ql-events 0.3.1. If I have it set to 40, it gets the first 40 and stops. Or 20, or 100. For whatever reason it does not make any subsequent requests to go get the rest of the events that exist.

I tested this several times and got consistent results that 0.3.0 returns all events and 0.3.1 only returns the first paged batch. (I think a couple times it may have fetched two batches, 80 rather than 40... but it still wasn't returning all of them.

kidunot89 commented 7 months ago

@james-wfaa What version of TEC are you using? And are you using TEC Pro?

james-wfaa commented 7 months ago

@kidunot89 I've been running 6.2.8 TEC and 6.2.4 TEC Pro. I've been focused exclusively on QLEvents 0.3.1 recently and have made some progress. I have been able to resolve the pagination issue completely when not running TEC Pro by making a few small adjustments to includes/data/connection/class-event-connection-resolver.php. But it is continuing to break when TEC Pro is enabled. I submitted a PR and then canceled it because it was the wrong approach.

One thing I've been digging into is that pagination will break when the last two events in a page of results both have the same start time. I can definitely recreate that and share with you if you are interested. I've been exploring with adding additional sort params... start date AND end date... AND event name... but I think ultimately since the only value that has to be unique between two events is the database ID, you are going to want to incorporate that too somehow. Otherwise there is always at least the possibility of the pagination code getting confused by two distinct events that have all of the same attributes.

kidunot89 commented 7 months ago

Have you enabled Events Pro support in plugin settings under GraphQL > Settings then the QL Events tab in the WP Admin?

james-wfaa commented 7 months ago

I have done this at times, but I’m not sure I’ve done it with my latest testing (where I got pagination working without Pro enabled but it was failing when Pro is enabled). I will re-test and make sure Pro support is enabled on the QL Events tab and get back to you.

From: Geoff Taylor @.> Date: Wednesday, December 13, 2023 at 3:54 PM To: the-events-calendar/ql-events @.> Cc: James Rotering @.>, Mention @.> Subject: Re: [the-events-calendar/ql-events] 0.3.1 only returns one paged set of events results with gatsby-source-wordpress (Issue #61)

CAUTION: This email originated from outside the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.

Have you enabled Events Pro support in plugin settings under GraphQL > Settings then the QL Events tab in the WP Admin?

— Reply to this email directly, view it on GitHubhttps://github.com/the-events-calendar/ql-events/issues/61#issuecomment-1854758722, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AQHMLRCZQD2IN5RAQOIVXSTYJIPYTAVCNFSM6AAAAAA6QDWXZGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJUG42TQNZSGI. You are receiving this because you were mentioned.Message ID: @.***>

This message may contain confidential, privileged, and proprietary information and is intended only for the person(s) to whom it is addressed. Any use, distribution, copying, or disclosure of this message by any other person is strictly prohibited. If you have received this message in error, please notify the email sender immediately, and delete the original message without making a copy. For information on the Wisconsin Foundation and Alumni Association privacy policy, please visit: https://advanceuw.org/privacy-policy/.

kidunot89 commented 6 months ago

@james-wfaa Did you confirm that the setting was enabled?

james-wfaa commented 6 months ago

I did confirm, and it makes no difference with the pagination error whether the setting is enabled or disabled.

I now have a working branch that handles forwards and backwards pagination with or without TEC Pro enabled. https://github.com/uwfoundation/ql-events/tree/fix-pagination

There were several issues interacting with each other that made this very confusing.

Line 115 defining the ‘graphql_cursor_id_key’ was causing problems, and also the lack of a specific sort parameter in lines 133-138. Both of those were contributing to situations where hasNextPage was returning false when it should have been returning true… cursor value was off by one, including the last result from page X as the first result of page X+1. This was causing wp-graphql/src/Data/Connection/AbstractConnectionResolver ->has_next_page() to return false because after removing the duplicate there were not more remaining IDs than the query amount.

Additional similar issues were occurring with my database when multiple events shared the same start date/time and fell at the end of a pagination set. I added ‘title’ as a secondary ‘orderby’ argument, and then realized I had to ultimately add the ID as a final ‘orderby’. Theoretically there could be multiple events that all share the same start time, end time, title, and many other details (at different locations for example).

With the branch linked above, you can page forwards through all of the results without getting any hasNextPage errors (which is necessary for a Gatsby build). You can also page backwards through all of the results without any discrepancies, in the same manner that you can with basic WordPress Posts.

From: Geoff Taylor @.> Date: Tuesday, December 19, 2023 at 10:07 AM To: the-events-calendar/ql-events @.> Cc: James Rotering @.>, Mention @.> Subject: Re: [the-events-calendar/ql-events] 0.3.1 only returns one paged set of events results with gatsby-source-wordpress (Issue #61)

CAUTION: This email originated from outside the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.

@james-wfaahttps://github.com/james-wfaa Did you confirm that the setting was enabled?

— Reply to this email directly, view it on GitHubhttps://github.com/the-events-calendar/ql-events/issues/61#issuecomment-1863054742, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AQHMLRA3VOQZIQSYUALKMCLYKG3VHAVCNFSM6AAAAAA6QDWXZGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRTGA2TINZUGI. You are receiving this because you were mentioned.Message ID: @.***>

This message may contain confidential, privileged, and proprietary information and is intended only for the person(s) to whom it is addressed. Any use, distribution, copying, or disclosure of this message by any other person is strictly prohibited. If you have received this message in error, please notify the email sender immediately, and delete the original message without making a copy. For information on the Wisconsin Foundation and Alumni Association privacy policy, please visit: https://advanceuw.org/privacy-policy/.