CLDC-2242 Add assigned to filter (#1775)

* feat: wip commit

* feat: rest of assigned to filter behaviour

* feat: start to update tests

* feat: update tests

* feat: update tests

* refactor: linting

* feat: add new test

* feat: add new test

* feat: update filters_count

* feat: let support users see all users

* feat: let users select users from stock owners/managing agents too

* refactor: typo

* feat: update tests

app/components/lettings_log_summary_component.html.erb

@@ -58,7 +58,7 @@
       <p class="govuk-body">
         Created <time datetime="<%= log.created_at.iso8601 %>"><%= log.created_at.to_formatted_s(:govuk_date) %></time>
         <% if log.created_by %>
-          <span class="app-log-summary__footer--actor">by <%= log.created_by.name || log.created_by.email %></span>
+          <span class="app-log-summary__footer--actor">Assigned to <%= log.created_by.name || log.created_by.email %></span>
         <% end %>
       </p>
     </footer>

app/components/sales_log_summary_component.html.erb

@@ -42,7 +42,7 @@
       <p class="govuk-body">
         Created <time datetime="<%= log.created_at.iso8601 %>"><%= log.created_at.to_formatted_s(:govuk_date) %></time>
         <% if log.created_by %>
-          <span class="app-log-summary__footer--actor">by <%= log.created_by.name || log.created_by.email %></span>
+          <span class="app-log-summary__footer--actor">Assigned to <%= log.created_by.name || log.created_by.email %></span>
         <% end %>
       </p>
     </footer>

app/helpers/filters_helper.rb

@@ -3,7 +3,8 @@ module FiltersHelper
     return false unless session[session_name_for(filter_type)]
 
     selected_filters = JSON.parse(session[session_name_for(filter_type)])
-    return true if selected_filters.blank? && filter == "user" && value == :all
+    return true if !selected_filters.key?("user") && filter == "assigned_to" && value == :all
+    return true if selected_filters["assigned_to"] == "specific_user" && filter == "assigned_to" && value == :specific_user
 
     return true if !selected_filters.key?("owning_organisation") && filter == "owning_organisation_select" && value == :all
     return true if !selected_filters.key?("managing_organisation") && filter == "managing_organisation_select" && value == :all
@@ -21,7 +22,7 @@ module FiltersHelper
     return false unless filters_json
 
     filters = JSON.parse(filters_json)
-    filters["user"] == "yours" ||
+    filters["user"].present? ||
       filters["organisation"].present? ||
       filters["managing_organisation"].present? ||
       filters["status"]&.compact_blank&.any? ||
@@ -48,6 +49,11 @@ module FiltersHelper
     [OpenStruct.new(id: "", name: "Select an option")] + organisation_options.map { |org| OpenStruct.new(id: org.id, name: org.name) }
   end
 
+  def assigned_to_filter_options(user)
+    user_options = user.support? ? User.all : (user.organisation.users + user.organisation.managing_agents.flat_map(&:users) + user.organisation.stock_owners.flat_map(&:users))
+    [OpenStruct.new(id: "", name: "Select an option")] + user_options.map { |user_option| OpenStruct.new(id: user_option.id, name: user_option.name) }
+  end
+
   def collection_year_options
     { "2023": "2023/24", "2022": "2022/23", "2021": "2021/22" }
   end
@@ -85,7 +91,7 @@ private
     filters.each.sum do |category, category_filters|
       if %w[status years bulk_upload_id].include?(category)
         category_filters.count(&:present?)
-      elsif %w[user organisation].include?(category)
+      elsif %w[assigned_to organisation].include?(category)
         category_filters != "all" ? 1 : 0
       else
         0

app/helpers/log_list_helper.rb

@@ -1,7 +1,7 @@
 module LogListHelper
   def display_delete_logs?(current_user, search_term, filter_type)
     if current_user.data_provider?
-      filter_selected?("user", "yours", filter_type)
+      filter_selected?("assigned_to", "you", filter_type)
     else
       any_filter_selected?(filter_type) || search_term.present?
     end

app/models/log.rb

@@ -38,11 +38,7 @@ class Log < ApplicationRecord
   }
   scope :filter_by_postcode, ->(postcode_full) { where("REPLACE(postcode_full, ' ', '') ILIKE ?", "%#{postcode_full.delete(' ')}%") }
   scope :filter_by_id, ->(id) { where(id:) }
