From d400b9cc986ad2e6a2681e56c12dabf6e29b6271 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Mon, 23 Sep 2024 10:43:24 +0100
Subject: [PATCH 1/7] CLDC-3611 Add performance tests (#2637)

* Add test to the pipeline

* Update performance_test.sh

* Set email and password from workflow

* lint

* Set credentials in workflow

* Run more tests against staging

* Remove concurrency

* Rmove hardcoded log ID

* Do a test post with curl

* Add Accept variable document length flag

* Update code check in the pipeline

* Remove seed user

* Move performance pipeline to staging

* lint

* Update task group
---
 .github/workflows/staging_pipeline.yml |  50 ++++++++++
 Dockerfile                             |   6 ++
 lib/tasks/performance_test.sh          | 124 +++++++++++++++++++++++++
 3 files changed, 180 insertions(+)
 create mode 100644 lib/tasks/performance_test.sh

diff --git a/.github/workflows/staging_pipeline.yml b/.github/workflows/staging_pipeline.yml
index 415a943d1..8c0553741 100644
--- a/.github/workflows/staging_pipeline.yml
+++ b/.github/workflows/staging_pipeline.yml
@@ -14,6 +14,11 @@ defaults:
   run:
     shell: bash
 
+env:
+  app_repo_role: arn:aws:iam::815624722760:role/core-application-repo
+  aws_region: eu-west-2
+  repository: core
+
 jobs:
   test:
     name: Tests
@@ -364,3 +369,48 @@ jobs:
       environment: staging
     permissions:
       id-token: write
+
+  performance:
+    needs: [aws_deploy]
+    runs-on: ubuntu-latest
+    permissions:
+      id-token: write
+
+    steps:
+      - name: Configure AWS credentials
+        uses: aws-actions/configure-aws-credentials@v3
+        with:
+          aws-region: ${{ env.aws_region }}
+          role-to-assume: ${{ env.app_repo_role }}
+
+      - name: Configure AWS credentials for the environment
+        uses: aws-actions/configure-aws-credentials@v3
+        with:
+          aws-region: eu-west-2
+          role-to-assume: arn:aws:iam::107155005276:role/core-staging-deployment
+          role-chaining: true
+
+      - name: Run Performance Test
+        env:
+          ad_hoc_task_definition: core-staging-ad-hoc
+          cluster: core-staging-app
+          service: core-staging-app
+        run: |
+          echo $cluster
+          network=$(aws ecs describe-services --cluster $cluster --services $service --query services[0].networkConfiguration)
+          overrides='{
+            "containerOverrides": [{
+              "name": "app",
+              "command": ["bash", "-c", "export email=$STAGING_PERFORMANCE_TEST_EMAIL && export password=$STAGING_PERFORMANCE_TEST_PASSWORD && sh ./lib/tasks/performance_test.sh"]
+            }]
+          }'
+          arn=$(aws ecs run-task --cluster $cluster --task-definition $ad_hoc_task_definition --network-configuration "$network" --overrides "$overrides" --group performance --launch-type FARGATE --query tasks[0].taskArn)
+
+          echo "Waiting for performance tests to run"
+          task_id=${arn##*/}
+          task_id=${task_id%*\"}
+
+          aws ecs wait tasks-stopped --cluster $cluster --tasks $task_id
+
+          code=$(aws ecs describe-tasks --cluster $cluster --tasks $task_id --query "tasks[0].containers[0].exitCode")
+          if [ "$code == 0" ]; then exit 0; else exit 1; fi
diff --git a/Dockerfile b/Dockerfile
index 6eeee6511..46bd31b5f 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -31,6 +31,9 @@ EXPOSE ${PORT}
 
 RUN adduser --system --no-create-home nonroot
 
+RUN apk add curl
+RUN apk add apache2-utils
+
 FROM base as test
 
 RUN bundle config set without ""
@@ -67,6 +70,9 @@ RUN mkdir -p tmp log
 RUN chown -R nonroot tmp log
 RUN chown nonroot db/schema.rb
 
+RUN mkdir -p performance_test
+RUN chown -R nonroot performance_test
+
 USER nonroot
 
 CMD bundle exec rails s -e ${RAILS_ENV} -p ${PORT} --binding=0.0.0.0
diff --git a/lib/tasks/performance_test.sh b/lib/tasks/performance_test.sh
new file mode 100644
index 000000000..6fdc4248a
--- /dev/null
+++ b/lib/tasks/performance_test.sh
@@ -0,0 +1,124 @@
+cd performance_test
+
+# Lettings logs page
+echo "Get token"
+TOKEN=$(curl -c token_cookies.txt -s https://staging.submit-social-housing-data.levellingup.gov.uk/account/sign-in | grep '<meta name="csrf-token"' | sed -n 's/.*content="\([^"]*\)".*/\1/p')
+
+echo "Logging in..."
+echo $email
+curl -L -o nul -c login_cookies.txt -b token_cookies.txt -X POST https://staging.submit-social-housing-data.levellingup.gov.uk/account/sign-in \
+  -d "user[email]=$email" \
+  -d "user[password]=$password" \
+  -d "authenticity_token=$TOKEN"
+
+COOKIES=$(awk '/_data_collector_session/ { print $6, $7 }' login_cookies.txt | tr ' ' '=')
+
+echo "Running lettings logs page performance test..."
+ab -n 50 -c 50 -l -C "$COOKIES" 'https://staging.submit-social-housing-data.levellingup.gov.uk/lettings-logs?years[]=2024&status[]=completed' > performance_lettings_test_results.txt
+file="performance_lettings_test_results.txt"
+
+failed_requests=$(grep "Failed requests:" "$file" | awk '{print $3}')
+non_2xx_responses=$(grep "Non-2xx responses:" "$file" | awk '{print $3}')
+time_per_request_all=$(grep "Time per request:" "$file" | awk 'NR==2{print $4}')
+requests_per_second=$(grep "Requests per second:" "$file" | awk '{print $4}')
+
+
+if [ "$failed_requests" -gt 0 ]; then
+  echo "Lettings logs: Performance test failed - $failed_requests failed requests"
+  exit 1
+fi
+
+if [ "$non_2xx_responses" -ne 0 ] && [ -n "$non_2xx_responses" ]; then
+  echo "Lettings logs: Performance test failed: There were $non_2xx_responses non-2xx responses."
+  exit 1
+fi
+
+if (( $(echo "$time_per_request_all > 250" | bc -l) )); then
+  echo "Lettings logs: Performance test failed - Time per request across all concurrent requests is more than 250 ms: $time_per_request_all ms"
+  exit 1
+fi
+
+if (( $(echo "$requests_per_second < 5" | bc -l) )); then
+  echo "Lettings logs: Performance test failed - Requests per second is less than 5: $requests_per_second"
+  exit 1
+fi
+
+echo "Lettings logs page test passed: No failed requests and no non-2xx responses."
+
+
+# Sales logs page
+echo "Running sales logs page performance test..."
+ab -n 50 -c 50 -l -C "$COOKIES" 'https://staging.submit-social-housing-data.levellingup.gov.uk/sales-logs?years[]=2024&status[]=completed' > performance_sales_test_results.txt
+file="performance_sales_test_results.txt"
+
+failed_requests=$(grep "Failed requests:" "$file" | awk '{print $3}')
+non_2xx_responses=$(grep "Non-2xx responses:" "$file" | awk '{print $3}')
+time_per_request_all=$(grep "Time per request:" "$file" | awk 'NR==2{print $4}')
+requests_per_second=$(grep "Requests per second:" "$file" | awk '{print $4}')
+
+
+if [ "$failed_requests" -gt 0 ]; then
+  echo "Sales logs: Performance test failed - $failed_requests failed requests"
+  exit 1
+fi
+
+if [ "$non_2xx_responses" -ne 0 ] && [ -n "$non_2xx_responses" ]; then
+  echo "Sales logs: Performance test failed: There were $non_2xx_responses non-2xx responses."
+  exit 1
+fi
+
+if (( $(echo "$time_per_request_all > 250" | bc -l) )); then
+  echo "Sales logs: Performance test failed - Time per request across all concurrent requests is more than 250 ms: $time_per_request_all ms"
+  exit 1
+fi
+
+if (( $(echo "$requests_per_second < 5" | bc -l) )); then
+  echo "Sales logs: Performance test failed - Requests per second is less than 5: $requests_per_second"
+  exit 1
+fi
+
+echo "Sales logs page test passed: No failed requests and no non-2xx responses."
+
+
+# Post data to a log test
+page_content=$(curl -b login_cookies.txt -s 'https://staging.submit-social-housing-data.levellingup.gov.uk/lettings-logs?years[]=2024&status[]=completed')
+completed_log_link=$(echo "$page_content" | sed -n 's/.*<a class="govuk-link" href="\([^"]*lettings-logs[^"]*\)".*/\1/p' | head -n 1)
+echo "testing post to $completed_log_link"
+
+TOKEN=$(curl -L -b login_cookies.txt -c login_cookies.txt https://staging.submit-social-housing-data.levellingup.gov.uk$completed_log_link/tenant-code | grep '<meta name="csrf-token"' | sed -n 's/.*content="\([^"]*\)".*/\1/p')
+
+COOKIES=$(awk '/_data_collector_session/ { print $6, $7 }' login_cookies.txt | tr ' ' '=')
+
+echo "lettings_log[tenancycode]=performance_test_tenancy_code&lettings_log[page]=tenant_code&authenticity_token=$TOKEN" > post_data.txt
+
+ab -n 50 -c 50 -l -T application/x-www-form-urlencoded \
+-H "X-CSRF-Token: $TOKEN" \
+-C "$COOKIES" \
+-p post_data.txt \
+"https://staging.submit-social-housing-data.levellingup.gov.uk$completed_log_link/tenant-code" > performance_post_test_results.txt
+
+file="performance_post_test_results.txt"
+failed_requests=$(grep "Failed requests:" "$file" | awk '{print $3}')
+time_per_request_all=$(grep "Time per request:" "$file" | awk 'NR==2{print $4}')
+requests_per_second=$(grep "Requests per second:" "$file" | awk '{print $4}')
+
+
+if [ "$failed_requests" -gt 0 ]; then
+  echo "Update logs: Performance test failed - $failed_requests failed requests"
+  exit 1
+fi
+
+if (( $(echo "$time_per_request_all > 500" | bc -l) )); then
+  echo "Update logs: Performance test failed - Time per request across all concurrent requests is more than 500 ms: $time_per_request_all ms"
+  exit 1
+fi
+
+if (( $(echo "$requests_per_second < 3" | bc -l) )); then
+  echo "Update logs: Performance test failed - Requests per second is less than 3: $requests_per_second"
+  exit 1
+fi
+
+echo "Update logs test passed: No failed requests and request times as expected."
+
+echo "All tests passed"
+exit 0

From 5a93df785a33c0486a1de0e18ff939cb0ffec9d3 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Mon, 23 Sep 2024 12:05:52 +0100
Subject: [PATCH 2/7] CLDC-3620 Count duplicate schemes and locations (#2645)

* Add duplicate sets scope to schemes

* Add rake task to write duplicate scheme sets

* Add duplicate sets scope to locations

* Add rake task to write duplicate locations

* lint

* Update location duplicate count

* Add scheme_id back to DUPLICATE_LOCATION_ATTRIBUTES
---
 app/models/location.rb                  |  24 +++++
 app/models/scheme.rb                    |  18 ++++
 lib/tasks/count_duplicates.rake         |  63 ++++++++++++
 spec/factories/scheme.rb                |   9 ++
 spec/lib/tasks/count_duplicates_spec.rb | 111 ++++++++++++++++++++
 spec/models/location_spec.rb            |  70 +++++++++++++
 spec/models/scheme_spec.rb              | 129 ++++++++++++++++++++++++
 7 files changed, 424 insertions(+)
 create mode 100644 lib/tasks/count_duplicates.rake
 create mode 100644 spec/lib/tasks/count_duplicates_spec.rb

diff --git a/app/models/location.rb b/app/models/location.rb
index 19cf5e211..03af24a94 100644
--- a/app/models/location.rb
+++ b/app/models/location.rb
@@ -121,6 +121,30 @@ class Location < ApplicationRecord
 
   scope :visible, -> { where(discarded_at: nil) }
 
+  scope :duplicate_sets, lambda {
+    scope = visible
+    .group(*DUPLICATE_LOCATION_ATTRIBUTES)
+    .where.not(scheme_id: nil)
+    .where.not(postcode: nil)
+    .where.not(mobility_type: nil)
+    .having(
+      "COUNT(*) > 1",
+    )
+    scope.pluck("ARRAY_AGG(id)")
+  }
+
+  scope :duplicate_sets_within_given_schemes, lambda {
+    scope = visible
+    .group(*DUPLICATE_LOCATION_ATTRIBUTES - %w[scheme_id])
+    .where.not(postcode: nil)
+    .where.not(mobility_type: nil)
+    .having(
+      "COUNT(*) > 1",
+    )
+    scope.pluck("ARRAY_AGG(id)")
+  }
+
+  DUPLICATE_LOCATION_ATTRIBUTES = %w[scheme_id postcode mobility_type].freeze
   LOCAL_AUTHORITIES = LocalAuthority.all.map { |la| [la.name, la.code] }.to_h
 
   enum local_authorities: LOCAL_AUTHORITIES
diff --git a/app/models/scheme.rb b/app/models/scheme.rb
index 6d3524723..2c73acc06 100644
--- a/app/models/scheme.rb
+++ b/app/models/scheme.rb
@@ -103,6 +103,22 @@ class Scheme < ApplicationRecord
 
   scope :visible, -> { where(discarded_at: nil) }
 
+  scope :duplicate_sets, lambda {
+    scope = visible
+    .group(*DUPLICATE_SCHEME_ATTRIBUTES)
+    .where.not(scheme_type: nil)
+    .where.not(registered_under_care_act: nil)
+    .where.not(primary_client_group: nil)
+    .where.not(has_other_client_group: nil)
+    .where.not(secondary_client_group: nil).or(where(has_other_client_group: 0))
+    .where.not(support_type: nil)
+    .where.not(intended_stay: nil)
+    .having(
+      "COUNT(*) > 1",
+    )
+    scope.pluck("ARRAY_AGG(id)")
+  }
+
   validate :validate_confirmed
   validate :validate_owning_organisation
 
@@ -192,6 +208,8 @@ class Scheme < ApplicationRecord
     "Missing": "X",
   }.freeze
 
+  DUPLICATE_SCHEME_ATTRIBUTES = %w[scheme_type registered_under_care_act primary_client_group secondary_client_group has_other_client_group support_type intended_stay].freeze
+
   enum arrangement_type: ARRANGEMENT_TYPE, _suffix: true
 
   def self.find_by_id_on_multiple_fields(scheme_id, location_id)
diff --git a/lib/tasks/count_duplicates.rake b/lib/tasks/count_duplicates.rake
new file mode 100644
index 000000000..e65688b4d
--- /dev/null
+++ b/lib/tasks/count_duplicates.rake
@@ -0,0 +1,63 @@
+namespace :count_duplicates do
+  desc "Count the number of duplicate schemes per organisation"
+  task scheme_duplicates_per_org: :environment do
+    duplicates_csv = CSV.generate(headers: true) do |csv|
+      csv << ["Organisation id", "Number of duplicate sets", "Total duplicate schemes"]
+
+      Organisation.visible.each do |organisation|
+        if organisation.owned_schemes.duplicate_sets.count.positive?
+          csv << [organisation.id, organisation.owned_schemes.duplicate_sets.count, organisation.owned_schemes.duplicate_sets.sum(&:size)]
+        end
+      end
+    end
+
+    filename = "scheme-duplicates-#{Time.zone.now}.csv"
+    storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["BULK_UPLOAD_BUCKET"])
+    storage_service.write_file(filename, "#{duplicates_csv}")
+
+    url = storage_service.get_presigned_url(filename, 72.hours.to_i)
+    Rails.logger.info("Download URL: #{url}")
+  end
+
+  desc "Count the number of duplicate locations per organisation"
+  task location_duplicates_per_org: :environment do
+    duplicates_csv = CSV.generate(headers: true) do |csv|
+      csv << ["Organisation id", "Duplicate sets within individual schemes", "Duplicate locations within individual schemes", "All duplicate sets", "All duplicates"]
+
+      Organisation.visible.each do |organisation|
+        duplicate_sets_within_individual_schemes = []
+
+        organisation.owned_schemes.each do |scheme|
+          duplicate_sets_within_individual_schemes += scheme.locations.duplicate_sets
+        end
+        duplicate_locations_within_individual_schemes = duplicate_sets_within_individual_schemes.flatten
+
+        duplicate_sets_within_duplicate_schemes = []
+        if organisation.owned_schemes.duplicate_sets.count.positive?
+          organisation.owned_schemes.duplicate_sets.each do |duplicate_set|
+            duplicate_sets_within_duplicate_schemes += Location.where(scheme_id: duplicate_set).duplicate_sets_within_given_schemes
+          end
+          duplicate_locations_within_duplicate_schemes_ids = duplicate_sets_within_duplicate_schemes.flatten
+
+          duplicate_sets_within_individual_schemes_without_intersecting_sets = duplicate_sets_within_individual_schemes.reject { |set| set.any? { |id| duplicate_sets_within_duplicate_schemes.any? { |duplicate_set| duplicate_set.include?(id) } } }
+          all_duplicate_sets_count = (duplicate_sets_within_individual_schemes_without_intersecting_sets + duplicate_sets_within_duplicate_schemes).count
+          all_duplicate_locations_count = (duplicate_locations_within_duplicate_schemes_ids + duplicate_locations_within_individual_schemes).uniq.count
+        else
+          all_duplicate_sets_count = duplicate_sets_within_individual_schemes.count
+          all_duplicate_locations_count = duplicate_locations_within_individual_schemes.count
+        end
+
+        if all_duplicate_locations_count.positive?
+          csv << [organisation.id, duplicate_sets_within_individual_schemes.count, duplicate_locations_within_individual_schemes.count, all_duplicate_sets_count, all_duplicate_locations_count]
+        end
+      end
+    end
+
+    filename = "location-duplicates-#{Time.zone.now}.csv"
+    storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["BULK_UPLOAD_BUCKET"])
+    storage_service.write_file(filename, "#{duplicates_csv}")
+
+    url = storage_service.get_presigned_url(filename, 72.hours.to_i)
+    Rails.logger.info("Download URL: #{url}")
+  end
+end
diff --git a/spec/factories/scheme.rb b/spec/factories/scheme.rb
index 7983bfa0a..5f4ad30bc 100644
--- a/spec/factories/scheme.rb
+++ b/spec/factories/scheme.rb
@@ -32,5 +32,14 @@ FactoryBot.define do
       confirmed { false }
       support_type { nil }
     end
+    trait :duplicate do
+      scheme_type { 4 }
+      registered_under_care_act { 1 }
+      primary_client_group { "O" }
+      secondary_client_group { "H" }
+      has_other_client_group { 1 }
+      support_type { 2 }
+      intended_stay { "M" }
+    end
   end
 end
diff --git a/spec/lib/tasks/count_duplicates_spec.rb b/spec/lib/tasks/count_duplicates_spec.rb
new file mode 100644
index 000000000..99da5b2fb
--- /dev/null
+++ b/spec/lib/tasks/count_duplicates_spec.rb
@@ -0,0 +1,111 @@
+require "rails_helper"
+require "rake"
+
+RSpec.describe "count_duplicates" do
+  before do
+    allow(Storage::S3Service).to receive(:new).and_return(storage_service)
+    allow(storage_service).to receive(:write_file)
+    allow(storage_service).to receive(:get_presigned_url).and_return(test_url)
+  end
+
+  describe "count_duplicates:scheme_duplicates_per_org", type: :task do
+    subject(:task) { Rake::Task["count_duplicates:scheme_duplicates_per_org"] }
+
+    let(:storage_service) { instance_double(Storage::S3Service) }
+    let(:test_url) { "test_url" }
+
+    before do
+      Rake.application.rake_require("tasks/count_duplicates")
+      Rake::Task.define_task(:environment)
+      task.reenable
+    end
+
+    context "when the rake task is run" do
+      context "and there are no duplicate schemes" do
+        before do
+          create(:organisation)
+        end
+
+        it "creates a csv with headers only" do
+          expect(storage_service).to receive(:write_file).with(/scheme-duplicates-.*\.csv/, "\uFEFFOrganisation id,Number of duplicate sets,Total duplicate schemes\n")
+          expect(Rails.logger).to receive(:info).with("Download URL: #{test_url}")
+          task.invoke
+        end
+      end
+
+      context "and there are duplicate schemes" do
+        let(:organisation) { create(:organisation) }
+        let(:organisation2) { create(:organisation) }
+
+        before do
+          create_list(:scheme, 2, :duplicate, owning_organisation: organisation)
+          create_list(:scheme, 3, :duplicate, primary_client_group: "I", owning_organisation: organisation)
+          create_list(:scheme, 5, :duplicate, owning_organisation: organisation2)
+        end
+
+        it "creates a csv with correct duplicate numbers" do
+          expect(storage_service).to receive(:write_file).with(/scheme-duplicates-.*\.csv/, "\uFEFFOrganisation id,Number of duplicate sets,Total duplicate schemes\n#{organisation.id},2,5\n#{organisation2.id},1,5\n")
+          expect(Rails.logger).to receive(:info).with("Download URL: #{test_url}")
+          task.invoke
+        end
+      end
+    end
+  end
+
+  describe "count_duplicates:location_duplicates_per_org", type: :task do
+    subject(:task) { Rake::Task["count_duplicates:location_duplicates_per_org"] }
+
+    let(:storage_service) { instance_double(Storage::S3Service) }
+    let(:test_url) { "test_url" }
+
+    before do
+      Rake.application.rake_require("tasks/count_duplicates")
+      Rake::Task.define_task(:environment)
+      task.reenable
+    end
+
+    context "when the rake task is run" do
+      context "and there are no duplicate locations" do
+        before do
+          create(:organisation)
+        end
+
+        it "creates a csv with headers only" do
+          expect(storage_service).to receive(:write_file).with(/location-duplicates-.*\.csv/, "\uFEFFOrganisation id,Duplicate sets within individual schemes,Duplicate locations within individual schemes,All duplicate sets,All duplicates\n")
+          expect(Rails.logger).to receive(:info).with("Download URL: #{test_url}")
+          task.invoke
+        end
+      end
+
+      context "and there are duplicate locations" do
+        let(:organisation) { create(:organisation) }
+        let(:scheme_a) { create(:scheme, :duplicate, owning_organisation: organisation) }
+        let(:scheme_b) { create(:scheme, :duplicate, owning_organisation: organisation) }
+        let(:scheme_c) { create(:scheme, owning_organisation: organisation) }
+        let(:organisation2) { create(:organisation) }
+        let(:scheme2) { create(:scheme, owning_organisation: organisation2) }
+        let(:scheme3) { create(:scheme, owning_organisation: organisation2) }
+
+        before do
+          create_list(:location, 2, postcode: "A1 1AB", mobility_type: "M", scheme: scheme_a) # Location A
+          create_list(:location, 1, postcode: "A1 1AB", mobility_type: "A", scheme: scheme_a) # Location B
+
+          create_list(:location, 1, postcode: "A1 1AB", mobility_type: "M", scheme: scheme_b) # Location A
+          create_list(:location, 1, postcode: "A1 1AB", mobility_type: "A", scheme: scheme_b) # Location B
+          create_list(:location, 2, postcode: "A1 1AB", mobility_type: "N", scheme: scheme_b) # Location C
+
+          create_list(:location, 2, postcode: "A1 1AB", mobility_type: "A", scheme: scheme_c) # Location B
+
+          create_list(:location, 5, postcode: "A1 1AB", mobility_type: "M", scheme: scheme2)
+          create_list(:location, 2, postcode: "A1 1AB", mobility_type: "M", scheme: scheme3)
+        end
+
+        it "creates a csv with correct duplicate numbers" do
+          expect(storage_service).to receive(:write_file).with(/location-duplicates-.*\.csv/, "\uFEFFOrganisation id,Duplicate sets within individual schemes,Duplicate locations within individual schemes,All duplicate sets,All duplicates\n#{organisation.id},3,6,4,9\n#{organisation2.id},2,7,2,7\n")
+          expect(Rails.logger).to receive(:info).with("Download URL: #{test_url}")
+          task.invoke
+        end
+      end
+    end
+  end
+end
diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb
index 4856c5662..79265d361 100644
--- a/spec/models/location_spec.rb
+++ b/spec/models/location_spec.rb
@@ -831,6 +831,76 @@ RSpec.describe Location, type: :model do
         expect(described_class.active.count).to eq(2)
       end
     end
+
+    context "when getting list of duplicate locations" do
+      let!(:scheme) { create(:scheme) }
+      let!(:location) { create(:location, postcode: "AB1 2CD", mobility_type: "M", scheme:) }
+      let!(:duplicate_location) { create(:location, postcode: "AB1 2CD", mobility_type: "M", scheme:) }
+      let(:duplicate_sets) { described_class.duplicate_sets }
+
+      it "returns a list of duplicates for the same scheme" do
+        expect(duplicate_sets.count).to eq(1)
+        expect(duplicate_sets.first).to contain_exactly(location.id, duplicate_location.id)
+      end
+
+      context "when there is a deleted duplicate location" do
+        before do
+          create(:location, postcode: "AB1 2CD", mobility_type: "M", discarded_at: Time.zone.now, scheme:)
+        end
+
+        it "does not return the deleted location as a duplicate" do
+          expect(duplicate_sets.count).to eq(1)
+          expect(duplicate_sets.first).to contain_exactly(location.id, duplicate_location.id)
+        end
+      end
+
+      context "when there is a location with a different postcode" do
+        before do
+          create(:location, postcode: "A1 1AB", mobility_type: "M", scheme:)
+        end
+
+        it "does not return a location with a different postcode as a duplicate" do
+          expect(duplicate_sets.count).to eq(1)
+          expect(duplicate_sets.first).to contain_exactly(location.id, duplicate_location.id)
+        end
+      end
+
+      context "when there is a location with a different mobility_type" do
+        before do
+          create(:location, postcode: "AB1 2CD", mobility_type: "A", scheme:)
+        end
+
+        it "does not return a location with a different mobility_type as a duplicate" do
+          expect(duplicate_sets.count).to eq(1)
+          expect(duplicate_sets.first).to contain_exactly(location.id, duplicate_location.id)
+        end
+      end
+
+      context "when there is a location with a different scheme" do
+        before do
+          create(:location, postcode: "AB1 2CD", mobility_type: "M")
+        end
+
+        it "does not return a location with a different scheme as a duplicate" do
+          expect(duplicate_sets.count).to eq(1)
+          expect(duplicate_sets.first).to contain_exactly(location.id, duplicate_location.id)
+        end
+      end
+
+      context "when there is a location with nil values for duplicate check fields" do
+        before do
+          [location, duplicate_location].each do |l|
+            l.postcode = nil
+            l.mobility_type = nil
+            l.save!(validate: false)
+          end
+        end
+
+        it "does not return a location with nil values as a duplicate" do
+          expect(duplicate_sets).to be_empty
+        end
+      end
+    end
   end
 
   describe "status" do
diff --git a/spec/models/scheme_spec.rb b/spec/models/scheme_spec.rb
index 9b3db15a4..5ca529d3e 100644
--- a/spec/models/scheme_spec.rb
+++ b/spec/models/scheme_spec.rb
@@ -208,6 +208,135 @@ RSpec.describe Scheme, type: :model do
           end
         end
       end
+
+      context "when getting list of duplicate schemes" do
+        let(:organisation) { create(:organisation) }
+        let!(:scheme) { create(:scheme, :duplicate, owning_organisation: organisation) }
+        let!(:duplicate_scheme) { create(:scheme, :duplicate, owning_organisation: organisation) }
+        let(:duplicate_sets) { described_class.duplicate_sets }
+
+        it "returns a list of duplicates in the same organisation" do
+          expect(duplicate_sets.count).to eq(1)
+          expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id)
+        end
+
+        context "when there is a deleted duplicate scheme" do
+          before do
+            create(:scheme, :duplicate, discarded_at: Time.zone.now)
+          end
+
+          it "does not return the deleted scheme as a duplicate" do
+            expect(duplicate_sets.count).to eq(1)
+            expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id)
+          end
+        end
+
+        context "when there is a scheme with a different scheme_type" do
+          before do
+            create(:scheme, :duplicate, scheme_type: 7)
+          end
+
+          it "does not return a scheme with a different scheme_type as a duplicate" do
+            expect(duplicate_sets.count).to eq(1)
+            expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id)
+          end
+        end
+
+        context "when there is a scheme with a different registered_under_care_act" do
+          before do
+            create(:scheme, :duplicate, registered_under_care_act: 2)
+          end
+
+          it "does not return a scheme with a different registered_under_care_act as a duplicate" do
+            expect(duplicate_sets.count).to eq(1)
+            expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id)
+          end
+        end
+
+        context "when there is a scheme with a different primary_client_group" do
+          before do
+            create(:scheme, :duplicate, primary_client_group: "H")
+          end
+
+          it "does not return a scheme with a different primary_client_group as a duplicate" do
+            expect(duplicate_sets.count).to eq(1)
+            expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id)
+          end
+        end
+
+        context "when there is a scheme with a different secondary_client_group" do
+          before do
+            create(:scheme, :duplicate, secondary_client_group: "O")
+          end
+
+          it "does not return a scheme with a different secondary_client_group as a duplicate" do
+            expect(duplicate_sets.count).to eq(1)
+            expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id)
+          end
+        end
+
+        context "when there is a scheme with a different has_other_client_group" do
+          before do
+            create(:scheme, :duplicate, has_other_client_group: 0)
+          end
+
+          it "does not return a scheme with a different has_other_client_group as a duplicate" do
+            expect(duplicate_sets.count).to eq(1)
+            expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id)
+          end
+        end
+
+        context "when there is a scheme with a different support_type" do
+          before do
+            create(:scheme, :duplicate, support_type: 4)
+          end
+
+          it "does not return a scheme with a different support_type as a duplicate" do
+            expect(duplicate_sets.count).to eq(1)
+            expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id)
+          end
+        end
+
+        context "when there is a scheme with a different intended_stay" do
+          before do
+            create(:scheme, :duplicate, intended_stay: "P")
+          end
+
+          it "does not return a scheme with a different intended_stay as a duplicate" do
+            expect(duplicate_sets.count).to eq(1)
+            expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id)
+          end
+        end
+
+        context "when there is a scheme with nil values for duplicate check fields" do
+          before do
+            [scheme, duplicate_scheme].each do |s|
+              s.scheme_type = nil
+              s.registered_under_care_act = nil
+              s.primary_client_group = nil
+              s.secondary_client_group = nil
+              s.has_other_client_group = nil
+              s.support_type = nil
+              s.intended_stay = nil
+              s.save!(validate: false)
+            end
+          end
+
+          it "does not return a scheme with nil values as a duplicate" do
+            expect(duplicate_sets).to be_empty
+          end
+        end
+
+        context "when there are duplicate schemes without secondary client group" do
+          let!(:scheme) { create(:scheme, :duplicate, owning_organisation: organisation, secondary_client_group: nil, has_other_client_group: 0) }
+          let!(:duplicate_scheme) { create(:scheme, :duplicate, owning_organisation: organisation, secondary_client_group: nil, has_other_client_group: 0) }
+
+          it "does not returns the duplicates" do
+            expect(duplicate_sets.count).to eq(1)
+            expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id)
+          end
+        end
+      end
     end
   end
 

