CLDC-NONE: Remove bundle exec from README

specify rbenv exec

remove confusing/outdated steps

include steps for RubyMine

Gemfile.lock

@@ -411,7 +411,7 @@ GEM
     responders (3.1.1)
       actionpack (>= 5.2)
       railties (>= 5.2)
-    rexml (3.3.9)
+    rexml (3.4.4)
     roo (2.10.1)
       nokogiri (~> 1)
       rubyzip (>= 1.3.0, < 3.0.0)

app/services/exports/lettings_log_export_service.rb

@@ -30,14 +30,32 @@ module Exports
       "core_#{year}_#{year + 1}_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, year)
-      if !full_update && recent_export
-        params = { from: recent_export.started_at, to: @start_time }
-        LettingsLog.exportable.where("(updated_at >= :from AND updated_at <= :to) OR (values_updated_at IS NOT NULL AND values_updated_at >= :from AND values_updated_at <= :to)", params).filter_by_year(year)
-      else
-        params = { to: @start_time }
-        LettingsLog.exportable.where("updated_at <= :to", params).filter_by_year(year)
-      end
+    def retrieve_resources_from_range(range, year)
+      relation = LettingsLog.exportable.filter_by_year(year).left_joins(:created_by, :updated_by, :assigned_to, :owning_organisation, :managing_organisation)
+
+      ids = relation
+        .where({ updated_at: range })
+        .or(
+          relation.where.not(values_updated_at: nil).where(values_updated_at: range),
+        )
+        .or(
+          relation.where.not({ created_by: { updated_at: nil } }).where({ created_by: { updated_at: range } }),
+        )
+        .or(
+          relation.where.not({ updated_by: { updated_at: nil } }).where({ updated_by: { updated_at: range } }),
+        )
+        .or(
+          relation.where.not({ assigned_to: { updated_at: nil } }).where({ assigned_to: { updated_at: range } }),
+        )
+        .or(
+          relation.where.not({ owning_organisation: { updated_at: nil } }).where({ owning_organisation: { updated_at: range } }),
+        )
+        .or(
+          relation.where.not({ managing_organisation: { updated_at: nil } }).where({ managing_organisation: { updated_at: range } }),
+        )
+        .pluck(:id)
+
+      LettingsLog.where(id: ids)
     end
 
     def apply_cds_transformation(lettings_log, export_mode)

app/services/exports/organisation_export_service.rb

@@ -25,24 +25,19 @@ module Exports
       "organisations_2024_2025_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, _year)
-      if !full_update && recent_export
-        params = { from: recent_export.started_at, to: @start_time }
+    def retrieve_resources_from_range(range, _year)
+      relation = Organisation.left_joins(:users, :organisation_name_changes)
+      ids = relation
+              .where({ updated_at: range })
+              .or(
+                relation.where(organisation_name_changes: { created_at: range }),
+              )
+              .or(
+                relation.where(users: { is_dpo: true, updated_at: range }),
+              )
+              .pluck(:id)
 
-        Organisation
-          .where(updated_at: params[:from]..params[:to])
-          .or(
-            Organisation.where(id: OrganisationNameChange.where(created_at: params[:from]..params[:to]).select(:organisation_id)),
-          )
-      else
-        params = { to: @start_time }
-
-        Organisation
-          .where("updated_at <= :to", params)
-          .or(
-            Organisation.where(id: OrganisationNameChange.where("created_at <= :to", params).select(:organisation_id)),
-          )
-      end
+      Organisation.where(id: ids)
     end
 
     def build_export_xml(organisations)

app/services/exports/sales_log_export_service.rb

@@ -30,14 +30,32 @@ module Exports
       "core_sales_#{year}_#{year + 1}_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, year)
