Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Performance-focused hardening pass for the discourse-workflow plugin, primarily targeting query shape (avoid Ruby-side ID materialization), N+1 avoidance in admin payloads, and more efficient daily stats persistence.
Changes:
- Reworked workflow list quick-filters to remain SQL-native and added regression specs guarding against topic_id plucks.
- Scoped charts payload loading to the selected workflow and added query-shape request specs.
- Reduced N+1s in admin controllers/serializers via explicit preloading, and optimized daily stats writes with set-based aggregation + bulk insert plus new supporting indexes.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/requests/workflow_list_filters_spec.rb | Adds SQL-shape guard ensuring quick-filter composition doesn’t pluck workflow_state topic IDs. |
| spec/requests/workflow_charts_controller_spec.rb | Adds query assertions to ensure chart loading is scoped to the selected workflow. |
| spec/requests/admin/workflows_controller_spec.rb | Adds query-count regression coverage for admin workflows index serialization. |
| spec/requests/admin/workflow_steps_controller_spec.rb | New request spec guarding against per-step queries when listing steps. |
| spec/requests/admin/workflow_step_options_controller_spec.rb | Adds query-count regression coverage for step option listing. |
| spec/lib/workflow_state_schema_spec.rb | New schema/index assertion for workflow_states.updated_at. |
| spec/lib/transition_spec.rb | Expands transition specs and adds a guard against per-transition target-step lookups. |
| spec/lib/post_notification_handler_spec.rb | Adds a bulk-loading regression check as watcher counts grow. |
| plugin.rb | Adds cached TopicList#has_workflow_topics? and bumps plugin version to 0.4.1. |
| lib/discourse_workflow/transition.rb | Refactors transition execution to use preloaded associations and accept actor objects. |
| lib/discourse_workflow/topic_query_extension.rb | Minor join formatting change (no behavioral change expected). |
| lib/discourse_workflow/stats.rb | Replaces per-row stats updates with grouped counts + insert_all! rebuild strategy. |
| lib/discourse_workflow/post_notification_handler.rb | Avoids per-watcher user lookups by writing notifications via user_id. |
| lib/discourse_workflow/list_controller_extension.rb | Converts list filtering to SQL-scoped subqueries and avoids Ruby set intersections. |
| db/migrate/20260223140000_add_updated_at_index_to_workflow_states.rb | Adds updated_at index for overdue/staleness filtering. |
| db/migrate/20260223130000_add_unique_daily_bucket_index_to_workflow_stats.rb | Adds unique daily bucket index to support set-based stats writes. |
| app/serializers/discourse_workflow/workflow_step_serializer.rb | Switches category serialization to a lighter serializer and uses association directly. |
| app/serializers/discourse_workflow/workflow_serializer.rb | Avoids repeated DB ordering/count calls by sorting preloaded steps once. |
| app/serializers/discourse_workflow/workflow_category_serializer.rb | New lightweight category serializer for workflow admin payloads. |
| app/models/discourse_workflow/workflow_state.rb | String style + schema comment update to reflect new index. |
| app/models/discourse_workflow/workflow_stat.rb | String style + schema comment update to reflect new unique index. |
| app/models/discourse_workflow/workflow.rb | Refactors validation warnings / kanban compatibility checks to reduce query churn. |
| app/controllers/discourse_workflow/workflow_charts_controller.rb | Loads only the selected workflow (with steps/categories) for charts. |
| app/controllers/discourse_workflow/workflow_action_controller.rb | Passes actor object into Transition to reduce lookup round-trips. |
| app/controllers/discourse_workflow/admin/workflows_controller.rb | Preloads nested associations to avoid per-workflow queries in index payload. |
| app/controllers/discourse_workflow/admin/workflow_steps_controller.rb | Preloads step associations for index payload (but currently contains a syntax issue). |
| app/controllers/discourse_workflow/admin/workflow_step_options_controller.rb | Preloads option associations for index payload (but currently contains a syntax issue). |
| README.md | Updates roadmap performance status items to reflect implemented work and follow-ups. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
app/controllers/discourse_workflow/admin/workflow_steps_controller.rb
Outdated
Show resolved
Hide resolved
app/controllers/discourse_workflow/admin/workflow_step_options_controller.rb
Outdated
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR is a focused performance hardening pass for discourse-workflow (9 commits, 28 files, +749/-363).
Key Changes