the-events-calendar / ql-events

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

Backport changes from wp-graphql-tec fork. #38

Open justlevine opened 2 years ago

justlevine commented 2 years ago

After learning from @craigwilcox and @kidunot89 that this plugin is no longer abandoned, it makes sense to backport the functionality implemented in wp-graphql-tec.

Important notes about the fork:

Here's a list of piecemeal steps that would be required (each would be a separate PR):

Full disclosure: backporting is a lot of work, and my current priority is adding the missing functionality to wp-graphql-tec that I need for production. Its likely the list of things required to backport will grow.

borkweb commented 2 years ago

Thank you for thinking through these steps and having interest in bringing those changes into ql-events. Your 4th bullet is sticking in my mind, however. I am left wondering:

If the only feature that isn't supported is the WooCommerce Tickets work, does it make more sense to merge wp-graphql-tec back in in its entirety as a new base? Then, instead, add the WooCommerce Tickets functionality on top of that - rather than go through the laborious task of backporting everything? Does that idea have merit, @kidunot89 & @justlevine, or would that alternative be filled with just as much effort?

My main goal would be to optimize any effort put toward the revitalization of ql-events!

kidunot89 commented 2 years ago

@borkweb I agree with merging the fork back, if possible.

justlevine commented 2 years ago

@borkweb It would be significantly faster and easier not to backport (2 maybe 3 weeks max to add unit testing and ETP support, and then I can use the rest of my time to move on to community events/tickets and virtual events).

For me, the reason to backport isnt about feature parity, but because of the project commit history. There's a decent amount of code snippets reused from ql-events, and others where my work was at least inspired by what came before.

When I thought this project was abandoned, I was okay leaving a note in the readme back here. But if this is an officially sanctioned replacement, then I'm uncomfortable unilaterally deciding to wipe the past contribution history. WP is about community involvement after all.

I'm primarily a nonprofit license user, so the extra several months of backporting (if that's what y'all decide) is the least I can do .

Talk it over with @kidunot89 and the TEC overlords, and let me know how you want me to proceed 😎

borkweb commented 2 years ago

@justlevine - I appreciate your thoughtfulness regarding swapping out the original contributions. What we could easily do is rename this repository and archive it for posterity and make a swap - that way we preserve the history of this repo and wp-graphql-tec.

justlevine commented 2 years ago

@borkweb I've begun work on backfiling unit tests as part of getting the fork ready for adoption/ public contributions. Is there any chance you could share (publicly or privately) the WPUnit factories - or even better, the entire /tests folder - for ECP (recurring events) and ETP (the various ticket types)?

Would be a lot quicker than trying to create my own factories from scratch

borkweb commented 2 years ago

@justlevine - I've invited you to the ECP repo so you can poke around actively!

borkweb commented 2 years ago

@justlevine and @kidunot89 - Since there's a goal of bringing the fork of QL-Events back into this plugin and wp-graphql-tec as well, I've created a branch called: feature/wp-graphql-tec that includes the entirety of the wp-graphql-tec repository (with all of its commits/history). My thinking is the following:

  1. @justlevine can PR his fork of QL-Events into this repo.
  2. We can merge the above feature branch into master if this approach with full preservation of all contributions looks good to everyone (knowing that there's a funky subdirectory)
  3. Then as wp-graphql-tec code from the subdirectory gets moved from that subdir into ql-events proper, we can do so with git commands and retain the history of the files while moving the world forward! 🕺
justlevine commented 2 years ago

@borkweb sounds good to me, I defer to your experience on repo management.

As mentioned, I do think before all this happens the wpunit tests need to be finished, even before feature parity with ql-events (ie wootickets and recurring events). I know ql-events doesn't have coverage most of it's codebase, but considering I wrote this all without public code review, there should be a guarantee the code is solid so others can productively contribute.

therealgilles commented 2 years ago

Hello, would it be possible to get an update on the ql-events vs wp-graphql-tec direction? I'm back working on a project where I need to access the TEC events with Gatsby.

Side question: any way to access the json+ld data to add it for SEO?

justlevine commented 2 years ago

@therealgilles I wasn't able to figure out the way to break the fork into piecemeal fixes, and once news of TEC v6 on the horizon (which makes substantial changes to recurring events) broke, I chose for now to prioritize my unpaid time contributing to WPGraphQL core instead - for now.

Depending on the pace of wp-graphql/wp-graphql#2399 , I hope to make public a fully functional version of core + pro + tickets + virtual within a few months of TEC v6's release - sans the legacy cpts/taxes - which this project is welcome to implement in a way that makes sense for it 😎. Community Events/Tickets will take longer.

Sadly none of this will likely help your current project

justlevine commented 2 years ago

(assuming this olugin is no longer compatible with the latest versions of WPGraphQL, you can use this class to retrieve the data and map it to the GraphQL fields you need. Take a look at the source code if either this plugin or my fork for an implementation example. )

therealgilles commented 2 years ago

@justlevine: Thank you for your detailed response. Totally understand the unpaid work trade-offs. My project falls into that category too.

I had some success with this plugin, though I haven't done much testing yet. I had less success with the forked plugin, which surprised me. I was kind of expecting the opposite result. I will see if I can get enough working to fit my needs. I won't be interested in tickets unless TEC starts supporting tickets for recurring events (which people have been asking for for years to no avail, so I won't be getting my hopes up – would have surely been nice for recurring classes). I use TEC for events and WC for tickets (created manually as products).

Thanks for the pointer for the json+ld data!

therealgilles commented 2 years ago

@justlevine do you have any pointers for adding the event start/end date to ORDER BY? I haven't dug into it yet and thought maybe you would have a suggestion.