temp

app/controllers/duplicate_logs_controller.rb

@@ -2,7 +2,6 @@ class DuplicateLogsController < ApplicationController
   include DuplicateLogsHelper
 
   before_action :authenticate_user!
-  before_action :authenticate_scope!, only: [:index]
   before_action :find_resource_by_named_id
   before_action :find_duplicate_logs
   before_action :find_original_log
@@ -26,12 +25,19 @@ class DuplicateLogsController < ApplicationController
   end
 
   def index
-    @duplicates = params.permit(duplicates: {})[:duplicates]&.to_h || duplicates_for_user(current_user)
-    return render_not_found unless @duplicates
+    if current_user.data_provider?
+      @duplicates = duplicates_for_user(current_user)
+    elsif current_user.support?
+      organisation = Organisation.find(params[:organisation_id])
+      render_not_found unless organisation
+      @duplicates = duplicates_for_organisation(organisation)
+    elsif current_user.data_coordinator?
+      @duplicates = duplicates_for_organisation(current_user.organisation)
+    end
 
-    @duplicates[:lettings] ||= {}
-    @duplicates[:sales] ||= {}
     @duplicate_sets_count = @duplicates[:lettings].count + @duplicates[:sales].count
+
+    render_not_found if @duplicate_sets_count.zero?
   end
 
 private

app/controllers/lettings_logs_controller.rb

@@ -13,10 +13,6 @@ class LettingsLogsController < LogsController
   before_action :redirect_if_bulk_upload_resolved, only: [:index]
 
   def index
-    if current_user.data_provider? && (@duplicates = duplicates_for_user(current_user))
-      @duplicate_sets_count = @duplicates[:lettings].count + @duplicates[:sales].count
-    end
-
     all_logs = current_user.lettings_logs.visible
     unpaginated_filtered_logs = filter_manager.filtered_logs(all_logs, search_term, session_filters)
 
@@ -26,6 +22,7 @@ class LettingsLogsController < LogsController
     @total_count = all_logs.size
     @unresolved_count = all_logs.unresolved.created_by(current_user).count
     @filter_type = "lettings_logs"
+    @duplicate_sets_count = duplicate_sets_count(current_user, nil)
     render "logs/index"
   end
 

app/controllers/organisations_controller.rb

@@ -1,6 +1,7 @@
 class OrganisationsController < ApplicationController
   include Pagy::Backend
   include Modules::SearchFilter
+  include DuplicateLogsHelper
 
   before_action :authenticate_user!
   before_action :find_resource, except: %i[index new create]
@@ -96,18 +97,15 @@ class OrganisationsController < ApplicationController
     organisation_logs = LettingsLog.visible.where(owning_organisation_id: @organisation.id)
     unpaginated_filtered_logs = filter_manager.filtered_logs(organisation_logs, search_term, session_filters)
 
-    respond_to do |format|
-      format.html do
-        @search_term = search_term
-        @pagy, @logs = pagy(unpaginated_filtered_logs)
-        @delete_logs_path = delete_lettings_logs_organisation_path(search: @search_term)
-        @searched = search_term.presence
-        @total_count = organisation_logs.size
-        @log_type = :lettings
-        @filter_type = "lettings_logs"
-        render "logs", layout: "application"
-      end
-    end
+    @search_term = search_term
+    @pagy, @logs = pagy(unpaginated_filtered_logs)
+    @delete_logs_path = delete_lettings_logs_organisation_path(search: @search_term)
+    @searched = search_term.presence
+    @total_count = organisation_logs.size
+    @log_type = :lettings
+    @filter_type = "lettings_logs"
+    @duplicate_sets_count = duplicate_sets_count(current_user, @organisation)
+    render "logs", layout: "application"
   end
 
   def download_lettings_csv
@@ -136,6 +134,7 @@ class OrganisationsController < ApplicationController
         @total_count = organisation_logs.size
         @log_type = :sales
         @filter_type = "sales_logs"
+        @duplicate_sets_count = duplicate_sets_count(current_user, @organisation)
         render "logs", layout: "application"
       end
 

app/controllers/sales_logs_controller.rb

@@ -1,4 +1,6 @@
 class SalesLogsController < LogsController
