Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add filtering functionality to user notes page #5255

Closed
wants to merge 7 commits into from
Closed

Add filtering functionality to user notes page #5255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c49aaf4
Add filter and sort scopes to note model
kcne Oct 8, 2024
0117a27
Add tests for filtering and sort scopes inside note model
kcne Oct 8, 2024
0f1045e
Integrate filtering and sorting methods into notes controller
kcne Oct 8, 2024
f5394d2
Add sorting and filtering functionality to note view
kcne Oct 8, 2024
bafa9be
Add tests to note controller and handle edge cases
kcne Oct 9, 2024
8725d95
Use turbo for filtering on select without refreshing the whole page
kcne Oct 16, 2024
ec76688
Remove filtering by interaction. Group status by open, closed and com…
kcne Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions app/controllers/notes_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,23 @@ class NotesController < ApplicationController
before_action :set_locale
around_action :web_timeout

##
# Display a list of notes by a specified user
def index
param! :page, Integer, :min => 1

@params = params.permit(:display_name)
@title = t ".title", :user => @user.display_name
@params = params.permit(:display_name, :from, :to, :status, :sort_by, :sort_order)
@title = t(".title", :user => @user.display_name)
@page = (params[:page] || 1).to_i
@page_size = 10
@notes = @user.notes
@notes = @notes.visible unless current_user&.moderator?
@notes = @notes.order("updated_at DESC, id").distinct.offset((@page - 1) * @page_size).limit(@page_size).preload(:comments => :author)
.filter_hidden_notes(current_user)
.filter_by_status(params[:status], @user.id)
.filter_by_date_range(params[:from], params[:to])
.sort_by_params(params[:sort_by], params[:sort_order])
.distinct
.offset((@page - 1) * @page_size)
.limit(@page_size)
.preload(:comments => :author)

render :layout => "site"
end
Expand Down
34 changes: 34 additions & 0 deletions app/models/note.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,40 @@ class Note < ApplicationRecord
scope :visible, -> { where.not(:status => "hidden") }
scope :invisible, -> { where(:status => "hidden") }

scope :filter_hidden_notes, lambda { |user|
if user&.moderator?
all
else
visible
end
}

scope :filter_by_status, lambda { |status, user_id = nil|
case status
when "open"
where(:status => "open")
when "closed"
where(:status => "closed")
when "commented"
where("NOT EXISTS (SELECT 1 FROM note_comments WHERE note_comments.note_id = notes.id AND note_comments.author_id = ? AND note_comments.id = (SELECT MIN(nc.id) FROM note_comments nc WHERE nc.note_id = notes.id))", user_id)
else
all
end
}

scope :filter_by_date_range, lambda { |from, to|
notes = all
notes = notes.where(:created_at => DateTime.parse(from).beginning_of_day..) if from.present?
notes = notes.where(:created_at => ..DateTime.parse(to).end_of_day) if to.present?
notes
}

scope :sort_by_params, lambda { |sort_by, sort_order|
sort_by ||= "updated_at"
sort_order ||= "desc"
order("#{sort_by} #{sort_order}")
}

after_initialize :set_defaults

DEFAULT_FRESHLY_CLOSED_LIMIT = 7.days
Expand Down
88 changes: 55 additions & 33 deletions app/views/notes/index.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<% content_for :heading_class, "pb-0" %>
<% content_for :heading do %>
<h1><%= t ".heading", :user => @user.display_name %></h1>
<p><%= t ".subheading_html",
Expand All @@ -6,41 +7,62 @@
:commented => tag.span(t(".subheading_commented"), :class => "px-2 py-1 bg-body") %></p>
<% end %>

<% if @notes.empty? %>
<h4><%= t ".no_notes" %></h4>
<%= form_tag(url_for("controller" => "notes", "action" => "index"), "method" => :get, "data" => { "turbo" => true, "turbo-frame" => "pagination", "turbo-action" => "advance" }) do %>
<div class="row gx-2">
<div class="col-md-auto mb-3">
<%= label_tag :status, t(".status") %>
<%= select_tag :status,
options_for_select([[t(".all"), "all"], [t(".open"), "open"], [t(".closed"), "closed"], [t(".commented"), "commented"]], params[:status] || "all"),
:class => "form-select",
:onchange => "this.form.requestSubmit()" %>
Copy link
Collaborator

@AntonKhorev AntonKhorev Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onchange is not going to work with the csp_enforce: true setting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locally, this works as expected for me. Could you provide more details on how to reproduce this issue locally, or is this something that only occurs on production servers? Thanks.