From b47738c4bec6f6b681ef36b9cbffd09d40ae4794 Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Mon, 23 Sep 2024 15:31:41 +0100
Subject: [PATCH 3/7] CLDC-3632 Bulk upload hide empty critical errors table 
 (#2627)

* Make critical errors table conditionally shown

* Add the same margin to buttons, just like in lettings logs and sales logs page
---
 .../bulk_upload_error_row_component.html.erb  | 37 ++++++++++---------
 app/views/schemes/index.html.erb              |  4 +-
 app/views/users/index.html.erb                |  4 +-
 3 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/app/components/bulk_upload_error_row_component.html.erb b/app/components/bulk_upload_error_row_component.html.erb
index 9447c9d37..65e38613f 100644
--- a/app/components/bulk_upload_error_row_component.html.erb
+++ b/app/components/bulk_upload_error_row_component.html.erb
@@ -9,24 +9,27 @@
 
   <div class="govuk-summary-card__content">
     <% potential_errors, critical_errors = bulk_upload_errors.partition { |error| error.category == "soft_validation" } %>
-    <h2 class="govuk-heading-m">Critical errors</h2>
-    <p class="govuk-body">These errors must be fixed to complete your logs.</p>
-    <%= govuk_table do |table| %>
-      <%= table.with_head do |head| %>
-        <% head.with_row do |row| %>
-          <% row.with_cell(header: true, text: "Cell") %>
-          <% row.with_cell(header: true, text: "Question") %>
-          <% row.with_cell(header: true, text: "Error") %>
-          <% row.with_cell(header: true, text: "Specification") %>
-        <% end %>
 
-        <%= table.with_body do |body| %>
-          <% critical_errors.each do |error| %>
-            <% body.with_row do |row| %>
-                <% row.with_cell(text: error.cell) %>
-                <% row.with_cell(text: question_for_field(error.field), html_attributes: { class: "govuk-!-width-one-half" }) %>
-                <% row.with_cell(text: error.error.html_safe, html_attributes: { class: "govuk-!-font-weight-bold govuk-!-width-one-half" }) %>
-                <% row.with_cell(text: error.field.humanize) %>
+    <% if critical_errors.any? %>
+      <h2 class="govuk-heading-m">Critical errors</h2>
+      <p class="govuk-body">These errors must be fixed to complete your logs.</p>
+      <%= govuk_table do |table| %>
+        <%= table.with_head do |head| %>
+          <% head.with_row do |row| %>
+            <% row.with_cell(header: true, text: "Cell") %>
+            <% row.with_cell(header: true, text: "Question") %>
+            <% row.with_cell(header: true, text: "Error") %>
+            <% row.with_cell(header: true, text: "Specification") %>
+          <% end %>
+
+          <%= table.with_body do |body| %>
+            <% critical_errors.each do |error| %>
+              <% body.with_row do |row| %>
+                  <% row.with_cell(text: error.cell) %>
+                  <% row.with_cell(text: question_for_field(error.field), html_attributes: { class: "govuk-!-width-one-half" }) %>
+                  <% row.with_cell(text: error.error.html_safe, html_attributes: { class: "govuk-!-font-weight-bold govuk-!-width-one-half" }) %>
+                  <% row.with_cell(text: error.field.humanize) %>
+              <% end %>
             <% end %>
           <% end %>
         <% end %>
diff --git a/app/views/schemes/index.html.erb b/app/views/schemes/index.html.erb
index d0ae44072..edaed6212 100644
--- a/app/views/schemes/index.html.erb
+++ b/app/views/schemes/index.html.erb
@@ -6,7 +6,9 @@
 <%= render partial: "organisations/headings", locals: current_user.support? ? { main: "Supported housing schemes", sub: nil } : { main: "Supported housing schemes", sub: current_user.organisation.name } %>
 
 <% if SchemePolicy.new(current_user, nil).create? %>
-  <%= govuk_button_link_to "Create a new supported housing scheme", new_scheme_path, html: { method: :post } %>
+  <div class="govuk-button-group govuk-!-margin-bottom-6">
+    <%= govuk_button_link_to "Create a new supported housing scheme", new_scheme_path, html: { method: :post } %>
+  </div>
 <% end %>
 <div class="app-filter-layout" data-controller="filter-layout">
   <%= render partial: "schemes/scheme_filters" %>
diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb
index 7da3deda1..4e5053563 100644
--- a/app/views/users/index.html.erb
+++ b/app/views/users/index.html.erb
@@ -6,7 +6,9 @@
 <%= render partial: "organisations/headings", locals: current_user.support? ? { main: "Users", sub: nil } : { main: "Users", sub: current_user.organisation.name } %>
 
 <% if current_user.data_coordinator? || current_user.support? %>
-  <%= govuk_button_link_to "Invite user", new_user_path, html: { method: :get } %>
+  <div class="govuk-button-group govuk-!-margin-bottom-6">
+    <%= govuk_button_link_to "Invite user", new_user_path, html: { method: :get } %>
+  </div>
 <% end %>
 <div class="app-filter-layout" data-controller="filter-layout">
   <%= render partial: "users/user_filters" %>

From e604d358b35502ea7be08ad38e2baeadbaf7ae11 Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Mon, 23 Sep 2024 15:45:35 +0100
Subject: [PATCH 4/7] CLDC-3627 Problem displaying apostrophes in browser tab
 title (#2643)

---
 app/helpers/application_helper.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index bb119b29e..5a9203da8 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -2,6 +2,7 @@ module ApplicationHelper
   include Pagy::Frontend
 
   def browser_title(title, pagy, *resources)
+    title = sanitize(title)&.gsub("&amp;", "&")
     if resources.any? { |r| r.present? && r.errors.present? }
       "Error: #{[title, t('service_name'), 'GOV.UK'].select(&:present?).join(' - ')}"
     else

From 806424992579127dffdee614216ed79d6b9c3af2 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Mon, 23 Sep 2024 16:31:23 +0100
Subject: [PATCH 5/7] Add checked field to log_validations (#2657)

---
 db/migrate/20240923145326_add_validation_checked_field.rb | 5 +++++
 db/schema.rb                                              | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)
 create mode 100644 db/migrate/20240923145326_add_validation_checked_field.rb

diff --git a/db/migrate/20240923145326_add_validation_checked_field.rb b/db/migrate/20240923145326_add_validation_checked_field.rb
new file mode 100644
index 000000000..899992974
--- /dev/null
+++ b/db/migrate/20240923145326_add_validation_checked_field.rb
@@ -0,0 +1,5 @@
+class AddValidationCheckedField < ActiveRecord::Migration[7.0]
+  def change
+    add_column :log_validations, :checked, :boolean
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 80463eaad..cd7708957 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2024_09_18_112702) do
+ActiveRecord::Schema[7.0].define(version: 2024_09_23_145326) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -410,6 +410,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_09_18_112702) do
     t.string "other_validated_models"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.boolean "checked"
   end
 
   create_table "logs_exports", force: :cascade do |t|