+  include DuplicateLogsHelper
+
   rescue_from ActiveRecord::RecordNotFound, with: :render_not_found
 
   before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv]
@@ -13,20 +15,17 @@ class SalesLogsController < LogsController
   end
 
   def index
-    respond_to do |format|
-      format.html do
-        all_logs = current_user.sales_logs.visible
-        unpaginated_filtered_logs = filter_manager.filtered_logs(all_logs, search_term, session_filters)
-
-        @delete_logs_path = delete_logs_sales_logs_path(search: search_term)
-        @search_term = search_term
-        @pagy, @logs = pagy(unpaginated_filtered_logs)
-        @searched = search_term.presence
-        @total_count = all_logs.size
-        @filter_type = "sales_logs"
-        render "logs/index"
-      end
-    end
+    all_logs = current_user.sales_logs.visible
+    unpaginated_filtered_logs = filter_manager.filtered_logs(all_logs, search_term, session_filters)
+
+    @delete_logs_path = delete_logs_sales_logs_path(search: search_term)
+    @search_term = search_term
+    @pagy, @logs = pagy(unpaginated_filtered_logs)
+    @searched = search_term.presence
+    @total_count = all_logs.size
+    @filter_type = "sales_logs"
+    @duplicate_sets_count = duplicate_sets_count(current_user, nil)
+    render "logs/index"
   end
 
   def show

app/helpers/duplicate_logs_helper.rb

@@ -29,38 +29,64 @@ module DuplicateLogsHelper
   end
   
   def duplicates_for_user(user)
-    duplicate_sets = { lettings: {}, sales: {} }
-    lettings_count = 0
-    sales_count = 0
-    duplicate_lettings_ids = Set.new
-    duplicate_sales_ids = Set.new
+    {
+      lettings: lettings_duplicate_sets_from_collection(LettingsLog.created_by(user), user.organisation),
+      sales: sales_duplicate_sets_from_collection(SalesLog.created_by(user), user.organisation),
+    }
+  end
+
+  def duplicates_for_organisation(organisation)
+    {
+      lettings: lettings_duplicate_sets_from_collection(LettingsLog.filter_by_organisation(organisation), organisation),
+      sales: sales_duplicate_sets_from_collection(SalesLog.filter_by_organisation(organisation), organisation),
+    }
+  end
 
-    LettingsLog.created_by(user).visible.each do |log|
-      next if duplicate_lettings_ids.include? log.id
+  def sales_duplicate_sets_from_collection(logs, organisation)
+    duplicate_sets = []
+    duplicate_ids_seen = Set.new
 
-      duplicates = user.lettings_logs.duplicate_logs(log)
+    logs.visible.each do |log|
+      next if duplicate_ids_seen.include? log.id
+
+      duplicates = SalesLog.filter_by_organisation(organisation).duplicate_logs(log)
       next if duplicates.none?
 
       duplicate_ids = [log.id, *duplicates.map(&:id)]
-      duplicate_sets[:lettings][lettings_count] = duplicate_ids
-      lettings_count += 1
-      duplicate_lettings_ids << duplicate_ids
+      duplicate_sets << duplicate_ids
+      duplicate_ids_seen.merge duplicate_ids
     end
 
-    SalesLog.created_by(user).visible.each do |log|
-      next if duplicate_sales_ids.include? log.id
+    duplicate_sets
+  end
+
+  def lettings_duplicate_sets_from_collection(logs, organisation)
+    duplicate_sets = []
+    duplicate_ids_seen = Set.new
+
+    logs.visible.each do |log|
+      next if duplicate_ids_seen.include? log.id
 
-      duplicates = user.sales_logs.duplicate_logs(log)
+      duplicates = LettingsLog.filter_by_organisation(organisation).duplicate_logs(log)
       next if duplicates.none?
 
       duplicate_ids = [log.id, *duplicates.map(&:id)]
-      duplicate_sets[:sales][sales_count] = duplicate_ids
-      sales_count += 1
-      duplicate_sales_ids << duplicate_ids
+      duplicate_sets << duplicate_ids
+      duplicate_ids_seen.merge duplicate_ids
     end
 
-    return if duplicate_lettings_ids.empty? && duplicate_sales_ids.empty?
-
     duplicate_sets
   end
