hackgvl / hackgreenville-com

HackGreenville's Website
https://hackgreenville.com
MIT License
16 stars 15 forks source link

Event Importer (Meetup + Eventbrite) along with Events Api + Org Api #186

Closed bogdankharchenko closed 6 months ago

bogdankharchenko commented 7 months ago

OK This is a big one... :fire: :fire: :fire:

Why

We have been talking about migrating + improving the events api. I figured I would take a stab at this. I think having this code inside Laravel would give us an ability to move forward very quickly with a v2 of the api, along with other wishlist items.

Events + Organizations API Support

These new API Endpoints have the same exact data structure as the current events.openupstate.org and organizations.openupstate.org - meaning we could simply change the URL's on any of the applications which depend on the openupstate api's to the hackgreenville.com api's and everything should continue to work as intended (some small tweaks may be necessary)

Simply put, we use the php artisan pull:orgs command to pull all the organization data and store it on hackgreenville’s database.

We no longer need to “pull:events” see “Event Importer” below

Once we deploy this - we should do a 301 redirect from the two api endpoints to point to hackgreenville’s api.

API Endpoints

https://hackgreenville.com/api/v0/events https://hackgreenville.com/api/v0/orgs

Side note

Event Importer

Event Importer replicates the same results as the events-api python project - but it stores it into hackgreenville’s event table - we no longer need to import events from openupstate api via php artisan pull:events

One of the biggest benefits, is that we can easily scale this to handle other event service providers, such as GetTogether or any other system. It is by far a much more robust system, which supports pagination and is backed by a test suite.

Currently supports:

Modular System

One of the things which is notable in this PR, is an introduction of a Modular code structure - see https://github.com/InterNACHI/modular

I work for InterNACHI and we have developed this package to be able to properly manage large code bases. This is a great way to “organize code” which will come into play as this repo grows and new comers look at our code.

Near Future

  1. We need to create an Admin system (very soon), perhaps using something like “filamentphp” to be able to manage the “Organizations” - once this is complete we will be able to ditch Organization management on the “Drupal” site.
  2. We will eventually need to support multi-event providers per Organizations https://github.com/hackgvl/hackgreenville-com/issues/211 (this will be relatively easy to execute, given that we have tests in place)

Local Testing

TODO

Let’s Chat

I’m all ears. Hit me up on slack or dump a comment below. There are a few, “fix me” and “todo” comments, worth discussing.

bogdankharchenko commented 7 months ago

@ThorntonMatthewD thank you!

ThorntonMatthewD commented 7 months ago

Is it worth it to add in support for the following query strings to the org endpoint? https://github.com/hackgvl/OpenData/blob/master/ORGANIZATIONS_API.md#query-string

I'm not sure if this is meant to be a replacement or a passthrough.

allella commented 7 months ago

I posted related thoughts on the Org side on https://github.com/hackgvl/hackgreenville-com/issues/187#issuecomment-1803039511

@bogdankharchenko sorry it's taken me so long to give this attention. I wanted to give it a my full attention before commenting, but then went from traveling, to being sick while trying to catch back up from traveling.

I haven't done a technical review, and trust I won't be of much use there anyway, but my main thought was we could test out the new API endpoint using less critical consuming apps to start, like the OpenWorks dashboard and the Slack Bot. If there's something that breaks with those it would likely be more obvious, easier to fix or revert, and impacting a smaller audience.

Also, I wonder if we'll have some of the same challenges with dev copies of this when there's a database component where we want more than made up seed data. I'm assuming you can do things like dev migrations to seed the database with real data, but I'm not too familiar with the modern solutions to how you try to sync a production database back to a dev copy without syncing too much, like privacy or security data.

allella commented 7 months ago

Also, we have the staging domain, so we could merge this PR in, or else checkout the PR branch on stage, and give it a soft-run on that domain by pointing the consuming apps at the stage API enpoints.

bogdankharchenko commented 7 months ago

