MAN-1553 switch events harvest source to lib cal#4455
Conversation
…tps://github.com/tulibraries/manifold into MAN-1553-switch-events-harvest-source-to-lib-cal
| raw(entity.map_to_schema_dot_org.to_json) | ||
| end | ||
|
|
||
| def render_weekly_hours(model) |
There was a problem hiding this comment.
@cdoyle-temple Is this related to the events refactor?
There was a problem hiding this comment.
yes, it was an existing issue that this work exposed
There was a problem hiding this comment.
_model_hours.html.erb was rendering @building.weekly_hours / @space.weekly_hours directly, and those can be blank when a location passed in from the events has no hours data. render_weekly_hours was added as a guard so we only render when a component is actually present.
There was a problem hiding this comment.
I still think that work could have been put in a different PR to make this more manageable.
|
@cdoyle-temple I know this work came from a single ticket, and I recognize that replacing an event-ingestion source touches a lot of interconnected pieces. That said, this PR has become much too large to review effectively. It includes the new LibCal sync, data-model and migration changes, location mapping, filtering changes, admin and public UI changes, removal of the legacy sync, backfill tasks, and several supporting fixes and test cleanups. Trying to review all of those changes together makes it difficult to understand the impact of each piece, identify regressions, and give the implementation the level of attention it deserves. I really think we need to approach work of this size in phases, even when it originates from one ticket. For example, this could potentially have been divided into a sequence like: Introduce the new event fields, associations, migrations, and location crosswalk without changing the active sync. These phases could still be tracked under the same ticket or an umbrella ticket, and feature flags or temporarily unused code could be used where one phase cannot be enabled until a later one is complete. I am not suggesting that the work needs to be restarted, but I do want us to be more intentional about identifying reviewable milestones before opening PRs of this size. Smaller PRs make reviews more manageable, make it easier to test and roll back changes, and reduce the chance that important details get lost in a very large diff. |
2 participants
This PR integrates the LibCal API so library events are ingested directly into the site. Events are fetched, matched to their managed building and space via a location crosswalk, and kept current on each sync — with an admin "Sync now" button for on-demand runs. Event categories (including system-wide ones like "Workshop") drive the workshop, DSS, and HSL filters automatically, online events are tagged when they carry a join URL (existing functionality carried over but currently unused but ready to be enabled), and events can be suppressed from public view entirely.
Alongside the sync, this work tightens up the event experience: locations resolve to the correct building page (falling back to a google maps link for outside venues), the show page displays full titles and a clear registration link, and a few supporting fixes came in with it — graceful handling of Google Sheets outages and improved test isolation.
Deploy notes: includes DB migrations, requires env vars LIBCAL_CLIENT_ID/LIBCAL_CLIENT_SECRET to be set, and a one-time rake libcal:backfill_locations links existing events to their buildings. New physical locations are configured in config/libcal_location_lookup.yml.