Refactor task retrieval logic to handle 'only_todo' filter#153
Refactor task retrieval logic to handle 'only_todo' filter#153miyake13000 merged 2 commits intonomlab:mainfrom
Conversation
修正内容
懸念点
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the task retrieval logic in the tasks controller to use the active scope from the Task model instead of directly filtering by state using Task.where. The changes improve code maintainability by centralizing the "todo" state filtering logic.
Key changes:
- Replaced direct state filtering with the
Task.activescope for the "only_todo" filter - Modified sort column references from
state_prioritytostate.priorityto properly reference the joined state table - Added hidden field tags to preserve query parameters across form submissions
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/controllers/tasks_controller.rb | Refactored to use Task.active scope instead of direct state filtering; updated sort column references to use joined state table syntax |
| app/views/tasks/index.html.erb | Added hidden field tags to preserve only_todo and search query parameters during form submissions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| <% if params[:q].present? %> | ||
| <% params[:q].each do |key, value| %> | ||
| <%= hidden_field_tag "q[#{key}]", value %> |
There was a problem hiding this comment.
Potential XSS vulnerability: params[:q] is being iterated without sanitization. If params[:q] contains nested hashes or arrays, this could lead to unexpected behavior or security issues. Consider validating the structure of params[:q] and sanitizing the values before rendering them in hidden fields.
| <%= hidden_field_tag "q[#{key}]", value %> | |
| <% if value.is_a?(String) || value.is_a?(Numeric) || value == true || value == false %> | |
| <%= hidden_field_tag "q[#{key}]", h(value) %> | |
| <% end %> |
|
LGTM |
activeスコープを用いるように修正した.