From d0ce0b87f2390079444a08dd09bd8aa6f81bc4ab Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Tue, 24 Sep 2024 11:21:18 +0100
Subject: [PATCH 6/7] CLDC 3623 Support user bulk uploading (#2629)

---
 .../create_log_actions_component.html.erb     |  2 +-
 .../bulk_upload_lettings_logs_controller.rb   |  6 +--
 .../bulk_upload_sales_logs_controller.rb      |  6 +--
 .../bulk_upload_lettings/checking_file.rb     |  7 ++-
 .../forms/bulk_upload_lettings/guidance.rb    |  3 +-
 .../forms/bulk_upload_lettings/needstype.rb   |  5 +-
 .../bulk_upload_lettings/prepare_your_file.rb |  8 +--
 .../bulk_upload_lettings/upload_your_file.rb  |  7 +--
 app/models/forms/bulk_upload_lettings/year.rb |  9 +++-
 .../forms/bulk_upload_sales/checking_file.rb  |  7 ++-
 .../forms/bulk_upload_sales/guidance.rb       |  3 +-
 .../bulk_upload_sales/prepare_your_file.rb    |  7 ++-
 .../bulk_upload_sales/upload_your_file.rb     |  6 ++-
 app/models/forms/bulk_upload_sales/year.rb    |  9 +++-
 .../lettings/year2024/row_parser.rb           | 28 +++++++---
 .../bulk_upload/sales/year2024/row_parser.rb  | 28 +++++++---
 .../forms/checking_file.html.erb              |  1 +
 .../forms/needstype.erb                       |  1 +
 .../forms/prepare_your_file_2024.html.erb     |  1 +
 .../forms/upload_your_file.html.erb           |  1 +
 .../forms/year.html.erb                       |  1 +
 .../show.html.erb                             |  2 +-
 .../summary.html.erb                          |  2 +-
 .../forms/checking_file.html.erb              |  1 +
 .../forms/prepare_your_file_2024.html.erb     |  1 +
 .../forms/upload_your_file.html.erb           |  1 +
 .../forms/year.html.erb                       |  1 +
 .../bulk_upload_sales_results/show.html.erb   |  2 +-
 .../summary.html.erb                          |  2 +-
 .../logs/_create_for_org_actions.html.erb     | 10 ++--
 ...332_add_organisation_id_to_bulk_uploads.rb |  5 ++
 db/schema.rb                                  |  1 +
 spec/factories/bulk_upload.rb                 |  1 +
 spec/features/organisation_spec.rb            |  8 +++
 .../lettings/year2024/row_parser_spec.rb      | 54 +++++++++++++++++++
 .../sales/year2024/row_parser_spec.rb         | 54 +++++++++++++++++++
 36 files changed, 244 insertions(+), 47 deletions(-)
 create mode 100644 db/migrate/20240905092332_add_organisation_id_to_bulk_uploads.rb

diff --git a/app/components/create_log_actions_component.html.erb b/app/components/create_log_actions_component.html.erb
index 1b6bd8aca..5a90587ed 100644
--- a/app/components/create_log_actions_component.html.erb
+++ b/app/components/create_log_actions_component.html.erb
@@ -3,7 +3,7 @@
     <% if create_button_href.present? %>
       <%= govuk_button_to create_button_copy, create_button_href, class: "govuk-!-margin-right-6" %>
     <% end %>
-    <% if upload_button_href.present? %>
+    <% if upload_button_href.present? && !user.support? %>
       <%= govuk_button_link_to upload_button_copy, upload_button_href, secondary: true %>
     <% end %>
   </div>
diff --git a/app/controllers/bulk_upload_lettings_logs_controller.rb b/app/controllers/bulk_upload_lettings_logs_controller.rb
index 465bbc5f6..a8ad14f9e 100644
--- a/app/controllers/bulk_upload_lettings_logs_controller.rb
+++ b/app/controllers/bulk_upload_lettings_logs_controller.rb
@@ -4,9 +4,9 @@ class BulkUploadLettingsLogsController < ApplicationController
 
   def start
     if have_choice_of_year?
-      redirect_to bulk_upload_lettings_log_path(id: "year")
+      redirect_to bulk_upload_lettings_log_path(id: "year", form: { organisation_id: params[:organisation_id] }.compact)
     else
-      redirect_to bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year: current_year })
+      redirect_to bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year: current_year, organisation_id: params[:organisation_id] }.compact)
     end
   end
 
