Conversation
merefield
commented
Nov 22, 2025
- makes associations more idiomatic
- adds explicit association with topic to workflow_state
- various points leverage associations
There was a problem hiding this comment.
Pull request overview
This PR focuses on performance improvements by making Active Record associations more idiomatic and leveraging eager loading to reduce N+1 queries. The changes align associations with Rails naming conventions (pluralizing association names) and introduce explicit preloading strategies to optimize database queries when accessing workflow-related data.
Key Changes:
- Pluralized association names across all models (e.g.,
workflow_step→workflow_steps,workflow_state→workflow_states) - Added
register_topic_preloader_associationsto preload workflow data when loading topics - Refactored methods to use associations directly instead of manual queries, improving code readability and leveraging preloaded data
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| plugin.rb | Refactored topic methods to use associations; added topic preloader registration; reformatted code for consistency |
| lib/discourse_workflow/transition.rb | Restructured transition logic with eager loading and improved readability; added early returns and break statement |
| lib/discourse_workflow/topic_extension.rb | Added explicit has_one :workflow_state association to Topic model |
| lib/discourse_workflow/ai_actions.rb | Refactored AI transition methods to use eager loading with find_each for better memory efficiency |
| app/models/discourse_workflow/workflow_step.rb | Pluralized association names from singular to plural |
| app/models/discourse_workflow/workflow_state.rb | Added explicit belongs_to :topic association |
| app/models/discourse_workflow/workflow.rb | Pluralized association names from singular to plural |
| app/controllers/discourse_workflow/workflow_visualisation_controller.rb | Added preloading for workflow steps and options; removed commented code; improved error handling |
Comments suppressed due to low confidence (1)
app/controllers/discourse_workflow/workflow_visualisation_controller.rb:44
- This call to a database query operation happens inside this loop, and could be hoisted to a single call outside the loop.
target_step = DiscourseWorkflow::WorkflowStep.find(option.target_step_id)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
app/controllers/discourse_workflow/workflow_visualisation_controller.rb
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@merefield I've opened a new pull request, #10, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@merefield I've opened a new pull request, #11, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@merefield I've opened a new pull request, #12, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@merefield I've opened a new pull request, #13, to work on those changes. Once the pull request is ready, I'll request review from you. |