Is it worth it to add in support for the following query strings to the org endpoint? https://github.com/hackgvl/OpenData/blob/master/ORGANIZATIONS_API.md#query-string

I'm not sure if this is meant to be a replacement or a passthrough.

So for Organizations, we need to hash out some of these details, and thank you for pointing me to the docs, I did not see that.

If everyone thinks this is a good idea, and we can talk about this on the hack night, I will go ahead and add those filters. But the reality is that, we will merge this PR, offer event-api first, and then finish up the orginization api + admin to manage it.

ThorntonMatthewD commented 7 months ago

I have confirmed that this implementation of the events API is compatible with the current state of the Slack bot. I haven't confirmed that this is the case with the following:

From a cursory comparison of key-values I think things will go prettty smoothly. I love the idea of placing this into a staging environment and gradually testing there- I love that there is that option.

allella commented 7 months ago

We've never really used the create, update or delete features of the CRUD parts of the orgs API, so I wouldn't worry about supporting anything beside Read, unless we have some future idea where authenticated CUD is needed.

allella commented 7 months ago

I think it would be good to have @zach2825 review this on the technical side, since I'm not too familiar with all the Laravel patterns

ThorntonMatthewD commented 7 months ago

I've received the following error while accessing the /api/v0/events route: Screenshot_2023-11-15_19-39-57

Steps to reproduce:

  1. I checked out the branch
  2. Ran composer install
  3. Performed a migration
  4. Executed php artisan pull:orgs
  5. Executed php artisan import:events
  6. Started up the backend and frontend
  7. Accessed http://localhost:8000/api/v0/events in my web browser.

5 events currently have $this->resource->organization == null

I'm not sure if just tacking on some safe-navigation operators is the call here, or if these should have orgs popping up for them.


Something that seems a bit odd is how Synergy Mill is missing an org on one event, but not another one of theirs:

{
        "event_name": "Building a simulated CubeSat",
        "group_name": "SynergyMill Community Workshop",
        "org": null,
        "url": "https://www.meetup.com/synergymill/events/297120903/",
        "time": "2023-11-14T23:30:00.000000Z",
        "status": "past",
        "rsvp_count": 4,
        "description": "Tim Fowler will be talking about some Low Earth Orbit (LEO) satellites he has been making:<br/>A New ETHOS…A journey into the why, how and who cares of building a simulated CubeSat.<br/>We'll have food sponsored by our favorite makerspace, SynergyMill, so if you plan to attend at OpenWorks and want to eat, please click register, or contact us by some other means so we can have a headcount. We'll start eating at 6:15pm or before to focus on Tim's presentation. Thank you to Joey at SynergyMill!!!<br/>Tim will be joining us, along with ColaLUG, the Columbia SC Linux User's Group, live on Jitsi:<br/><a href=\"https://meet.jit.si/sclugs\" class=\"linkified\">https://meet.jit.si/sclugs</a><br/>Enter OpenWorks from the 3rd floor of Richardson St. Garage . ",
        "uuid": "e87b6816-d0ed-4889-9096-7d5e017a30cd",
        "data_as_of": "2023-11-16T00:36:44.820490Z",
        "service_id": "297120903",
        "service": "meetup",
        "venue": {
            "name": "OpenWorks",
            "address": "101 N Main St #302",
            "city": "Greenville",
            "state": "SC",
            "zip": "29601",
            "country": null,
            "lat": "34.852020263672",
            "lon": null
        },
        "created_at": "2023-11-13T13:36:41.000000Z"
    }