@@ -60,6 +60,6 @@ private
   end
 
   def form_params
-    params.fetch(:form, {}).permit(:year, :needstype, :file)
+    params.fetch(:form, {}).permit(:year, :needstype, :file, :organisation_id)
   end
 end
diff --git a/app/controllers/bulk_upload_sales_logs_controller.rb b/app/controllers/bulk_upload_sales_logs_controller.rb
index 56bd1d4de..2fd312d10 100644
--- a/app/controllers/bulk_upload_sales_logs_controller.rb
+++ b/app/controllers/bulk_upload_sales_logs_controller.rb
@@ -4,9 +4,9 @@ class BulkUploadSalesLogsController < ApplicationController
 
   def start
     if have_choice_of_year?
-      redirect_to bulk_upload_sales_log_path(id: "year")
+      redirect_to bulk_upload_sales_log_path(id: "year", form: { organisation_id: params[:organisation_id] }.compact)
     else
-      redirect_to bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: current_year })
+      redirect_to bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: current_year, organisation_id: params[:organisation_id] }.compact)
     end
   end
 
@@ -58,6 +58,6 @@ private
   end
 
   def form_params
-    params.fetch(:form, {}).permit(:year, :file)
+    params.fetch(:form, {}).permit(:year, :file, :organisation_id)
   end
 end