+
+  def duplicate_sets_count(user, organisation)
+    duplicates = if user.support?
+                   duplicates_for_organisation(organisation)
+                 elsif user.data_coordinator?
+                   duplicates_for_organisation(user.organisation)
+                 elsif user.data_provider?
+                   duplicates_for_user(user)
+                 end
+
+    duplicates[:lettings].count + duplicates[:sales].count
+  end
 end

app/models/sales_log.rb

@@ -49,7 +49,7 @@ class SalesLog < Log
     .where.not(sex1: nil)
     .where.not(ecstat1: nil)
     .where.not(postcode_full: nil)
-    .where("age1 IS NOT NULL OR age1_known = 1 OR age1_known = 2")
+    .age1_answered
   }
   scope :after_date, ->(date) { where("saledate >= ?", date) }
 

app/views/duplicate_logs/index.html.erb

@@ -7,7 +7,7 @@
     <% end %>
   <% end %>
   <%= table.body do |body| %>
-    <% @duplicates[:lettings].each do |_i, duplicate_set| %>
+    <% @duplicates[:lettings].each do |duplicate_set| %>
       <% body.row do |row| %>
         <% row.cell text: "Lettings" %>
         <% row.cell text: duplicate_set.map { |id| "Log #{id}" }.join(", ") %>
@@ -16,7 +16,7 @@
         <% end %>
       <% end %>
     <% end %>
-    <% @duplicates[:sales].each do |_i, duplicate_set| %>
+    <% @duplicates[:sales].each do |duplicate_set| %>
       <% body.row do |row| %>
         <% row.cell text: "Sales" %>
         <% row.cell text: duplicate_set.map { |id| "Log #{id}" }.join(", ") %>

app/views/logs/index.html.erb

@@ -8,8 +8,8 @@
   organisation: @organisation,
 ) %>
 
-<% if @duplicates %>
-  <%= govuk_notification_banner(title_text: "Important", text: govuk_link_to("Review logs", duplicate_logs_path(duplicates: @duplicates))) do |banner|
+<% if @duplicate_sets_count&.positive? %>
+  <%= govuk_notification_banner(title_text: "Important", text: govuk_link_to("Review logs", duplicate_logs_path)) do |banner|
     banner.with_heading(text: I18n.t("notification.duplicate_sets", count: @duplicate_sets_count))
   end %>
 <% end %>

app/views/organisations/logs.html.erb

@@ -10,6 +10,12 @@
   organisation: @organisation,
 ) %>
 