</div>
<div class="col-md-auto mb-3">
<%= label_tag :from, t(".from") %>
<%= date_field_tag :from, params[:from], :placeholder => t(".from"), :class => "form-control", :autocomplete => "off", :onchange => "this.form.requestSubmit()" %>
</div>
<div class="col-md-auto mb-3">
<%= label_tag :to, t(".to") %>
<%= date_field_tag :to, params[:to], :placeholder => t(".to"), :class => "form-control", :autocomplete => "off", :onchange => "this.form.requestSubmit()" %>
</div>
<div class="col-md-auto mb-3 mt-auto">
<%= submit_tag t(".filter"), :class => "btn btn-primary" %>
</div>
</div>

<% else %>
<%= render :partial => "notes_paging_nav" %>
<% if @notes.empty? %>
<h4><%= t ".no_notes" %></h4>
<% else %>
<%= render :partial => "notes_paging_nav" %>

<table class="table table-sm note_list">
<thead>
<tr>
<th></th>
<th><%= t ".id" %></th>
<th><%= t ".creator" %></th>
<th><%= t ".description" %></th>
<th><%= t ".created_at" %></th>
<th><%= t ".last_changed" %></th>
<table class="table table-sm note_list">
<thead>
<tr>
<th></th>
<th><%= t ".id" %></th>
<th><%= t ".creator" %></th>
<th><%= t ".description" %></th>
<th><%= t ".created_at" %></th>
<th><%= t ".last_changed" %></th>
</tr>
</thead>
<% @notes.each do |note| -%>
<tr<% if note.author == @user %> class="table-primary"<% end %>>
<td>
<% if note.closed? %>
<%= image_tag("closed_note_marker.svg", :alt => "closed", :width => 25, :height => 40) %>
<% else %>
<%= image_tag("open_note_marker.svg", :alt => "open", :width => 25, :height => 40) %>
<% end %>
</td>
<td><%= link_to note.id, note %></td>
<td><%= note_author(note.author) %></td>
<td><%= note.comments.first.body.to_html %></td>
<td><%= friendly_date_ago(note.created_at) %></td>
<td><%= friendly_date_ago(note.updated_at) %></td>
</tr>
</thead>
<% @notes.each do |note| -%>
<tr<% if note.author == @user %> class="table-primary"<% end %>>
<td>
<% if note.closed? %>
<%= image_tag("closed_note_marker.svg", :alt => "closed", :width => 25, :height => 40) %>
<% else %>
<%= image_tag("open_note_marker.svg", :alt => "open", :width => 25, :height => 40) %>
<% end %>
</td>
<td><%= link_to note.id, note %></td>
<td><%= note_author(note.author) %></td>
<td><%= note.comments.first.body.to_html %></td>
<td><%= friendly_date_ago(note.created_at) %></td>
<td><%= friendly_date_ago(note.updated_at) %></td>
</tr>
<% end -%>
</table>

<%= render :partial => "notes_paging_nav" %>
<% end -%>
</table>

<%= render :partial => "notes_paging_nav" %>
<% end %>
<% end -%>
10 changes: 10 additions & 0 deletions config/locales/en.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2966,6 +2966,16 @@ en:
description: "Description"
created_at: "Created at"
last_changed: "Last changed"
open: "Open"
closed: "Closed"
all: "All"
filter: "Filter"
from: "From"
to: "To"
status: "Status"
interaction_type: "Interaction Type"
submitted: "Submitted"
commented: "Commented"
show:
title: "Note: %{id}"
description: "Description"
Expand Down
114 changes: 110 additions & 4 deletions test/controllers/notes_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def test_index_success
assert_select ".content-heading a[href='#{user_path first_user}']", :text => first_user.display_name
assert_select "table.note_list tbody tr", :count => 1

# Check for a regular user (second user)
get user_notes_path(second_user)
assert_response :success
assert_select ".content-heading a[href='#{user_path second_user}']", :text => second_user.display_name
Expand All @@ -55,10 +56,6 @@ def test_index_success

session_for(moderator_user)

get user_notes_path(first_user)
assert_response :success
assert_select "table.note_list tbody tr", :count => 1

get user_notes_path(second_user)
assert_response :success
assert_select "table.note_list tbody tr", :count => 2
Expand Down Expand Up @@ -184,4 +181,113 @@ def test_new_note
assert_template "notes/new"
assert_select "#sidebar_content a[href='#{login_path(:referer => new_note_path)}']", :count => 0
end