diff --git a/app/models/forms/bulk_upload_lettings/checking_file.rb b/app/models/forms/bulk_upload_lettings/checking_file.rb
index 8cd9ee696..3b43cb269 100644
--- a/app/models/forms/bulk_upload_lettings/checking_file.rb
+++ b/app/models/forms/bulk_upload_lettings/checking_file.rb
@@ -6,13 +6,18 @@ module Forms
       include Rails.application.routes.url_helpers
 
       attribute :year, :integer
+      attribute :organisation_id, :integer
 
       def view_path
         "bulk_upload_lettings_logs/forms/checking_file"
       end
 
       def back_path
-        bulk_upload_lettings_log_path(id: "start")
+        if organisation_id.present?
+          lettings_logs_organisation_path(organisation_id)
+        else
+          bulk_upload_lettings_log_path(id: "start")
+        end
       end
 
       def year_combo
diff --git a/app/models/forms/bulk_upload_lettings/guidance.rb b/app/models/forms/bulk_upload_lettings/guidance.rb
index b6cf5bf74..24bc531f2 100644
--- a/app/models/forms/bulk_upload_lettings/guidance.rb
+++ b/app/models/forms/bulk_upload_lettings/guidance.rb
@@ -7,6 +7,7 @@ module Forms
 
       attribute :year, :integer
       attribute :referrer
+      attribute :organisation_id, :integer
 
       def view_path
         "bulk_upload_shared/guidance"
@@ -15,7 +16,7 @@ module Forms
       def back_path
         case referrer
         when "prepare-your-file"
-          bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year: })
+          bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year:, organisation_id: }.compact)
         when "home"
           root_path
         else
diff --git a/app/models/forms/bulk_upload_lettings/needstype.rb b/app/models/forms/bulk_upload_lettings/needstype.rb
index 0cadd647c..32ed0acf3 100644
--- a/app/models/forms/bulk_upload_lettings/needstype.rb
+++ b/app/models/forms/bulk_upload_lettings/needstype.rb
@@ -7,6 +7,7 @@ module Forms
 
       attribute :needstype, :integer
       attribute :year, :integer
+      attribute :organisation_id, :integer
 
       validates :needstype, presence: true
 
@@ -19,11 +20,11 @@ module Forms
       end
 
       def back_path
-        bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year:, needstype: })
+        bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year:, needstype:, organisation_id: }.compact)
       end
 
       def next_path
-        bulk_upload_lettings_log_path(id: "upload-your-file", form: { year:, needstype: })
+        bulk_upload_lettings_log_path(id: "upload-your-file", form: { year:, needstype:, organisation_id: }.compact)
       end
 
       def year_combo
diff --git a/app/models/forms/bulk_upload_lettings/prepare_your_file.rb b/app/models/forms/bulk_upload_lettings/prepare_your_file.rb
index b5069d908..159436ce1 100644
--- a/app/models/forms/bulk_upload_lettings/prepare_your_file.rb
+++ b/app/models/forms/bulk_upload_lettings/prepare_your_file.rb
@@ -7,6 +7,7 @@ module Forms
 
       attribute :year, :integer
       attribute :needstype, :integer
+      attribute :organisation_id, :integer
 
       def view_path
         case year
@@ -19,15 +20,16 @@ module Forms
 
       def back_path
         if have_choice_of_year?
-          Rails.application.routes.url_helpers.bulk_upload_lettings_log_path(id: "year", form: { year: })
+          Rails.application.routes.url_helpers.bulk_upload_lettings_log_path(id: "year", form: { year:, organisation_id: }.compact)
+        elsif organisation_id.present?
+          lettings_logs_organisation_path(organisation_id)
         else
           Rails.application.routes.url_helpers.lettings_logs_path
         end
       end
 
       def next_path
-        page_id = year == 2022 ? "needstype" : "upload-your-file"
-        bulk_upload_lettings_log_path(id: page_id, form: { year:, needstype: })
+        bulk_upload_lettings_log_path(id: "upload-your-file", form: { year:, needstype:, organisation_id: }.compact)
       end
 
       def legacy_template_path
diff --git a/app/models/forms/bulk_upload_lettings/upload_your_file.rb b/app/models/forms/bulk_upload_lettings/upload_your_file.rb
index 9ccec7622..954c6e6e6 100644
--- a/app/models/forms/bulk_upload_lettings/upload_your_file.rb
+++ b/app/models/forms/bulk_upload_lettings/upload_your_file.rb
@@ -11,6 +11,7 @@ module Forms
       attribute :needstype, :integer
       attribute :file
       attribute :current_user
+      attribute :organisation_id, :integer
 
       validates :file, presence: true
       validate :validate_file_is_csv
@@ -20,8 +21,7 @@ module Forms
       end
 
       def back_path
-        page_id = year == 2022 ? "needstype" : "prepare-your-file"
-        bulk_upload_lettings_log_path(id: page_id, form: { year:, needstype: })
+        bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year:, needstype:, organisation_id: }.compact)
       end
 
       def year_combo
@@ -29,7 +29,7 @@ module Forms
       end
 
       def next_path
-        bulk_upload_lettings_log_path(id: "checking-file", form: { year: })
+        bulk_upload_lettings_log_path(id: "checking-file", form: { year:, organisation_id: }.compact)
       end
 
       def save!
@@ -39,6 +39,7 @@ module Forms
           year:,
           needstype:,
           filename: file.original_filename,
+          organisation_id: (organisation_id if current_user.support?) || current_user.organisation_id,
         )
 
         storage_service.write_file(bulk_upload.identifier, File.read(file.path))
diff --git a/app/models/forms/bulk_upload_lettings/year.rb b/app/models/forms/bulk_upload_lettings/year.rb
index e199559fa..396b91a79 100644
--- a/app/models/forms/bulk_upload_lettings/year.rb
+++ b/app/models/forms/bulk_upload_lettings/year.rb
@@ -6,6 +6,7 @@ module Forms
       include Rails.application.routes.url_helpers
 
       attribute :year, :integer
+      attribute :organisation_id, :integer
 
       validates :year, presence: true
 
@@ -20,11 +21,15 @@ module Forms
       end
 
       def back_path
-        lettings_logs_path
+        if organisation_id.present?
+          lettings_logs_organisation_path(organisation_id)
+        else
+          lettings_logs_path
+        end
       end
 
       def next_path
-        bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year: })
+        bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year:, organisation_id: }.compact)
       end
 
       def save!
diff --git a/app/models/forms/bulk_upload_sales/checking_file.rb b/app/models/forms/bulk_upload_sales/checking_file.rb
index a37be3ccb..243d64070 100644
--- a/app/models/forms/bulk_upload_sales/checking_file.rb
+++ b/app/models/forms/bulk_upload_sales/checking_file.rb
@@ -6,13 +6,18 @@ module Forms
       include Rails.application.routes.url_helpers
 
       attribute :year, :integer
+      attribute :organisation_id, :integer
 
       def view_path
         "bulk_upload_sales_logs/forms/checking_file"
       end
 
       def back_path
-        bulk_upload_sales_log_path(id: "start")
+        if organisation_id.present?
+          sales_logs_organisation_path(organisation_id)
+        else
+          bulk_upload_sales_log_path(id: "start")
+        end
       end
 
       def year_combo
diff --git a/app/models/forms/bulk_upload_sales/guidance.rb b/app/models/forms/bulk_upload_sales/guidance.rb
index 28ca6c3b5..ef792a3e4 100644
--- a/app/models/forms/bulk_upload_sales/guidance.rb
+++ b/app/models/forms/bulk_upload_sales/guidance.rb
@@ -7,6 +7,7 @@ module Forms
 
       attribute :year, :integer
       attribute :referrer
+      attribute :organisation_id, :integer
 
       def view_path
         "bulk_upload_shared/guidance"
@@ -15,7 +16,7 @@ module Forms
       def back_path
         case referrer
         when "prepare-your-file"
-          bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: })
+          bulk_upload_sales_log_path(id: "prepare-your-file", form: { year:, organisation_id: }.compact)
         when "home"
           root_path
         else
diff --git a/app/models/forms/bulk_upload_sales/prepare_your_file.rb b/app/models/forms/bulk_upload_sales/prepare_your_file.rb
index 387f6239f..4bf0797a8 100644
--- a/app/models/forms/bulk_upload_sales/prepare_your_file.rb
+++ b/app/models/forms/bulk_upload_sales/prepare_your_file.rb
@@ -6,6 +6,7 @@ module Forms
       include Rails.application.routes.url_helpers
 
       attribute :year, :integer
+      attribute :organisation_id, :integer
 
       def view_path
         case year
@@ -18,14 +19,16 @@ module Forms
 
       def back_path
         if have_choice_of_year?
-          Rails.application.routes.url_helpers.bulk_upload_sales_log_path(id: "year", form: { year: })
+          Rails.application.routes.url_helpers.bulk_upload_sales_log_path(id: "year", form: { year: }.compact)
+        elsif organisation_id.present?
+          sales_logs_organisation_path(organisation_id)
         else
           Rails.application.routes.url_helpers.sales_logs_path
         end
       end
 
       def next_path
-        bulk_upload_sales_log_path(id: "upload-your-file", form: { year: })
+        bulk_upload_sales_log_path(id: "upload-your-file", form: { year:, organisation_id: }.compact)
       end
 
       def legacy_template_path
