Conversation
|
1 similar comment
|
There was a problem hiding this comment.
Pull request overview
This PR updates the Lapse integration to Lapse v2 by storing Lapse timelapse IDs and playback URLs directly on devlogs (instead of downloading/reuploading videos), and introduces shared services for fetching/caching timelapses and building “Tracked with Lapse” badges.
Changes:
- Add
lapse_timelapse_id,lapse_playback_url, andlapse_playback_url_refreshed_attopost_devlogs, backfill legacy records, and removelapse_video_processing. - Replace the old Lapse service/fetcher with
Lapse::Api,Lapse::TimelapsesFetcher,Lapse::TimelapsesCache, andLapse::DevlogBadgeBuilder(+ new cache-warming job). - Update project/devlog flows and UI to render Lapse videos via
video_tagwhen a playback URL is available.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| test/models/post/devlog_test.rb | Updates schema annotation for new Lapse fields (annotation now appears stale). |
| test/fixtures/post/devlogs.yml | Updates schema annotation for new Lapse fields (annotation now appears stale). |
| db/schema.rb | Reflects new Lapse columns and removal of lapse_video_processing. |
| db/migrate/20260312220300_add_lapse_timelapse_id_to_post_devlogs.rb | Adds lapse_timelapse_id column. |
| db/migrate/20260313154802_add_lapse_playback_fields_to_post_devlogs.rb | Adds playback URL + refreshed timestamp columns. |
| db/migrate/20260313154803_backfill_lapse_playback_urls_and_remove_lapse_video_processing.rb | Backfills IDs/URLs and drops lapse_video_processing. |
| config/routes.rb | Adds clarifying comment for the lapse_timelapses route. |
| app/services/project_lapse_timelapses_fetcher.rb | Removes legacy project fetcher. |
| app/services/lapse_service.rb | Removes legacy Lapse service wrapper. |
| app/services/lapse/api.rb | Adds consolidated Lapse API wrapper (Timelapse + Hackatime endpoints). |
| app/services/lapse/timelapses_fetcher.rb | Adds new timelapse fetcher with guard checks and filtering. |
| app/services/lapse/timelapses_cache.rb | Adds shared cache keying/TTL for project timelapses. |
| app/services/lapse/timelapse_resolver.rb | Adds resolver utility (currently unused). |
| app/services/lapse/devlog_badge_builder.rb | Extracts badge-building logic into a service. |
| app/models/post/devlog.rb | Stores new fields, refreshes playback URL, updates attachment validation, updates PaperTrail ignore list. |
| app/jobs/process_lapse_timelapse_job.rb | Converts legacy job class into a shim inheriting new implementation. |
| app/jobs/lapse/process_timelapse_job.rb | Adds legacy job implementation that now stores URLs instead of downloading. |
| app/jobs/lapse/cache_project_timelapses_job.rb | Adds job to warm timelapses cache. |
| app/jobs/cache/project_lapse_timelapses_job.rb | Converts legacy cache job into a shim inheriting new implementation. |
| app/controllers/projects/devlogs_controller.rb | Updates devlog creation to store playback URL directly (currently contains a runtime bug). |
| app/controllers/projects_controller.rb | Uses new cache/fetcher + badge builder; adds Turbo-frame modal endpoint behavior updates. |
| app/components/post_component.rb | Exposes lapse_playback_url and expands badge logic. |
| app/components/post_component.html.erb | Replaces “processing” placeholder with an embedded video player when URL exists. |
| app/assets/stylesheets/components/_project_show_card.scss | Layout/typography tweaks for project show cards. |
| app/assets/stylesheets/components/_post.scss | Visual tweaks to post badge/button styling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR updates the Lapse (timelapse) integration to stop downloading/reuploading videos and instead store and render Lapse-provided playback URLs, while also refactoring timelapse fetching and caching into new namespaced services/jobs and migrating existing data off the legacy lapse_video_processing flag.
Changes:
- Adds
lapse_timelapse_id,lapse_playback_url, andlapse_playback_url_refreshed_attopost_devlogsand removeslapse_video_processingafter backfilling. - Introduces
Lapse::Api,Lapse::TimelapsesFetcher(with cache), andLapse::DevlogBadgeBuilder; replaces legacyLapseService/ProjectLapseTimelapsesFetcher. - Updates devlog creation, project pages, and post rendering to use stored playback URLs and new cache-warming jobs.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/models/post/devlog_test.rb | Schema comment header updated for new Lapse fields. |
| test/fixtures/post/devlogs.yml | Schema comment header updated for new Lapse fields. |
| db/schema.rb | Reflects new Lapse columns and removal of lapse_video_processing. |
| db/migrate/20260312220300_add_lapse_timelapse_id_to_post_devlogs.rb | Adds lapse_timelapse_id column. |
| db/migrate/20260313154802_add_lapse_playback_fields_to_post_devlogs.rb | Adds playback URL + refreshed timestamp columns. |
| db/migrate/20260313154803_backfill_lapse_playback_urls_and_remove_lapse_video_processing.rb | Backfills Lapse IDs/URLs and drops legacy column. |
| config/routes.rb | Adds comment clarifying lapse_timelapses is typically Turbo-framed. |
| app/services/project_lapse_timelapses_fetcher.rb | Removed in favor of new Lapse::TimelapsesFetcher. |
| app/services/lapse_service.rb | Removed in favor of new Lapse::Api + fetcher. |
| app/services/lapse/api.rb | New API wrapper for Lapse endpoints. |
| app/services/lapse/timelapses_fetcher.rb | New fetcher with guard checks and per-project caching. |
| app/services/lapse/devlog_badge_builder.rb | Extracted badge computation logic into a service. |
| app/jobs/process_lapse_timelapse_job.rb | Kept as shim delegating to new namespaced job for legacy enqueued jobs. |
| app/jobs/lapse/process_timelapse_job.rb | New legacy job behavior: store playback URL instead of downloading. |
| app/jobs/lapse/cache_project_timelapses_job.rb | New job to warm timelapse cache. |
| app/jobs/cache/project_lapse_timelapses_job.rb | Kept as shim delegating to new namespaced cache-warming job. |
| app/models/post/devlog.rb | Adds playback URL staleness/refresh helpers and updates attachment validation condition. |
| app/controllers/projects/devlogs_controller.rb | Devlog creation now stores Lapse IDs/URLs directly; timelapse loading uses new fetcher. |
| app/controllers/projects_controller.rb | Uses new cached fetcher and badge builder; updates lapse timelapses modal action. |
| app/components/post_component.rb | Badge logic updated; adds helper to obtain playback URL. |
| app/components/post_component.html.erb | Renders Lapse playback video when available instead of processing placeholder. |
| app/assets/stylesheets/components/_project_show_card.scss | Adjusts layout spacing/centering and adds balanced text wrapping. |
| app/assets/stylesheets/components/_post.scss | Updates styling for a post UI element (size/border/shadow/colors). |
Comments suppressed due to low confidence (1)
app/components/post_component.html.erb:160
- When
lapse_timelapse_idis present but the playback URL is still blank (timelapse processing), this template renders no attachments section at all (first branch requireslapse_playback_url.present?, second requiresattachments.any?). That’s a user-visible regression from the old “Video processing” placeholder. Consider adding a placeholder branch for the processing state (e.g., timelapse id present but no playback URL yet).
<% if devlog? && lapse_playback_url.present? && attachments.none? %>
<div class="post__attachments">
<div class="post__viewport">
<%= video_tag lapse_playback_url, class: "post__attachment post__attachment--video", controls: true, preload: "none" %>
</div>
</div>
<% elsif attachments.any? %>
<div class="post__attachments" data-controller="post-attachments">
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Not planned anymore. |
Changes:
Lapsemodule)lapse_timelapse_idandlapse_playback_urlto devlogs