+<% if @duplicate_sets_count&.positive? %>
+  <%= govuk_notification_banner(title_text: "Important", text: govuk_link_to("Review logs", organisation_duplicates_path(@organisation))) do |banner|
+    banner.with_heading(text: I18n.t("notification.duplicate_sets", count: @duplicate_sets_count))
+  end %>
+<% end %>
+
 <% if current_user.support? %>
   <%= render SubNavigationComponent.new(
     items: secondary_items(request.path, @organisation.id),

config/routes.rb

@@ -119,6 +119,8 @@ Rails.application.routes.draw do
   end
 
   resources :organisations do
+    get "duplicates", to: "duplicate_logs#index"
+
     member do
       get "details", to: "organisations#details"
       get "data-sharing-agreement", to: "organisations#data_sharing_agreement"

spec/helpers/duplicate_logs_helper_spec.rb

@@ -6,61 +6,64 @@ RSpec.describe DuplicateLogsHelper do
   let(:current_user) { create(:user, organisation: org) }
   let(:user_same_org) { create(:user, organisation: org) }
   let(:user_other_org) { create(:user, organisation: other_org) }
-
-  let(:result) { duplicates_for_user(current_user) }
+  let(:empty_duplicates) { { lettings: [], sales: [] } }
 
   let!(:lettings_log) { create(:lettings_log, :duplicate, created_by: current_user) }
   let!(:sales_log) { create(:sales_log, :duplicate, created_by: current_user) }
 
-  context "when there are no duplicates" do
-    it "returns nil" do
-      expect(result).to be nil
-    end
-  end
+  describe "#duplicates_for_user" do
+    let(:result) { duplicates_for_user(current_user) }
 
-  context "when there are duplicates in another org" do
-    before do
-      create(:lettings_log, :duplicate, created_by: user_other_org)
-      create(:sales_log, :duplicate, created_by: user_other_org)
+    context "when there are no duplicates" do
+      it "returns nil" do
+        expect(result).to eq empty_duplicates
+      end
     end
 
-    it "returns nil" do
-      expect(result).to be nil
+    context "when there are duplicates in another org" do
+      before do
+        create(:lettings_log, :duplicate, created_by: user_other_org)
+        create(:sales_log, :duplicate, created_by: user_other_org)
+      end
+
+      it "does not locate duplicates" do
+        expect(result).to eq empty_duplicates
+      end
     end
-  end
 
-  context "when another user in the same org has created a duplicate lettings log" do
-    let!(:duplicate_lettings_log) { create(:lettings_log, :duplicate, created_by: user_same_org) }
+    context "when another user in the same org has created a duplicate lettings log" do
+      let!(:duplicate_lettings_log) { create(:lettings_log, :duplicate, created_by: user_same_org) }
 
-    it "returns the ids of the duplicates in a hash under the lettings key" do
-      expect(result).to be_a Hash
-      expect(result[:lettings].values).to match_array [[lettings_log.id, duplicate_lettings_log.id]]
+      it "returns the ids of the duplicates in a hash under the lettings key" do
+        expect(result).to be_a Hash
+        expect(result[:lettings]).to match_array [[lettings_log.id, duplicate_lettings_log.id]]
+      end
     end
-  end
 
-  context "when another user in the same org has created a duplicate sales log" do
-    let!(:duplicate_sales_log) { create(:sales_log, :duplicate, created_by: user_same_org) }
+    context "when another user in the same org has created a duplicate sales log" do
+      let!(:duplicate_sales_log) { create(:sales_log, :duplicate, created_by: user_same_org) }
 
-    it "returns the ids of the duplicates in a hash under the sales key" do
-      expect(result).to be_a Hash
-      expect(result[:sales].values).to match_array [[sales_log.id, duplicate_sales_log.id]]
+      it "returns the ids of the duplicates in a hash under the sales key" do
+        expect(result).to be_a Hash
+        expect(result[:sales]).to match_array [[sales_log.id, duplicate_sales_log.id]]
+      end
     end
-  end
 
-  context "when there are multiple sets of duplicates across lettings and sales" do
-    let!(:duplicate_lettings_log) { create(:lettings_log, :duplicate, created_by: user_same_org) }
-    let!(:duplicate_sales_log) { create(:sales_log, :duplicate, created_by: user_same_org) }
-    let!(:further_sales_log) { create(:sales_log, :duplicate, purchid: "further", created_by: current_user) }
-    let!(:further_duplicate_sales_logs) { create_list(:sales_log, 2, :duplicate, purchid: "further", created_by: user_same_org) }
+    context "when there are multiple sets of duplicates across lettings and sales" do
+      let!(:duplicate_lettings_log) { create(:lettings_log, :duplicate, created_by: user_same_org) }
+      let!(:duplicate_sales_log) { create(:sales_log, :duplicate, created_by: user_same_org) }
+      let!(:further_sales_log) { create(:sales_log, :duplicate, purchid: "further", created_by: current_user) }
+      let!(:further_duplicate_sales_logs) { create_list(:sales_log, 2, :duplicate, purchid: "further", created_by: user_same_org) }
 
-    it "returns them all with no repeats" do
-      expected_sales_duplicates_result = [
-        [sales_log.id, duplicate_sales_log.id],
-        [further_sales_log.id, *further_duplicate_sales_logs.map(&:id)],
-      ]
+      it "returns them all with no repeats" do
+        expected_sales_duplicates_result = [
+          [sales_log.id, duplicate_sales_log.id],
+          [further_sales_log.id, *further_duplicate_sales_logs.map(&:id)],
+        ]
 
-      expect(result[:lettings].values).to match_array [[lettings_log.id, duplicate_lettings_log.id]]
-      expect(result[:sales].values).to match_array expected_sales_duplicates_result
+        expect(result[:lettings]).to match_array [[lettings_log.id, duplicate_lettings_log.id]]
+        expect(result[:sales]).to match_array expected_sales_duplicates_result
+      end
     end
   end
 end