diff --git a/app/models/forms/bulk_upload_sales/upload_your_file.rb b/app/models/forms/bulk_upload_sales/upload_your_file.rb
index de650c831..5a0114caf 100644
--- a/app/models/forms/bulk_upload_sales/upload_your_file.rb
+++ b/app/models/forms/bulk_upload_sales/upload_your_file.rb
@@ -10,6 +10,7 @@ module Forms
       attribute :year, :integer
       attribute :file
       attribute :current_user
+      attribute :organisation_id, :integer
 
       validates :file, presence: true
       validate :validate_file_is_csv
@@ -19,7 +20,7 @@ module Forms
       end
 
       def back_path
-        bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: })
+        bulk_upload_sales_log_path(id: "prepare-your-file", form: { year:, organisation_id: }.compact)
       end
 
       def year_combo
@@ -27,7 +28,7 @@ module Forms
       end
 
       def next_path
-        bulk_upload_sales_log_path(id: "checking-file", form: { year: })
+        bulk_upload_sales_log_path(id: "checking-file", form: { year:, organisation_id: }.compact)
       end
 
       def save!
@@ -36,6 +37,7 @@ module Forms
           log_type: BulkUpload.log_types[:sales],
           year:,
           filename: file.original_filename,
+          organisation_id: (organisation_id if current_user.support?) || current_user.organisation_id,
         )
 
         storage_service.write_file(bulk_upload.identifier, File.read(file.path))
diff --git a/app/models/forms/bulk_upload_sales/year.rb b/app/models/forms/bulk_upload_sales/year.rb
index 85959c4e5..1de8dec04 100644
--- a/app/models/forms/bulk_upload_sales/year.rb
+++ b/app/models/forms/bulk_upload_sales/year.rb
@@ -6,6 +6,7 @@ module Forms
       include Rails.application.routes.url_helpers
 
       attribute :year, :integer
+      attribute :organisation_id, :integer
 
       validates :year, presence: true
 
@@ -20,11 +21,15 @@ module Forms
       end
 
       def back_path
-        sales_logs_path
+        if organisation_id.present?
+          sales_logs_organisation_path(organisation_id)
+        else
+          sales_logs_path
+        end
       end
 
       def next_path
-        bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: })
+        bulk_upload_sales_log_path(id: "prepare-your-file", form: { year:, organisation_id: }.compact)
       end
 
       def save!
diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb
index 8fc913055..dc0b2dca3 100644
--- a/app/services/bulk_upload/lettings/year2024/row_parser.rb
+++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb
@@ -432,6 +432,7 @@ class BulkUpload::Lettings::Year2024::RowParser
 
   validate :validate_assigned_to_exists, on: :after_log
   validate :validate_assigned_to_related, on: :after_log
+  validate :validate_assigned_to_when_support, on: :after_log
   validate :validate_all_charges_given, on: :after_log, if: proc { is_carehome.zero? }
 
   validate :validate_address_option_found, on: :after_log, unless: -> { supported_housing? }
@@ -582,6 +583,12 @@ private
     end
   end
 
+  def validate_assigned_to_when_support
+    if field_3.blank? && bulk_upload.user.support?
+      errors.add(:field_3, category: :setup, message: I18n.t("validations.not_answered", question: "what is the CORE username of the account this letting log should be assigned to?"))
+    end
+  end
+
   def validate_assigned_to_related
     return unless assigned_to
     return if assigned_to.organisation == owning_organisation || assigned_to.organisation == managing_organisation
@@ -891,12 +898,17 @@ private
   end
 
   def validate_owning_org_permitted
-    if owning_organisation && !bulk_upload.user.organisation.affiliated_stock_owners.include?(owning_organisation)
-      block_log_creation!
+    return unless owning_organisation
+    return if bulk_upload_organisation.affiliated_stock_owners.include?(owning_organisation)
 
-      if errors[:field_1].blank?
-        errors.add(:field_1, "You do not have permission to add logs for this owning organisation", category: :setup)
-      end
+    block_log_creation!
+
+    return if errors[:field_1].present?
+
+    if bulk_upload.user.support?
+      errors.add(:field_1, "This owning organisation is not affiliated with #{bulk_upload_organisation.name}", category: :setup)
+    else
+      errors.add(:field_1, "You do not have permission to add logs for this owning organisation", category: :setup)
     end
   end
 
@@ -1137,7 +1149,7 @@ private
     attributes["renewal"] = renewal
     attributes["scheme"] = scheme
     attributes["location"] = location
-    attributes["assigned_to"] = assigned_to || bulk_upload.user
+    attributes["assigned_to"] = assigned_to || (bulk_upload.user.support? ? nil : bulk_upload.user)
     attributes["created_by"] = bulk_upload.user
     attributes["needstype"] = field_4
     attributes["rent_type"] = RENT_TYPE_BU_MAPPING[field_11]
@@ -1625,4 +1637,8 @@ private
   def reason_is_other?
     field_98 == 20
   end
+
+  def bulk_upload_organisation
+    Organisation.find(bulk_upload.organisation_id)
+  end
 end
diff --git a/app/services/bulk_upload/sales/year2024/row_parser.rb b/app/services/bulk_upload/sales/year2024/row_parser.rb
index 8be08d62f..0b7a70c27 100644
--- a/app/services/bulk_upload/sales/year2024/row_parser.rb
+++ b/app/services/bulk_upload/sales/year2024/row_parser.rb
@@ -462,6 +462,7 @@ class BulkUpload::Sales::Year2024::RowParser
 
   validate :validate_assigned_to_exists, on: :after_log
   validate :validate_assigned_to_related, on: :after_log
+  validate :validate_assigned_to_when_support, on: :after_log
   validate :validate_managing_org_related, on: :after_log
   validate :validate_relevant_collection_window, on: :after_log
   validate :validate_incomplete_soft_validations, on: :after_log
@@ -925,7 +926,7 @@ private
 
     attributes["owning_organisation"] = owning_organisation
     attributes["managing_organisation"] = managing_organisation
-    attributes["assigned_to"] = assigned_to || bulk_upload.user
+    attributes["assigned_to"] = assigned_to || (bulk_upload.user.support? ? nil : bulk_upload.user)
     attributes["created_by"] = bulk_upload.user
     attributes["hhregres"] = field_72
     attributes["hhregresstill"] = field_73
@@ -1302,12 +1303,17 @@ private
   end
 
   def validate_owning_org_permitted
-    if owning_organisation && !bulk_upload.user.organisation.affiliated_stock_owners.include?(owning_organisation)
-      block_log_creation!
+    return unless owning_organisation
+    return if bulk_upload_organisation.affiliated_stock_owners.include?(owning_organisation)
 
-      if errors[:field_1].blank?
-        errors.add(:field_1, "You do not have permission to add logs for this owning organisation", category: :setup)
-      end
+    block_log_creation!
+
+    return if errors[:field_1].present?
+
+    if bulk_upload.user.support?
+      errors.add(:field_1, "This owning organisation is not affiliated with #{bulk_upload_organisation.name}", category: :setup)
+    else
+      errors.add(:field_1, "You do not have permission to add logs for this owning organisation", category: :setup)
     end
   end
 
@@ -1319,6 +1325,12 @@ private
     end
   end
 
+  def validate_assigned_to_when_support
+    if field_3.blank? && bulk_upload.user.support?
+      errors.add(:field_3, category: :setup, message: I18n.t("validations.not_answered", question: "what is the CORE username of the account this sales log should be assigned to?"))
+    end
+  end
+
   def validate_assigned_to_related
     return unless assigned_to
     return if assigned_to.organisation == owning_organisation || assigned_to.organisation == managing_organisation
@@ -1492,4 +1504,8 @@ private
   def valid_nationality_options
     %w[0] + GlobalConstants::COUNTRIES_ANSWER_OPTIONS.keys # 0 is "Prefers not to say"
   end
+
+  def bulk_upload_organisation
+    Organisation.find(bulk_upload.organisation_id)
+  end
 end
diff --git a/app/views/bulk_upload_lettings_logs/forms/checking_file.html.erb b/app/views/bulk_upload_lettings_logs/forms/checking_file.html.erb
index 7524072a5..2e6d4cd10 100644
--- a/app/views/bulk_upload_lettings_logs/forms/checking_file.html.erb
+++ b/app/views/bulk_upload_lettings_logs/forms/checking_file.html.erb
@@ -6,6 +6,7 @@
   <div class="govuk-grid-column-two-thirds">
     <%= form_with model: @form, scope: :form, url: bulk_upload_lettings_log_path(id: "prepare-your-file"), method: :patch do |f| %>
       <%= f.hidden_field :year %>
+      <%= f.hidden_field :organisation_id %>
 
       <span class="govuk-caption-l">Upload lettings logs in bulk (<%= @form.year_combo %>)</span>
       <h1 class="govuk-heading-l">We’re checking the file</h1>
diff --git a/app/views/bulk_upload_lettings_logs/forms/needstype.erb b/app/views/bulk_upload_lettings_logs/forms/needstype.erb
index 6deec7e1d..644dd9f5f 100644
--- a/app/views/bulk_upload_lettings_logs/forms/needstype.erb
+++ b/app/views/bulk_upload_lettings_logs/forms/needstype.erb
@@ -7,6 +7,7 @@
     <%= form_with model: @form, scope: :form, url: bulk_upload_lettings_log_path(id: "needstype"), method: :patch do |f| %>
       <%= f.govuk_error_summary %>
       <%= f.hidden_field :year %>
+      <%= f.hidden_field :organisation_id %>
 
       <%= f.govuk_collection_radio_buttons :needstype,
             @form.options,
diff --git a/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2024.html.erb b/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2024.html.erb
index 45ce4b385..7657396f1 100644
--- a/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2024.html.erb
+++ b/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2024.html.erb
@@ -6,6 +6,7 @@
   <div class="govuk-grid-column-two-thirds">
     <%= form_with model: @form, scope: :form, url: bulk_upload_lettings_log_path(id: "prepare-your-file"), method: :patch do |f| %>
       <%= f.hidden_field :year %>
+      <%= f.hidden_field :organisation_id %>
 
       <span class="govuk-caption-l">Upload lettings logs in bulk (<%= @form.year_combo %>)</span>
       <h1 class="govuk-heading-l">Prepare your file</h1>
diff --git a/app/views/bulk_upload_lettings_logs/forms/upload_your_file.html.erb b/app/views/bulk_upload_lettings_logs/forms/upload_your_file.html.erb
index bbafc28b3..2814eb1ea 100644
--- a/app/views/bulk_upload_lettings_logs/forms/upload_your_file.html.erb
+++ b/app/views/bulk_upload_lettings_logs/forms/upload_your_file.html.erb
@@ -7,6 +7,7 @@
     <%= form_with model: @form, scope: :form, url: bulk_upload_lettings_log_path(id: "upload-your-file"), method: :patch do |f| %>
       <%= f.hidden_field :year %>
       <%= f.hidden_field :needstype %>