-      if !full_update && recent_export
-        params = { from: recent_export.started_at, to: @start_time }
-        SalesLog.exportable.where("(updated_at >= :from AND updated_at <= :to) OR (values_updated_at IS NOT NULL AND values_updated_at >= :from AND values_updated_at <= :to)", params).filter_by_year(year)
-      else
-        params = { to: @start_time }
-        SalesLog.exportable.where("updated_at <= :to", params).filter_by_year(year)
-      end
+    def retrieve_resources_from_range(range, year)
+      relation = SalesLog.exportable.filter_by_year(year).left_joins(:created_by, :updated_by, :assigned_to, :owning_organisation, :managing_organisation)
+
+      ids = relation
+              .where({ updated_at: range })
+              .or(
+                relation.where.not(values_updated_at: nil).where(values_updated_at: range),
+              )
+              .or(
+                relation.where.not({ created_by: { updated_at: nil } }).where({ created_by: { updated_at: range } }),
+              )
+              .or(
+                relation.where.not({ updated_by: { updated_at: nil } }).where({ updated_by: { updated_at: range } }),
+              )
+              .or(
+                relation.where.not({ assigned_to: { updated_at: nil } }).where({ assigned_to: { updated_at: range } }),
+              )
+              .or(
+                relation.where.not({ owning_organisation: { updated_at: nil } }).where({ owning_organisation: { updated_at: range } }),
+              )
+              .or(
+                relation.where.not({ managing_organisation: { updated_at: nil } }).where({ managing_organisation: { updated_at: range } }),
+              )
+              .pluck(:id)
+
+      SalesLog.where(id: ids)
     end
 
     def apply_cds_transformation(sales_log, _export_mode)

app/services/exports/user_export_service.rb

@@ -25,14 +25,19 @@ module Exports
       "users_2024_2025_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, _year)
-      if !full_update && recent_export
-        params = { from: recent_export.started_at, to: @start_time }
-        User.where("(updated_at >= :from AND updated_at <= :to) OR (values_updated_at IS NOT NULL AND values_updated_at >= :from AND values_updated_at <= :to)", params)
-      else
-        params = { to: @start_time }
-        User.where("updated_at <= :to", params)
-      end
+    def retrieve_resources_from_range(range, _year)
+      relation = User.left_joins(organisation: :organisation_name_changes)
+      ids = relation
+              .where({ updated_at: range })
+              .or(
+                relation.where.not(organisations: { updated_at: nil }).where(organisations: { updated_at: range }),
+              )
+              .or(
+                relation.where(organisation_name_changes: { created_at: range }),
+              )
+              .pluck(:id)
+
+      User.where(id: ids)
     end
 
     def build_export_xml(users)

app/services/exports/xml_export_service.rb

@@ -31,6 +31,16 @@ module Exports
       Export.new(collection:, year:, started_at: @start_time, base_number:, increment_number:)
     end
 
+    def retrieve_resources(recent_export, full_update, year)
+      range = if !full_update && recent_export
+                recent_export.started_at..@start_time
+              else
+                ..@start_time
+              end
+
+      retrieve_resources_from_range(range, year)
+    end
+
     def write_export_archive(export, year, recent_export, full_update)
       archive = get_archive_name(year, export.base_number, export.increment_number)
 

docs/Gemfile.lock

@@ -226,7 +226,7 @@ GEM
     rb-fsevent (0.11.2)
     rb-inotify (0.10.1)
       ffi (~> 1.0)
-    rexml (3.3.9)
+    rexml (3.4.2)
     rouge (3.26.0)
     ruby2_keywords (0.0.5)
     rubyzip (2.3.2)

docs/setup.md

