Add status filter to user's note page #5297
Conversation
kcne
force-pushed
the
832-status-filter
branch
2 times, most recently
from
5670d46 to
0d0045e
Compare
app/models/note.rb
Outdated
| else | ||
| all | ||
| end | ||
| } |
There was a problem hiding this comment.
Rather than trying to handle the all case specially here I think I would make this much simpler, like the with_status on the issue model, and then add an unless params[:status] == "all" guard in the controller so that the filter isn't applied when all statuses are requested.
There was a problem hiding this comment.
Do we even need the scope? A similar scope for issues is not used by the issues controller:
There was a problem hiding this comment.
Fixed and updated the PR. Since this is the only place using the scope, I removed it to simplify the code and maintain consistency. If a need arises, we can reintroduce it later.
Also, I’ve added a test in the controller to cover the basic status filtering functionality. Let me know if further tests are needed.
kcne
force-pushed
the
832-status-filter
branch
3 times, most recently
from
337d37e to
b56dc10
Compare
app/views/notes/index.html.erb
Outdated
| @@ -6,6 +6,20 @@ | |||
| :commented => tag.span(t(".subheading_commented"), :class => "px-2 py-1 bg-body") %></p> | |||
| <% end %> | |||
|
|
|||
| <%= form_with :url => url_for(:controller => "notes", :action => "index"), :method => :get, :data => { :turbo => true } do %> | |||
There was a problem hiding this comment.
Why :url => url_for(:controller => ...) and not :url => user_notes_path(@user)?
There was a problem hiding this comment.
Missed this one. Refactored. Thank you.
app/views/notes/index.html.erb
Outdated
| :class => "form-select" %> | ||
| </div> | ||
| <div class="col-sm-auto mb-3"> | ||
| <%= submit_tag t(".apply"), :class => "btn btn-primary" %> |
There was a problem hiding this comment.
We have this in issues search form to avoid adding &commit=... to urls:
| <%= submit_tag t(".apply"), :class => "btn btn-primary" %> | |
| <%= submit_tag t(".apply"), :name => nil, :class => "btn btn-primary" %> |
There was a problem hiding this comment.
Couldn't realize why this was happening, I didn't like it either. Fixed now. Thank you.
kcne
force-pushed
the
832-status-filter
branch
from
b56dc10 to
738e66a
Compare
|
Merged, thanks. |
Description
This PR partly addresses #832 and serves as a smaller alternative to #5255 as suggested in comments. It aims to enhance user experience on user note pages by providing filtering functionality for notes by status. The
with_statusscope is added to theNotemodel, and theNotesControlleris updated accordingly. Default behavior remains unchanged since the status preselected value is set toAllby default.How has this been tested?
Test for the new scope is added to note model test to ensure functionality is working as expected.
Screenshots