+      <%= f.hidden_field :organisation_id %>
 
       <%= f.govuk_error_summary %>
 
diff --git a/app/views/bulk_upload_lettings_logs/forms/year.html.erb b/app/views/bulk_upload_lettings_logs/forms/year.html.erb
index 8ba1c280f..4fef16dd7 100644
--- a/app/views/bulk_upload_lettings_logs/forms/year.html.erb
+++ b/app/views/bulk_upload_lettings_logs/forms/year.html.erb
@@ -4,6 +4,7 @@
 
 <%= form_with model: @form, scope: :form, url: bulk_upload_lettings_log_path(id: "year"), method: :patch do |f| %>
   <%= f.govuk_error_summary %>
+  <%= f.hidden_field :organisation_id %>
 
   <%= f.govuk_collection_radio_buttons :year,
         @form.options,
diff --git a/app/views/bulk_upload_lettings_results/show.html.erb b/app/views/bulk_upload_lettings_results/show.html.erb
index 30a6fd585..15c486b91 100644
--- a/app/views/bulk_upload_lettings_results/show.html.erb
+++ b/app/views/bulk_upload_lettings_results/show.html.erb
@@ -25,4 +25,4 @@
   </div>
 </div>
 
-<%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path %>
+<%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path(organisation_id: @bulk_upload.organisation_id) %>
diff --git a/app/views/bulk_upload_lettings_results/summary.html.erb b/app/views/bulk_upload_lettings_results/summary.html.erb
index b144793bf..8a59e8999 100644
--- a/app/views/bulk_upload_lettings_results/summary.html.erb
+++ b/app/views/bulk_upload_lettings_results/summary.html.erb
@@ -29,4 +29,4 @@
   <% end %>
 </div>
 
-<%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path %>
+<%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path(organisation_id: @bulk_upload.organisation_id) %>
diff --git a/app/views/bulk_upload_sales_logs/forms/checking_file.html.erb b/app/views/bulk_upload_sales_logs/forms/checking_file.html.erb
index c47a97d5e..8bcef5cad 100644
--- a/app/views/bulk_upload_sales_logs/forms/checking_file.html.erb
+++ b/app/views/bulk_upload_sales_logs/forms/checking_file.html.erb
@@ -6,6 +6,7 @@
   <div class="govuk-grid-column-two-thirds">
     <%= form_with model: @form, scope: :form, url: bulk_upload_sales_log_path(id: "prepare-your-file"), method: :patch do |f| %>
       <%= f.hidden_field :year %>
+      <%= f.hidden_field :organisation_id %><%= f.hidden_field :organisation_id %>
 
       <span class="govuk-caption-l">Upload sales logs in bulk (<%= @form.year_combo %>)</span>
       <h1 class="govuk-heading-l">We’re checking the file</h1>
diff --git a/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2024.html.erb b/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2024.html.erb
index 81c947e15..2a64de689 100644
--- a/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2024.html.erb
+++ b/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2024.html.erb
@@ -6,6 +6,7 @@
   <div class="govuk-grid-column-two-thirds">
     <%= form_with model: @form, scope: :form, url: bulk_upload_sales_log_path(id: "prepare-your-file"), method: :patch do |f| %>
       <%= f.hidden_field :year %>
+      <%= f.hidden_field :organisation_id %>
 
       <span class="govuk-caption-l">Upload sales logs in bulk (<%= @form.year_combo %>)</span>
       <h1 class="govuk-heading-l">Prepare your file</h1>
diff --git a/app/views/bulk_upload_sales_logs/forms/upload_your_file.html.erb b/app/views/bulk_upload_sales_logs/forms/upload_your_file.html.erb
index 36e7240dc..75350ae9e 100644
--- a/app/views/bulk_upload_sales_logs/forms/upload_your_file.html.erb
+++ b/app/views/bulk_upload_sales_logs/forms/upload_your_file.html.erb
@@ -6,6 +6,7 @@
   <div class="govuk-grid-column-two-thirds">
     <%= form_with model: @form, scope: :form, url: bulk_upload_sales_log_path(id: "upload-your-file"), method: :patch do |f| %>
       <%= f.hidden_field :year %>
+      <%= f.hidden_field :organisation_id %>
 
       <%= f.govuk_error_summary %>
 
diff --git a/app/views/bulk_upload_sales_logs/forms/year.html.erb b/app/views/bulk_upload_sales_logs/forms/year.html.erb
index d8aa09172..2aaa7c0f8 100644
--- a/app/views/bulk_upload_sales_logs/forms/year.html.erb
+++ b/app/views/bulk_upload_sales_logs/forms/year.html.erb
@@ -4,6 +4,7 @@
 
 <%= form_with model: @form, scope: :form, url: bulk_upload_sales_log_path(id: "year"), method: :patch do |f| %>
   <%= f.govuk_error_summary %>
+  <%= f.hidden_field :organisation_id %>
 
   <%= f.govuk_collection_radio_buttons :year,
         @form.options,
diff --git a/app/views/bulk_upload_sales_results/show.html.erb b/app/views/bulk_upload_sales_results/show.html.erb
index 0d645db33..776fdfa2f 100644
--- a/app/views/bulk_upload_sales_results/show.html.erb
+++ b/app/views/bulk_upload_sales_results/show.html.erb
@@ -25,4 +25,4 @@
   </div>
 </div>
 
-<%= govuk_button_link_to "Upload your file again", start_bulk_upload_sales_logs_path %>
+<%= govuk_button_link_to "Upload your file again", start_bulk_upload_sales_logs_path(organisation_id: @bulk_upload.organisation_id) %>
diff --git a/app/views/bulk_upload_sales_results/summary.html.erb b/app/views/bulk_upload_sales_results/summary.html.erb
index 0fa51b9dc..0e423621d 100644
--- a/app/views/bulk_upload_sales_results/summary.html.erb
+++ b/app/views/bulk_upload_sales_results/summary.html.erb
@@ -29,4 +29,4 @@
   <% end %>
 </div>
 
-<%= govuk_button_link_to "Upload your file again", start_bulk_upload_sales_logs_path %>
+<%= govuk_button_link_to "Upload your file again", start_bulk_upload_sales_logs_path(organisation_id: @bulk_upload.organisation_id) %>
diff --git a/app/views/logs/_create_for_org_actions.html.erb b/app/views/logs/_create_for_org_actions.html.erb
index f6ed9ad82..0564e678a 100644
--- a/app/views/logs/_create_for_org_actions.html.erb
+++ b/app/views/logs/_create_for_org_actions.html.erb
@@ -1,10 +1,12 @@
 <div class="govuk-button-group app-filter-toggle">
   <% if @organisation.data_protection_confirmed? %>
     <% if current_page?(controller: 'organisations', action: 'lettings_logs') %>
-      <%= govuk_button_to "Create a new lettings log for this organisation", lettings_logs_path(lettings_log: { owning_organisation_id: @organisation.id }, method: :post) %>
-    <% end %>
+      <%= govuk_button_to "Create a new lettings log for this organisation", lettings_logs_path(lettings_log: { owning_organisation_id: @organisation.id }, method: :post), class: "govuk-!-margin-right-6" %>
+      <%= govuk_button_link_to "Upload lettings logs in bulk", bulk_upload_lettings_log_path(id: "start", organisation_id: @organisation.id), secondary: true %>
+<% end %>
     <% if current_page?(controller: 'organisations', action: 'sales_logs') %>
-      <%= govuk_button_to "Create a new sales log for this organisation", sales_logs_path(sales_log: { owning_organisation_id: @organisation.id }, method: :post) %>
-    <% end %>
+      <%= govuk_button_to "Create a new sales log for this organisation", sales_logs_path(sales_log: { owning_organisation_id: @organisation.id }, method: :post), class: "govuk-!-margin-right-6" %>
+      <%= govuk_button_link_to "Upload sales logs in bulk", bulk_upload_sales_log_path(id: "start", organisation_id: @organisation.id), secondary: true %>
+<% end %>
   <% end %>
 </div>
diff --git a/db/migrate/20240905092332_add_organisation_id_to_bulk_uploads.rb b/db/migrate/20240905092332_add_organisation_id_to_bulk_uploads.rb
new file mode 100644
index 000000000..d9f4f16da
--- /dev/null
+++ b/db/migrate/20240905092332_add_organisation_id_to_bulk_uploads.rb
@@ -0,0 +1,5 @@
+class AddOrganisationIdToBulkUploads < ActiveRecord::Migration[7.0]
+  def change
+    add_column :bulk_uploads, :organisation_id, :integer
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index cd7708957..4d6f18b84 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -42,6 +42,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_09_23_145326) do
     t.text "choice"
     t.integer "total_logs_count"
     t.string "rent_type_fix_status", default: "not_applied"
+    t.integer "organisation_id"
     t.integer "moved_user_id"
     t.index ["identifier"], name: "index_bulk_uploads_on_identifier", unique: true
     t.index ["user_id"], name: "index_bulk_uploads_on_user_id"
diff --git a/spec/factories/bulk_upload.rb b/spec/factories/bulk_upload.rb
index c848fb071..cefe95c2b 100644
--- a/spec/factories/bulk_upload.rb
+++ b/spec/factories/bulk_upload.rb
@@ -9,6 +9,7 @@ FactoryBot.define do
     sequence(:filename) { |n| "bulk-upload-#{n}.csv" }
     needstype { 1 }
     rent_type_fix_status { BulkUpload.rent_type_fix_statuses.values.sample }
+    organisation_id { user.organisation_id }
 
     trait(:sales) do
       log_type { BulkUpload.log_types[:sales] }
diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb
index 1867285eb..3d65cda87 100644
--- a/spec/features/organisation_spec.rb
+++ b/spec/features/organisation_spec.rb
@@ -139,6 +139,10 @@ RSpec.describe "User Features" do
         expect(page).to have_button("Create a new lettings log for this organisation")
       end
 
+      it "shows a upload lettings logs in bulk link" do
+        expect(page).to have_link("Upload lettings logs in bulk")
+      end
+
       context "when creating a log for that organisation" do
         it "pre-fills the value for owning organisation for that log" do
           click_button("Create a new lettings log for this organisation")
@@ -230,6 +234,10 @@ RSpec.describe "User Features" do
         expect(page).to have_button("Create a new sales log for this organisation")
       end
 
+      it "shows a upload sales logs in bulk link" do
+        expect(page).to have_link("Upload sales logs in bulk")
+      end
+
       context "when creating a log for that organisation" do
         it "pre-fills the value for owning organisation for that log" do
           click_button("Create a new sales log for this organisation")