@@ -22,6 +22,8 @@ We recommend using [nvm](https://github.com/nvm-sh/nvm) to manage NodeJS version
 
 1. Install PostgreSQL
 
+   If you already have a valid Postgres installation you can skip this step.
+
    macOS:
 
    ```bash
@@ -38,6 +40,8 @@ We recommend using [nvm](https://github.com/nvm-sh/nvm) to manage NodeJS version
 
 2. Create a Postgres user
 
+   If you already have a valid Postgres installation you can skip this step.
+
    ```bash
    sudo su - postgres -c "createuser <username> -s -P"
    ```
@@ -84,25 +88,10 @@ We recommend using [nvm](https://github.com/nvm-sh/nvm) to manage NodeJS version
    brew install yarn
    ```
 
-   or you could run it without specifying the version and it should use the version from .nvmrc
-
-   ```bash
-   nvm install
-   nvm use
-   brew install yarn
-   ```
-
    Linux (Debian):
 
    ```bash
-   curl -sL https://deb.nodesource.com/setup_20.x | sudo bash -
-   sudo apt -y install nodejs
-   mkdir -p ~/.npm-packages
-   npm config set prefix ~/.npm-packages
-   echo 'NPM_PACKAGES="~/.npm-packages"' >> ~/.bashrc
-   echo 'export PATH="$PATH:$NPM_PACKAGES/bin"' >> ~/.bashrc
-   source ~/.bashrc
-   npm install --location=global yarn
+   npm install --global yarn
    ```
 
 6. (For running tests) Install Gecko Driver
@@ -147,22 +136,30 @@ Also ensure you have firefox installed
    bundle exec rake db:seed
    ```
 
-5. For Ordinance Survey related functionality, such as using the UPRN, you will need to set `OS_DATA_KEY` in your .env file. This key is shared across the team and can be found in AWS Secrets Manager.
-6. For email functionality, you will need a GOV.UK Notify API key, which is individual to you. Ask an existing team member to invite you to the "CORE Helpdesk" Notify service. Once invited, sign in and go to "API integration" to generate an API key, and set this as `GOVUK_NOTIFY_API_KEY` in your .env file.
+5. Build assets once before running the app for the first time:
+
+   ```bash
+   yarn build --mode=development
+   ```
+
+6. For Ordinance Survey related functionality, such as using the UPRN, you will need to set `OS_DATA_KEY` in your .env file. This key is shared across the team and can be found in AWS Secrets Manager.
+7. For email functionality, you will need a GOV.UK Notify API key, which is individual to you. Ask an existing team member to invite you to the "CORE Helpdesk" Notify service. Once invited, sign in and go to "API integration" to generate an API key, and set this as `GOVUK_NOTIFY_API_KEY` in your .env file.
 
 ## Running Locally
 
 ### Application
 
-Start the dev servers
+Start the dev servers via one of the following methods:
+
+a. If using RubyMine, run the "submit-social-housing-lettings-and-sales-data" Rails configuration.
 
-a. Using Foreman:
+b. Using Foreman:
 
 ```bash
 ./bin/dev
 ```
 
-b. Individually:
+c. Individually:
 
 Rails:
 
@@ -176,20 +173,18 @@ JavaScript (for hot reloading):
 yarn build --mode=development --watch
 ```
 
-If you’re not modifying front end assets you can bundle them as a one off task:
-
-```bash
-yarn build --mode=development
-```
-
 Development mode will target the latest versions of Chrome, Firefox and Safari for transpilation while production mode will target older browsers.
 
 The Rails server will start on <http://localhost:3000>.
 
 To sign in locally, you can use any username and password from your local database. The seed task in `seeds.rb` creates users in various roles all with the password `REVIEW_APP_USER_PASSWORD` from your .env file (which has default value `password`).
-To create any other users, you can edit the seed commands, or run similar commands in the rails console.
+To create any other users, you can log in as a support user and create new users via the admin interface. Or, you can edit the seed commands, or run similar commands in the rails console.
 You can also insert a new user row using SQL, but you will need to generate a correctly encrypted password. You can find the value to use for encrypted password which corresponds to the password `YOURPASSWORDHERE` using `User.new(:password => [YOURPASSWORDHERE]).encrypted_password`.
 
+### rbenv
+
+In general, whenever needing to run a Ruby command, use `bundle exec <command>` to ensure the correct Ruby version and gemset are used. Rbenv will automatically use the correct Ruby version.
+
 ### Debugging
 
 Add the line `binding.pry` to the code to pause the execution of the code at that line and open a powerful interactive console for debugging.
@@ -219,6 +214,8 @@ More details on debugging in RubyMine can be found at <https://www.jetbrains.com
 bundle exec rspec
 ```
 
+Or if using RubyMine, right click a spec file and select the 'Run' option.
+
 To run a specific folder use
 
 ```bash

spec/services/exports/lettings_log_export_service_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Exports::LettingsLogExportService do
+  include CollectionTimeHelper
+
   subject(:export_service) { described_class.new(storage_service, start_time) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -16,7 +18,7 @@ RSpec.describe Exports::LettingsLogExportService do
   let(:expected_manifest_filename) { "manifest.xml" }
   let(:start_time) { Time.zone.local(2022, 5, 1) }
   let(:organisation) { create(:organisation, name: "MHCLG", housing_registration_no: 1234) }
-  let(:user) { FactoryBot.create(:user, email: "test1@example.com", organisation:) }
+  let(:user) { create(:user, email: "test1@example.com", organisation:) }
 
   def replace_entity_ids(lettings_log, export_template)
     export_template.sub!(/\{id\}/, (lettings_log["id"] + Exports::LettingsLogExportService::LOG_ID_OFFSET).to_s)
@@ -480,6 +482,75 @@ RSpec.describe Exports::LettingsLogExportService do
         end
       end
     end
+
+    context "and one lettings log has not been updated in the time range" do
+      let(:expected_zip_filename) { "core_#{current_collection_start_year}_#{current_collection_end_year}_apr_mar_f0001_inc0001.zip" }
+      let(:start_time) { current_collection_start_date }
+      let!(:owning_organisation) { create(:organisation, name: "MHCLG owning", housing_registration_no: 1234) }
+      let!(:managing_organisation) { create(:organisation, name: "MHCLG managing", housing_registration_no: 1234) }
+      let!(:created_by_user) { create(:user, email: "test-created-by@example.com", organisation: managing_organisation) }
+      let!(:updated_by_user) { create(:user, email: "test-updated-by@example.com", organisation: managing_organisation) }
+      let!(:assigned_to_user) { create(:user, email: "test-assigned-to@example.com", organisation: managing_organisation) }
+      let!(:lettings_log) { create(:lettings_log, :completed, startdate: current_collection_start_date, created_by: created_by_user, updated_by: updated_by_user, assigned_to: assigned_to_user, owning_organisation:, managing_organisation:) }
+
+      before do
+        # touch all the related records to ensure their updated_at value is outside the export range
+        Timecop.freeze(start_time + 1.month)
+        owning_organisation.touch
+        managing_organisation.touch
+        created_by_user.touch
+        updated_by_user.touch
+        assigned_to_user.touch
+        lettings_log.touch
+        Timecop.freeze(start_time)
+      end
+
+      it "does not export the lettings log" do
+        expect(storage_service).not_to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if created_by_user is updated" do
+        created_by_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if updated_by_user is updated" do
+        updated_by_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if assigned_to_user is updated" do
+        assigned_to_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if owning_organisation is updated" do
+        owning_organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if managing_organisation is updated" do
+        managing_organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+    end
   end
 
   context "when exporting a supported housing lettings logs in XML" do

spec/services/exports/organisation_export_service_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Exports::OrganisationExportService do
+  include CollectionTimeHelper
+
   subject(:export_service) { described_class.new(storage_service, start_time) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -283,5 +285,40 @@ RSpec.describe Exports::OrganisationExportService do
         expect(export_service.export_xml_organisations).to eq({ expected_zip_filename.gsub(".zip", "") => start_time })
       end
     end
+
+    context "and one organisation has not been updated in the time range" do
+      let(:dpo_user) { create(:user, email: "dpo@example.com", is_dpo: true, organisation:) }
+      let(:organisation) { create(:organisation, with_dsa: false) }
+
+      before do
+        # touch all the related records to ensure their updated_at value is outside the export range
+        Timecop.freeze(start_time + 1.month)
+        organisation.touch
+        dpo_user.touch
+        Timecop.freeze(start_time)
+      end
+
+      it "does not export the organisation" do
+        expect(storage_service).not_to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_organisations
+      end
+
+      it "does export the organisation if an organisation name change is made" do
+        FactoryBot.create(:organisation_name_change, organisation:)
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_organisations
+      end
+
+      it "does export the organisation if dpo_user is updated" do
+        dpo_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_organisations
+      end
+    end
   end
 end

spec/services/exports/sales_log_export_service_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Exports::SalesLogExportService do
+  include CollectionTimeHelper
+
   subject(:export_service) { described_class.new(storage_service, start_time) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -421,5 +423,74 @@ RSpec.describe Exports::SalesLogExportService do
         end
       end
     end
+
+    context "and one sales log has not been updated in the time range" do
+      let(:expected_zip_filename) { "core_sales_#{current_collection_start_year}_#{current_collection_end_year}_apr_mar_f0001_inc0001.zip" }
+      let(:start_time) { current_collection_start_date }
+      let!(:owning_organisation) { create(:organisation, name: "MHCLG owning", housing_registration_no: 1234) }
+      let!(:managing_organisation) { create(:organisation, name: "MHCLG managing", housing_registration_no: 1234) }
+      let!(:created_by_user) { create(:user, email: "test-created-by@example.com", organisation: managing_organisation) }
+      let!(:updated_by_user) { create(:user, email: "test-updated-by@example.com", organisation: managing_organisation) }
+      let!(:assigned_to_user) { create(:user, email: "test-assigned-to@example.com", organisation: managing_organisation) }
+      let!(:sales_log) { create(:sales_log, :export, saledate: start_time, assigned_to: assigned_to_user, created_by: created_by_user, updated_by: updated_by_user, owning_organisation:, managing_organisation:) }
+
+      before do
+        # touch all the related records to ensure their updated_at value is outside the export range
+        Timecop.freeze(start_time + 1.month)
+        owning_organisation.touch
+        managing_organisation.touch
+        created_by_user.touch
+        updated_by_user.touch
+        assigned_to_user.touch
+        sales_log.touch
+        Timecop.freeze(start_time)
+      end
+
+      it "does not export the sales log" do
+        expect(storage_service).not_to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if created_by_user is updated" do
+        created_by_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if updated_by_user is updated" do
+        updated_by_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if assigned_to_user is updated" do
+        assigned_to_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if owning_organisation is updated" do
+        owning_organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if managing_organisation is updated" do
+        managing_organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+    end
   end
 end

spec/services/exports/user_export_service_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Exports::UserExportService do
+  include CollectionTimeHelper
+
   subject(:export_service) { described_class.new(storage_service, start_time) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -235,5 +237,40 @@ RSpec.describe Exports::UserExportService do
         expect(export_service.export_xml_users).to eq({ expected_zip_filename.gsub(".zip", "") => start_time })
       end
     end
+
+    context "and one user has not been updated in the time range" do
+      let(:start_time) { current_collection_start_date }
+      let!(:user) { create(:user, organisation:) }
+
+      before do
+        # touch all the related records to ensure their updated_at value is outside the export range
+        Timecop.freeze(start_time + 1.month)
+        organisation.touch
+        user.touch
+        Timecop.freeze(start_time)
+      end
+
+      it "does not export the user" do
+        expect(storage_service).not_to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_users
+      end
+
+      it "does export the user if organisation is updated" do
+        organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_users
+      end
+
+      it "does export the user if an organisation name change is made" do
+        FactoryBot.create(:organisation_name_change, organisation:)
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_users
+      end
+    end
   end
 end