versus

    {
        "event_name": "OPEN HOUSE! Public welcome! You're invited!",
        "group_name": "Synergy Mill",
        "org": {
            "id": 53,
            "category_id": 2,
            "title": "Synergy Mill",
            "path": "https://data.openupstate.org/organization/synergy-mill",
            "city": "Greenville",
            "focus_area": "Makerspace - Woodwork, Metalwork, Welding",
            "uri": "https://synergymill.com",
            "primary_contact_person": "Doug Cone",
            "organization_type": "Meetup Groups",
            "event_calendar_uri": "https://www.meetup.com/synergymill/",
            "service": "meetup",
            "status": "active",
            "service_api_key": "synergymill",
            "cache": "",
            "established_at": "2016-01-01T05:00:00.000000Z",
            "created_at": "2023-11-13T13:36:30.000000Z",
            "updated_at": "2023-11-16T00:27:03.000000Z",
            "deleted_at": null
        },
        "url": "https://www.meetup.com/synergymill/events/297239695/",
        "time": "2023-11-20T23:00:00.000000Z",
        "status": "upcoming",
        "rsvp_count": 1,
        "description": "<p>We're looking for the best makers, inventors, creators!<br/>We'll be accepting new member applications so if you want to get in on what we have going on, now is the time!</p> <p>Come see some cool tool demonstrations and learn about our new upcoming classes and activities we have planned.</p> <p>If you have become a new member recently, come meet the awesome people in the community. Bring a friend, come see some cool projects in process, meet some cool makers.</p> <p>Also, there will be pizza and new members will receive a free gift.</p> ",
        "uuid": "43d3427eb461232d88ea6a8af556af36",
        "data_as_of": "2023-11-16T00:36:44.832381Z",
        "service_id": "297239695",
        "service": "meetup",
        "venue": {
            "name": "Synergy Mill",
            "address": "400 Birnie St Ext",
            "city": "Greenville",
            "state": "SC",
            "zip": "",
            "country": "us",
            "lat": "34.849491119385",
            "lon": null
        },
        "created_at": "2023-11-13T13:36:41.000000Z"
    }

The group names differ, which is odd. Is that the reason for the issue with the first record? I'm not sure yet..

ThorntonMatthewD commented 7 months ago

I'm running into an issue where whenever I provide the start_date param to /api/v0/events I end up being redirected to the homepage.

Screenshot_2023-11-15_22-49-43

:heavy_check_mark: /api/v0/events?tags=1 :heavy_check_mark: /api/v0/events?end_date=2023-12-01 :heavy_check_mark: /api/v0/events?end_date=2023-12-01&tags=1 :x: /api/v0/events?start_date=2023-11-01 :x: /api/v0/events?start_date=2023-11-01&tags=1 :x: /api/v0/events?start_date=2023-11-01&end_date=2023-12-01 :x: /api/v0/events?start_date=2023-11-01&end_date=2023-12-01&tags=1

ThorntonMatthewD commented 7 months ago

I tried clearing the route cache to no avail. This is a real brain tickler!

ThorntonMatthewD commented 7 months ago

I changed that piece in the controller to the following:

->when($request->filled('test'), function (Builder $query) use ($request) {
    $query->where('active_at', '>=', $request->date('test'));
})

... and it worked? (/api/v0/events?test=2023-12-10)

~Is start_date reserved somehow?~ Nvm, it's defintely not.

ThorntonMatthewD commented 7 months ago

After editing EventApiV0Request#rules to use the new name it began to redirect again, so I think it's something there.

bogdankharchenko commented 6 months ago

@ThorntonMatthewD it looks like we just needed to add date format to the validator.

Also, I need to add a better way to respond to api errors, currently it does a 302 but we should responding with some json structure

ThorntonMatthewD commented 6 months ago

@ThorntonMatthewD it looks like we just needed to add date format to the validator.

Also, I need to add a better way to respond to api errors, currently it does a 302 but we should responding with some json structure

I came across this last night: https://laravel.com/docs/10.x/validation#quick-writing-the-validation-logic

If validation fails during a traditional HTTP request, a redirect response to the previous URL will be generated. If the incoming request is an XHR request, a JSON response containing the validation error messages will be returned.

I had only tested it in my browser, so I wonder what the result would be if I did it via another method.

bogdankharchenko commented 6 months ago

@ThorntonMatthewD It looks like if you pass "Accept" header, you get that validation error

Screenshot 2023-11-16 at 2 31 37 PM
ThorntonMatthewD commented 6 months ago