def test_index_filter_by_status
user = create(:user)
other_user = create(:user)

open_note = create(:note, :status => "open") do |note|
create(:note_comment, :note => note, :author => user)
end

closed_note = create(:note, :status => "closed") do |note|
create(:note_comment, :note => note, :author => user)
end

commented_note = create(:note, :status => "open") do |note|
create(:note_comment, :note => note, :author => other_user)
create(:note_comment, :note => note, :author => user)
end

anonymous_commented_note = create(:note, :status => "open") do |note|
create(:note_comment, :note => note, :author => nil)
create(:note_comment, :note => note, :author => user)
end

get user_notes_path(user), :params => { :status => "open" }
assert_response :success
assert_select "table.note_list tbody tr", :count => 3
assert_select "table.note_list tbody tr", :text => /#{open_note.id}/

get user_notes_path(user), :params => { :status => "closed" }
assert_response :success
assert_select "table.note_list tbody tr", :count => 1
assert_select "table.note_list tbody tr", :text => /#{closed_note.id}/

get user_notes_path(user), :params => { :status => "commented" }
assert_response :success
assert_select "table.note_list tbody tr", :count => 2
assert_select "table.note_list tbody tr", :text => /#{commented_note.id}/
assert_select "table.note_list tbody tr", :text => /#{anonymous_commented_note.id}/
end

def test_index_filter_by_date_range
user = create(:user)

old_note = create(:note, :created_at => 1.year.ago)
create(:note_comment, :note => old_note, :author => user)

recent_note = create(:note, :created_at => 1.day.ago)
create(:note_comment, :note => recent_note, :author => user)

middle_note = create(:note, :created_at => 6.months.ago)
create(:note_comment, :note => middle_note, :author => user)

very_recent_note = create(:note, :created_at => 2.hours.ago)
create(:note_comment, :note => very_recent_note, :author => user)

# Filter for notes created between 1 year ago + 1 day and 1 month ago (should only include middle_note)
get user_notes_path(user), :params => { :from => (1.year.ago + 1.day).to_date, :to => 1.month.ago.end_of_month.to_date }
assert_response :success
assert_select "table.note_list tbody tr", :count => 1
assert_select "table.note_list tbody tr", :text => /#{middle_note.id}/
end

def test_index_sort_by_params
user = create(:user)

older_note = create(:note)
create(:note_comment, :note => older_note, :author => user)
older_note.updated_at = 2.days.ago
older_note.created_at = 5.days.ago
older_note.save!

newer_note = create(:note)
create(:note_comment, :note => newer_note, :author => user)
newer_note.updated_at = 13.minutes.ago
newer_note.created_at = 3.days.ago
newer_note.save!

middle_note = create(:note)
create(:note_comment, :note => middle_note, :author => user)
middle_note.updated_at = 1.day.ago
middle_note.created_at = 4.days.ago
middle_note.save!

very_recent_note = create(:note)
create(:note_comment, :note => very_recent_note, :author => user)
very_recent_note.updated_at = 5.minutes.ago
very_recent_note.created_at = 1.day.ago
very_recent_note.save!

get user_notes_path(user), :params => { :sort_by => "updated_at", :sort_order => "asc" }
assert_response :success
assert_select "table.note_list tbody tr:first-child td:nth-child(6) time", :text => /2 days ago/
assert_select "table.note_list tbody tr:last-child td:nth-child(6) time", :text => /5 minutes ago/

get user_notes_path(user), :params => { :sort_by => "updated_at", :sort_order => "desc" }
assert_response :success
assert_select "table.note_list tbody tr:first-child td:nth-child(6) time", :text => /5 minutes ago/
assert_select "table.note_list tbody tr:last-child td:nth-child(6) time", :text => /2 days ago/

get user_notes_path(user), :params => { :sort_by => "created_at", :sort_order => "asc" }
assert_response :success
assert_select "table.note_list tbody tr:first-child td:nth-child(5) time", :text => /5 days ago/
assert_select "table.note_list tbody tr:last-child td:nth-child(5) time", :text => /1 day ago/
# created at
get user_notes_path(user), :params => { :sort_by => "created_at", :sort_order => "desc" }
assert_response :success
assert_select "table.note_list tbody tr:first-child td:nth-child(5) time", :text => /1 day ago/
assert_select "table.note_list tbody tr:last-child td:nth-child(5) time", :text => /5 days ago/
end
end
Loading
Loading