diff --git a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
index 3faa0a699..68de5d22b 100644
--- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
+++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
@@ -815,6 +815,23 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do
         end
       end
 
+      context "when blank and bulk upload user is support" do
+        let(:bulk_upload) { create(:bulk_upload, :sales, user: create(:user, :support), year: 2024) }
+
+        let(:attributes) { setup_section_params.merge(bulk_upload:, field_3: nil) }
+
+        it "is not permitted" do
+          parser.valid?
+          expect(parser.errors[:field_3]).to be_present
+          expect(parser.errors[:field_3]).to include("You must answer what is the CORE username of the account this letting log should be assigned to?")
+        end
+
+        it "blocks log creation" do
+          parser.valid?
+          expect(parser).to be_block_log_creation
+        end
+      end
+
       context "when user could not be found" do
         let(:attributes) { { bulk_upload:, field_3: "idonotexist@example.com" } }
 
@@ -1439,6 +1456,43 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do
           expect(parser.errors[:field_10]).to include(/Enter a date when the owning and managing organisation was active/)
         end
       end
+
+      context "when user is an unaffiliated non-support user and bulk upload organisation is affiliated with the owning organisation" do
+        let(:affiliated_org) { create(:organisation, :with_old_visible_id) }
+        let(:unaffiliated_user) { create(:user, organisation: create(:organisation)) }
+        let(:attributes) { { bulk_upload:, field_1: affiliated_org.old_visible_id } }
+        let(:organisation_id) { unaffiliated_user.organisation_id }
+
+        before do
+          create(:organisation_relationship, parent_organisation: owning_org, child_organisation: affiliated_org)
+          bulk_upload.update!(organisation_id:, user: unaffiliated_user)
+        end
+
+        it "blocks log creation and adds an error to field_1" do
+          parser = described_class.new(attributes)
+          parser.valid?
+          expect(parser).to be_block_log_creation
+          expect(parser.errors[:field_1]).to include("You do not have permission to add logs for this owning organisation")
+        end
+      end
+
+      context "when user is an unaffiliated support user and bulk upload organisation is affiliated with the owning organisation" do
+        let(:affiliated_org) { create(:organisation, :with_old_visible_id) }
+        let(:unaffiliated_support_user) { create(:user, :support, organisation: create(:organisation)) }
+        let(:attributes) { { bulk_upload:, field_1: affiliated_org.old_visible_id } }
+        let(:organisation_id) { affiliated_org.id }
+
+        before do
+          create(:organisation_relationship, parent_organisation: owning_org, child_organisation: affiliated_org)
+          bulk_upload.update!(organisation_id:, user: unaffiliated_support_user)
+        end
+
+        it "does not block log creation and does not add an error to field_1" do
+          parser = described_class.new(attributes)
+          parser.valid?
+          expect(parser.errors[:field_1]).not_to include("You do not have permission to add logs for this owning organisation")
+        end
+      end
     end
 
     describe "#field_2" do # managing org
diff --git a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
index e428f7792..ca8f29e92 100644
--- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
+++ b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
@@ -554,6 +554,43 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
           expect(parser.errors[:field_6]).to include(/Enter a date when the owning organisation was active/)
         end
       end
+
+      context "when user is an unaffiliated non-support user and bulk upload organisation is affiliated with the owning organisation" do
+        let(:affiliated_org) { create(:organisation, :with_old_visible_id) }
+        let(:unaffiliated_user) { create(:user, organisation: create(:organisation)) }
+        let(:attributes) { { bulk_upload:, field_1: affiliated_org.old_visible_id } }
+        let(:organisation_id) { unaffiliated_user.organisation_id }
+
+        before do
+          create(:organisation_relationship, parent_organisation: owning_org, child_organisation: affiliated_org)
+          bulk_upload.update!(organisation_id:, user: unaffiliated_user)
+        end
+
+        it "blocks log creation and adds an error to field_1" do
+          parser = described_class.new(attributes)
+          parser.valid?
+          expect(parser).to be_block_log_creation
+          expect(parser.errors[:field_1]).to include("You do not have permission to add logs for this owning organisation")
+        end
+      end
+
+      context "when user is an unaffiliated support user and bulk upload organisation is affiliated with the owning organisation" do
+        let(:affiliated_org) { create(:organisation, :with_old_visible_id) }
+        let(:unaffiliated_support_user) { create(:user, :support, organisation: create(:organisation)) }
+        let(:attributes) { { bulk_upload:, field_1: affiliated_org.old_visible_id } }
+        let(:organisation_id) { affiliated_org.id }
+
+        before do
+          create(:organisation_relationship, parent_organisation: owning_org, child_organisation: affiliated_org)
+          bulk_upload.update!(organisation_id:, user: unaffiliated_support_user)
+        end
+
+        it "does not block log creation and does not add an error to field_1" do
+          parser = described_class.new(attributes)
+          parser.valid?
+          expect(parser.errors[:field_1]).not_to include("You do not have permission to add logs for this owning organisation")
+        end
+      end
     end
 
     describe "#field_3" do # username for assigned_to
@@ -576,6 +613,23 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
         end
       end
 
+      context "when blank and bulk upload user is support" do
+        let(:bulk_upload) { create(:bulk_upload, :sales, user: create(:user, :support), year: 2024) }
+
+        let(:attributes) { setup_section_params.merge(bulk_upload:, field_3: nil) }
+
+        it "is not permitted" do
+          parser.valid?
+          expect(parser.errors[:field_3]).to be_present
+          expect(parser.errors[:field_3]).to include("You must answer what is the CORE username of the account this sales log should be assigned to?")
+        end
+
+        it "blocks log creation" do
+          parser.valid?
+          expect(parser).to be_block_log_creation
+        end
+      end
+
       context "when user could not be found" do
         let(:attributes) { { bulk_upload:, field_3: "idonotexist@example.com" } }
 

From 997e9bd62d2a45c808ec523a9e9ac764f5deda61 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Wed, 25 Sep 2024 09:36:03 +0100
Subject: [PATCH 7/7] CLDC-3615 Add pending email change banner (#2642)

* Add pending email change banner

* Update pending_email_change_banner_text and test descriptions

* Fix closing tag
---
 app/helpers/user_helper.rb       | 19 ++++++++++
 app/views/users/show.html.erb    |  9 +++++
 spec/helpers/user_helper_spec.rb | 61 ++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+)

diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb
index 06fe2bc7d..bbcb0acae 100644
--- a/app/helpers/user_helper.rb
+++ b/app/helpers/user_helper.rb
@@ -56,4 +56,23 @@ module UserHelper
       user.errors.add(attribute, message)
     end
   end
+
+  def display_pending_email_change_banner?(user)
+    user.unconfirmed_email.present? && user.email != user.unconfirmed_email
+  end
+
+  def pending_email_change_title_text(current_user, user)
+    if current_user == user
+      "You have requested to change your email address to #{user.unconfirmed_email}."
+    else
+      "There has been a request to change this user’s email address to #{user.unconfirmed_email}."
+    end
+  end
+
+  def pending_email_change_banner_text(current_user)
+    text = "A confirmation link has been sent to the new email address. The current email will continue to work until the change is confirmed."
+    text += " Deactivating this user will cancel the email change request." if current_user.support? || current_user.data_coordinator?
+
+    text
+  end
 end
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index 7f42107f8..a43b03f82 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -11,6 +11,15 @@
   <% end %>
 <% end %>
 
+<% if display_pending_email_change_banner?(@user) %>
+  <%= govuk_notification_banner(title_text: "Important") do %>
+    <p class="govuk-notification-banner__heading govuk-!-width-full" style="max-width: fit-content">
+      <%= pending_email_change_title_text(current_user, @user) %>
+    </p>
+    <%= pending_email_change_banner_text(current_user) %>
+  <% end %>
+<% end %>
+
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
     <h1 class="govuk-heading-l">
diff --git a/spec/helpers/user_helper_spec.rb b/spec/helpers/user_helper_spec.rb
index c88790aa0..eb0a672db 100644
--- a/spec/helpers/user_helper_spec.rb
+++ b/spec/helpers/user_helper_spec.rb
@@ -105,6 +105,32 @@ RSpec.describe UserHelper do
     end
   end
 
+  describe "display_pending_email_change_banner?" do
+    context "when the user doesn't have an unconfirmed email" do
+      let(:user) { FactoryBot.create(:user, :data_provider, unconfirmed_email: nil) }
+
+      it "does not display pending email change banner" do
+        expect(display_pending_email_change_banner?(user)).to be false
+      end
+    end
+
+    context "when the user has the same unconfirmed email as current email" do
+      let(:user) { FactoryBot.create(:user, :data_provider, unconfirmed_email: "updated_email@example.com", email: "updated_email@example.com") }
+
+      it "does not display pending email change banner" do
+        expect(display_pending_email_change_banner?(user)).to be false
+      end
+    end
+
+    context "when the user has a different unconfirmed email" do
+      let(:user) { FactoryBot.create(:user, :data_provider, unconfirmed_email: "updated_email@example.com", email: "old_email@example.com") }
+
+      it "displays pending email change banner" do
+        expect(display_pending_email_change_banner?(user)).to be true
+      end
+    end
+  end
+
   describe "organisation_change_confirmation_warning" do
     context "when user owns logs" do
       before do
@@ -147,4 +173,39 @@ RSpec.describe UserHelper do
       end
     end
   end
+
+  describe "pending_email_change_title_text" do
+    let(:user) { FactoryBot.create(:user, :data_provider, unconfirmed_email: "updated_email@example.com", email: "old_email@example.com") }
+    let(:current_user) { FactoryBot.create(:user, :support) }
+
+    context "when viewing own profile" do
+      it "returns the correct text" do
+        expect(pending_email_change_title_text(user, user)).to eq("You have requested to change your email address to updated_email@example.com.")
+      end
+    end
+
+    context "when viewing another user's profile" do
+      it "returns the correct text" do
+        expect(pending_email_change_title_text(current_user, user)).to eq("There has been a request to change this user’s email address to updated_email@example.com.")
+      end
+    end
+  end
+
+  describe "pending_email_change_banner_text" do
+    context "with provider user" do
+      let(:user) { FactoryBot.create(:user, :data_provider) }
+
+      it "returns the correct text" do
+        expect(pending_email_change_banner_text(user)).to eq("A confirmation link has been sent to the new email address. The current email will continue to work until the change is confirmed.")
+      end
+    end
+
+    context "with support user" do
+      let(:user) { FactoryBot.create(:user, :support) }
+
+      it "returns the correct text" do
+        expect(pending_email_change_banner_text(user)).to eq("A confirmation link has been sent to the new email address. The current email will continue to work until the change is confirmed. Deactivating this user will cancel the email change request.")
+      end
+    end
+  end
 end