Screenshot_2023-11-16_14-35-29

That did the trick!

ThorntonMatthewD commented 6 months ago

Screenshot_2023-11-16_14-36-32

Pulled down the latest commit and it's working great now. :smile:

allella commented 6 months ago

Since folks seem happy with it I'm good with merging this into develop and pulling to stage to see.

I suspect we could point some consumers at the stage APIs if we want an extra level of semi-live tests.

ThorntonMatthewD commented 6 months ago

That would be great! I can open a PR up for the Slack bot to move it over to the new endpoints.

bogdankharchenko commented 6 months ago

@bogdankharchenko thanks, I see that the max_days_in_past and the new max_days_in_the_future are implemented.

The other config value was default_days_in_the_past, which we had set to 1 day in the past so when a consuming app asked for upcoming events, without specifying a start date, it would still show some past events. This was mainly useful so a dashboard like at OpenWorks didn't roll off an event after it started. It might also be useful in situations like a multi-day conference when using the start_date of "now" in response to an API request might cause something that's ongoing to disappear before it actually finished.

The "default" days, was if no start and end date were specified by an API request. It would usually return events from yesterday and into the future.

Thoughts on that config value?

The defaults are 30, and 180 days. See event-importer-handlers.php

ThorntonMatthewD commented 6 months ago

I just took the latest changes for a test drive and they work great! Nicely done!! I've left a single comment about .env.sample, but that was it.

allella commented 6 months ago

@bogdankharchenko are there any commands we need to run one-time with this PR?

And, are there any new commands we'd want to add to scripts/handle-deploy-update.sh deployment script for regular execution?

ThorntonMatthewD commented 6 months ago

@bogdankharchenko are there any commands we need to run one-time with this PR?

And, are there any new commands we'd want to add to scripts/handle-deploy-update.sh deployment script for regular execution?

@allella

php artisan pull:orgs

php artisan import:events

These commands will "prime the pump", and then events will be imported every 3 hours from that point forward.

Based on my experience locally the only other command I had to execute was composer install, and that is already included in the deployment script.

bogdankharchenko commented 6 months ago

@allella @ThorntonMatthewD

OK so one thing that is not mentioned in this PR but its critical.

We are adding a relationship between the events table and the organizations. Currently we do not have a way to map old events to organizations (in laravel) -- as soon as we deploy this pr, a few things will break....

To mitigate this, we need to wipe our events and organizations from the database, and then run the commands to import organizations and events.

We can either do,

  1. Manually delete the records from database - truncate table
  2. via terminal we can execute a php artisan tinker command
  3. Create another command to wipe that data

Obvious downsides are we lose old events from the database, but the event-importer will importer will attempt to go back 30 days and import those events again.

allella commented 6 months ago

@bogdankharchenko there's no worry about losing past events. We've done it many times to try to fix other issues.

Would one of the existing database commands be sufficient, like db:wipe?

image

ThorntonMatthewD commented 6 months ago

@allella @bogdankharchenko

I've set up a small command to wipe just those two tables: https://github.com/bogdankharchenko/hackgreenville-com/pull/3

php artisan app:clear-orgs-and-events will truncate events and orgs.

allella commented 6 months ago

Kudos to @bogdankharchenko for the work and Matt for the heavy lifting on the review and testing.

This is now staged.

https://stage.hackgreenville.com/events

The only minor hiccup I've seen so far is remembering to copy the .env.example variables to .env, setting the Eventbrite private, key and then clearing the config cache file because it wasn't recognizing the new .env vars.

php artisan app:truncate-orgs-and-events
sh scripts/handle-deploy-update.sh
php artisan pull:orgs
php artisan import:events

php artisan config:clear
php artisan config:cache    

The events data does differ slightly from stage and live, but we've always had a bunch of unknown cancelled events, so it's possible / likely the new API is properly avoiding an issue with the existing events API.

It's worth comparing stage to live before we point some of the consuming apps to the stage copy for the next level of testing.