-  scope :filter_by_user, lambda { |selected_user, user|
+  scope :filter_by_user, ->(selected_user, _user = nil) { selected_user.present? ? where(created_by: selected_user) : all }
-    if !selected_user.include?("all") && user.present?
-      where(created_by: user)
-    end
-  }
   scope :filter_by_bulk_upload_id, lambda { |bulk_upload_id, user|
     joins(:bulk_upload)
       .where(bulk_upload: { id: bulk_upload_id, user: })

app/models/user.rb

@@ -144,9 +144,9 @@ class User < ApplicationRecord
 
   def logs_filters(specific_org: false)
     if (support? && !specific_org) || organisation.has_managing_agents? || organisation.has_stock_owners?
-      %w[status years user managing_organisation owning_organisation bulk_upload_id]
+      %w[status years assigned_to user managing_organisation owning_organisation bulk_upload_id]
     else
-      %w[status years user bulk_upload_id]
+      %w[status years assigned_to user bulk_upload_id]
     end
   end
 

app/services/filter_manager.rb

@@ -21,9 +21,9 @@ class FilterManager
 
     filters.each do |category, values|
       next if Array(values).reject(&:empty?).blank?
-
       next if category == "owning_organisation" && all_orgs
       next if category == "managing_organisation" && all_orgs
+      next if category == "assigned_to"
 
       logs = logs.public_send("filter_by_#{category}", values, user)
     end
@@ -59,6 +59,9 @@ class FilterManager
     new_filters = new_filters.except("owning_organisation") if params["owning_organisation_select"] == "all"
     new_filters = new_filters.except("managing_organisation") if params["managing_organisation_select"] == "all"
 
+    new_filters = new_filters.except("user") if params["assigned_to"] == "all"
+    new_filters["user"] = current_user.id.to_s if params["assigned_to"] == "you"
+
     new_filters
   end
 

app/views/logs/_log_filters.html.erb

@@ -6,7 +6,6 @@
 
     <div class="app-filter__content">
       <%= form_with html: { method: :get } do |f| %>
-        <% all_or_yours = { "all": { label: "All" }, "yours": { label: "Yours" } } %>
 
         <div class="govuk-grid-row" style="white-space: nowrap">
           <p class="govuk-grid-column-one-half">
@@ -47,9 +46,21 @@
           <%= render partial: "filters/radio_filter",
                      locals: {
                        f:,
-                     options: all_or_yours,
+                       options: {
-                     label: "Logs",
+                         "all": { label: "Any user" },
+                         "you": { label: "You" },
+                         "specific_user": {
+                           label: "Specific user",
+                           conditional_filter: {
+                             type: "select",
+                             label: "User",
                              category: "user",
+                             options: assigned_to_filter_options(@current_user),
+                           },
+                         },
+                       },
+                       label: "Assigned to",
+                       category: "assigned_to",
                      } %>
 
          <% if @current_user.support? || @current_user.organisation.stock_owners.count > 1 && request.path == "/lettings-logs" %>

spec/components/lettings_log_summary_component_spec.rb

@@ -17,7 +17,7 @@ RSpec.describe LettingsLogSummaryComponent, type: :component do
       expect(result).to have_text("General needs")
       expect(result).to have_text("Tenancy starts #{Time.zone.today.strftime('%e %B %Y').strip}")
       expect(result).to have_text("Created #{Time.zone.today.strftime('%e %B %Y').strip}")
-      expect(result).to have_text("by Danny Rojas")
+      expect(result).to have_text("Assigned to Danny Rojas")
       expect(result).to have_content("Owned by\n              DLUHC")
       expect(result).to have_content("Managed by\n              DLUHC")
     end

spec/features/lettings_log_spec.rb

@@ -79,7 +79,7 @@ RSpec.describe "Lettings Log Features" do
         before do
           check("Not started")
           check("In progress")
-          choose("Yours")
+          choose("You")
           click_button("Apply filters")
         end
 
@@ -253,7 +253,7 @@ RSpec.describe "Lettings Log Features" do
       expect(page).not_to have_link "Delete logs"
       within ".app-filter" do
         check "status-in-progress-field"
-        choose "user-yours-field"
+        choose "assigned-to-you-field"
         click_button
       end
       expect(page).to have_selector "article.app-log-summary", count: 2

spec/features/organisation_spec.rb

@@ -194,7 +194,7 @@ RSpec.describe "User Features" do
         end
         check("years-2021-field")
         click_button("Apply filters")
-        expect(page).to have_current_path("/organisations/#{org_id}/lettings-logs?years[]=&years[]=2021&status[]=&user=all&owning_organisation_select=all&owning_organisation=")
+        expect(page).to have_current_path("/organisations/#{org_id}/lettings-logs?years[]=&years[]=2021&status[]=&assigned_to=all&user=&owning_organisation_select=all&owning_organisation=")
         expect(page).not_to have_link first_log.id.to_s, href: "/lettings-logs/#{first_log.id}"
       end
     end
@@ -227,7 +227,7 @@ RSpec.describe "User Features" do
         end
         check("years-2021-field")
         click_button("Apply filters")
-        expect(page).to have_current_path("/organisations/#{org_id}/sales-logs?years[]=&years[]=2021&status[]=&user=all&owning_organisation_select=all&owning_organisation=")
+        expect(page).to have_current_path("/organisations/#{org_id}/sales-logs?years[]=&years[]=2021&status[]=&assigned_to=all&user=&owning_organisation_select=all&owning_organisation=")
         expect(page).not_to have_link first_log.id.to_s, href: "/sales-logs/#{first_log.id}"
       end
     end

spec/features/sales_log_spec.rb

@@ -58,7 +58,7 @@ RSpec.describe "Sales Log Features" do
         expect(page).not_to have_link "Delete logs"
 
         within ".app-filter" do
-          choose "user-yours-field"
+          choose "assigned-to-you-field"
           click_button
         end
 
@@ -116,7 +116,7 @@ RSpec.describe "Sales Log Features" do
         before do
           check("Not started")
           check("In progress")
-          choose("Yours")
+          choose("You")
           click_button("Apply filters")
         end
 

spec/features/user_spec.rb

@@ -726,10 +726,10 @@ RSpec.describe "User Features" do
           expect(page).to have_field("owning-organisation-field", with: "")
           find("#owning-organisation-field").click.native.send_keys("F", "i", "l", "t", :down, :enter)
           click_button("Apply filters")
-          expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&owning_organisation_select=specific_org&owning_organisation=#{parent_organisation.id}&managing_organisation_select=all")
+          expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&assigned_to=all&owning_organisation_select=specific_org&owning_organisation=#{parent_organisation.id}&managing_organisation_select=all")
           choose("owning-organisation-select-all-field", allow_label_click: true)
           click_button("Apply filters")
-          expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&owning_organisation_select=all&managing_organisation_select=all")
+          expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&assigned_to=all&owning_organisation_select=all&managing_organisation_select=all")
         end
       end
     end

spec/helpers/filters_helper_spec.rb

@@ -16,8 +16,9 @@ RSpec.describe FiltersHelper do
 
       context "when looking at the all value" do
         it "returns true if no filters have been set yet" do
-          expect(filter_selected?("user", :all, "lettings_logs")).to be true
+          expect(filter_selected?("assigned_to", :all, "lettings_logs")).to be true
-          expect(filter_selected?("user", :yours, "lettings_logs")).to be false
+          expect(filter_selected?("assigned_to", :you, "lettings_logs")).to be false
+          expect(filter_selected?("assigned_to", :specific_user, "lettings_logs")).to be false
         end
       end
     end
@@ -60,7 +61,7 @@ RSpec.describe FiltersHelper do
 
     context "when the specific owning organisation filter is not set" do
       before do
-        session[:lettings_logs_filters] = { "status" => [""], "years" => [""], "user" => "all" }.to_json
+        session[:lettings_logs_filters] = { "status" => [""], "years" => [""], "assigned_to" => "all" }.to_json
       end
 
       it "marks the all options as checked" do
@@ -85,15 +86,15 @@ RSpec.describe FiltersHelper do
     end
 
     context "when organisation and user are set to all" do
-      let(:filters) { { "owning_organisation_select" => "all", "user" => "all" } }
+      let(:filters) { { "owning_organisation_select" => "all", "assigned_to" => "all" } }
 
       it "returns false" do
         expect(result).to be_falsey
       end
     end
 
-    context "when user is set to 'yours'" do
+    context "when user is set to 'you'" do
-      let(:filters) { { "user" => "yours" } }
+      let(:filters) { { "user" => "you" } }
 
       it "returns true" do
         expect(result).to be true
@@ -135,7 +136,7 @@ RSpec.describe FiltersHelper do
     context "when a range of filters is applied" do
       let(:filters) do
         {
-          "user" => "all",
+          "assigned_to" => "all",
           "status" => %w[in_progress completed],
           "years" => [""],
           "organisation" => 2,
@@ -218,7 +219,7 @@ RSpec.describe FiltersHelper do
     context "when no filters are applied" do
       let(:filters) do
         {
-          "user" => "all",
+          "assigned_to" => "all",
           "status" => [""],
           "years" => [""],
           "organisation" => "all",
@@ -233,7 +234,7 @@ RSpec.describe FiltersHelper do
     context "when a range of filters is applied" do
       let(:filters) do
         {
-          "user" => "all",
+          "assigned_to" => "all",
           "status" => %w[in_progress completed],
           "years" => [""],
           "organisation" => 2,

spec/helpers/log_list_helper_spec.rb

@@ -16,22 +16,22 @@ RSpec.describe LogListHelper, type: :helper do
         expect(result).to be false
       end
 
-      it "returns true if the user filter is set to 'yours'" do
+      it "returns true if the assigned to filter is set to you" do
-        allow(self).to receive(:filter_selected?).with("user", "yours", filter_type).and_return true
+        allow(self).to receive(:filter_selected?).with("assigned_to", "you", filter_type).and_return true
         expect(result).to be true
       end
 
       it "returns false if any filters other than the user filter are set" do
         allow(self).to receive(:filter_selected?).and_return true
-        allow(self).to receive(:filter_selected?).with("user", "yours", filter_type).and_return false
+        allow(self).to receive(:filter_selected?).with("assigned_to", "you", filter_type).and_return false
         expect(result).to be false
       end
 
       context "when there is a search term present" do
         let(:search_term) { "word" }
 
-        it "still returns false as long as the user filter is not set to yours" do
+        it "still returns false as long as the assigned to filter is not set to you" do
-          allow(self).to receive(:filter_selected?).with("user", "yours", filter_type).and_return false
+          allow(self).to receive(:filter_selected?).with("assigned_to", "you", filter_type).and_return false
           expect(result).to be false
         end
       end

spec/jobs/email_csv_job_spec.rb

@@ -12,7 +12,7 @@ describe EmailCsvJob do
   let(:sales_log_csv_service) { instance_double(Csv::SalesLogCsvService) }
   let(:lettings_log_csv_service) { instance_double(Csv::LettingsLogCsvService) }
   let(:search_term) { "meaning" }
-  let(:filters) { { "user" => "yours", "status" => %w[in_progress] } }
+  let(:filters) { { "user" => "you", "status" => %w[in_progress] } }
   let(:all_orgs) { false }
   let(:organisation) { build(:organisation) }
   let(:codes_only_export) { true }

spec/models/forms/delete_logs_form_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe Forms::DeleteLogsForm do
     {
       "years" => [""],
       "status" => ["", "completed"],
-      "user" => "yours",
+      "user" => "you",
     }
   end
   let(:selected_ids) { [visible_logs.first.id] }

spec/models/lettings_log_spec.rb

@@ -2824,15 +2824,11 @@ RSpec.describe LettingsLog do
       end
 
       it "allows filtering on current user" do
-        expect(described_class.filter_by_user(%w[yours], created_by_user).count).to eq(2)
+        expect(described_class.filter_by_user(created_by_user.id.to_s).count).to eq(2)
       end
 
       it "returns all logs when all logs selected" do
-        expect(described_class.filter_by_user(%w[all], created_by_user).count).to eq(3)
+        expect(described_class.filter_by_user(nil).count).to eq(3)
-      end
-
-      it "returns all logs when all and your users selected" do
-        expect(described_class.filter_by_user(%w[all yours], created_by_user).count).to eq(3)
       end
     end
 

spec/models/user_spec.rb

@@ -142,7 +142,7 @@ RSpec.describe User, type: :model do
         end
 
         it "can filter lettings logs by user, year and status" do
-          expect(user.logs_filters).to eq(%w[status years user bulk_upload_id])
+          expect(user.logs_filters).to eq(%w[status years assigned_to user bulk_upload_id])
         end
       end
 
@@ -152,7 +152,7 @@ RSpec.describe User, type: :model do
         end
 
         it "can filter lettings logs by user, year, status, managing_organisation and owning_organisation" do
-          expect(user.logs_filters).to match_array(%w[status years user managing_organisation owning_organisation bulk_upload_id])
+          expect(user.logs_filters).to match_array(%w[status years assigned_to user managing_organisation owning_organisation bulk_upload_id])
         end
       end
     end
@@ -193,7 +193,7 @@ RSpec.describe User, type: :model do
       end
 
       it "can filter lettings logs by user, year, status, managing_organisation and owning_organisation" do
-        expect(user.logs_filters).to match_array(%w[status years user owning_organisation managing_organisation bulk_upload_id])
+        expect(user.logs_filters).to match_array(%w[status years assigned_to user owning_organisation managing_organisation bulk_upload_id])
       end
     end
 

spec/requests/delete_logs_controller_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe "DeleteLogs", type: :request do
       logs_filters = {
         "years" => [""],
         "status" => ["", "in_progress"],
-        "user" => "all",
+        "assigned_to" => "all",
       }
       get lettings_logs_path(logs_filters) # adds the filters to the session
 
@@ -71,7 +71,7 @@ RSpec.describe "DeleteLogs", type: :request do
       logs_filters = {
         "years" => [""],
         "status" => ["", "in_progress"],
-        "user" => "all",
+        "assigned_to" => "all",
       }
       get lettings_logs_path(logs_filters) # adds the filters to the session
 
@@ -262,7 +262,7 @@ RSpec.describe "DeleteLogs", type: :request do
       logs_filters = {
         "years" => [""],
         "status" => ["", "in_progress"],
-        "user" => "all",
+        "assigned_to" => "all",
       }
       get sales_logs_path(logs_filters) # adds the filters to the session
 
@@ -311,7 +311,7 @@ RSpec.describe "DeleteLogs", type: :request do
       logs_filters = {
         "years" => [""],
         "status" => ["", "in_progress"],
-        "user" => "all",
+        "assigned_to" => "all",
       }
       get sales_logs_path(logs_filters) # adds the filters to the session
 
@@ -506,7 +506,7 @@ RSpec.describe "DeleteLogs", type: :request do
         logs_filters = {
           "years" => [""],
           "status" => ["", "in_progress"],
-          "user" => "all",
+          "assigned_to" => "all",
         }
         get lettings_logs_path(logs_filters) # adds the filters to the session
 
@@ -555,7 +555,7 @@ RSpec.describe "DeleteLogs", type: :request do
         logs_filters = {
           "years" => [""],
           "status" => ["", "in_progress"],
-          "user" => "all",
+          "assigned_to" => "all",
         }
         get lettings_logs_path(logs_filters) # adds the filters to the session
 
@@ -731,7 +731,7 @@ RSpec.describe "DeleteLogs", type: :request do
         logs_filters = {
           "years" => [""],
           "status" => ["", "in_progress"],
-          "user" => "all",
+          "assigned_to" => "all",
         }
         get sales_logs_path(logs_filters) # adds the filters to the session
 
@@ -780,7 +780,7 @@ RSpec.describe "DeleteLogs", type: :request do
         logs_filters = {
           "years" => [""],
           "status" => ["", "in_progress"],
-          "user" => "all",
+          "assigned_to" => "all",
         }
         get sales_logs_path(logs_filters) # adds the filters to the session
 

spec/requests/lettings_logs_controller_spec.rb

@@ -484,7 +484,7 @@ RSpec.describe LettingsLogsController, type: :request do
                 expect(page).not_to have_content(excluded_log.tenancycode)
               end
 
-              it "dislays how many logs remaining to fix" do
+              it "displays how many logs remaining to fix" do
                 get "/lettings-logs?bulk_upload_id[]=#{bulk_upload.id}"
                 expect(page).to have_content("You need to fix 1 log")
               end
@@ -780,7 +780,7 @@ RSpec.describe LettingsLogsController, type: :request do
           let(:tenant_code_2) { "TC8745" }
 
           before do
-            FactoryBot.create(:lettings_log, :in_progress, owning_organisation: org_1, tenancycode: tenant_code_1)
+            FactoryBot.create(:lettings_log, :in_progress, owning_organisation: org_1, tenancycode: tenant_code_1, created_by: user)
             FactoryBot.create(:lettings_log, :in_progress, owning_organisation: org_2, tenancycode: tenant_code_2)
             allow(user).to receive(:need_two_factor_authentication?).and_return(false)
             sign_in user
@@ -792,9 +792,17 @@ RSpec.describe LettingsLogsController, type: :request do
             expect(page).to have_content(tenant_code_2)
           end
 
+          context "when filtering by a specific user" do
+            it "only show the selected user's logs" do
+              get "/lettings-logs?assigned_to=specific_user&user=#{user.id}", headers:, params: {}
+              expect(page).to have_content(tenant_code_1)
+              expect(page).not_to have_content(tenant_code_2)
+            end
+          end
+
           context "when the support user has filtered by organisation, then switches back to all organisations" do
             it "shows all organisations" do
-              get "http://localhost:3000/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&organisation_select=all&organisation=#{org_1.id}", headers:, params: {}
+              get "http://localhost:3000/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&assigned_to=all&organisation_select=all&organisation=#{org_1.id}", headers:, params: {}
               expect(page).to have_content(tenant_code_1)
               expect(page).to have_content(tenant_code_2)
             end

spec/requests/sales_logs_controller_spec.rb

@@ -362,7 +362,7 @@ RSpec.describe SalesLogsController, type: :request do
                 expect(page).not_to have_content(excluded_log.id)
               end
 
-              it "dislays how many logs remaining to fix" do
+              it "displays how many logs remaining to fix" do
                 get "/sales-logs?bulk_upload_id[]=#{bulk_upload.id}"
                 expect(page).to have_content("You need to fix 1 log")
               end