feat: tech review comments

app/components/log_summary_component.html.erb

@@ -25,7 +25,7 @@
         <% end %>
       </header>
 
-      <% if log.is_a?(LettingsLog) && (log.needstype? or log.startdate?) %>
+      <% if log.lettings? && (log.needstype? or log.startdate?) %>
         <p class="govuk-body govuk-!-margin-bottom-2">
           <% if log.needstype? %>
             <%= log.is_general_needs? ? "General needs" : "Supported housing" %><br>
@@ -37,13 +37,14 @@
       <% end %>
 
       <% if current_user.support? %>
-        <% if log.owning_organisation %>
         <dl class="app-metadata">
+          <% if log.owning_organisation %>
             <div class="app-metadata__item">
               <dt class="app-metadata__term">Owned by</dt>
               <dd class="app-metadata__definition"><%= log.owning_organisation&.name %></dd>
             </div>
-            <% if log.instance_of?(LettingsLog) && log.managing_organisation %>
+          <% end %>
+          <% if log.lettings? && log.managing_organisation %>
             <div class="app-metadata__item">
               <dt class="app-metadata__term">Managed by</dt>
               <dd class="app-metadata__definition"><%= log.managing_organisation&.name %></dd>
@@ -51,7 +52,6 @@
           <% end %>
         </dl>
       <% end %>
-      <% end %>
     </div>
 
     <footer class="govuk-grid-column-one-half app-log-summary__footer">

app/services/filter_service.rb

@@ -18,7 +18,7 @@ class FilterService
     end
     logs = logs.order(created_at: :desc)
     if user.support?
-      if logs.first.instance_of?(LettingsLog)
+      if logs.first.lettings?
         logs.all.includes(:owning_organisation, :managing_organisation)
       else
         logs.all.includes(:owning_organisation)

db/schema.rb

@@ -10,10 +10,24 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_01_10_094518) do
+ActiveRecord::Schema[7.0].define(version: 2023_01_13_125117) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
+  create_table "bulk_upload_errors", force: :cascade do |t|
+    t.bigint "bulk_upload_id"
+    t.text "cell"
+    t.text "row"
+    t.text "tenant_code"
+    t.text "property_ref"
+    t.text "purchaser_code"
+    t.text "field"
+    t.text "error"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["bulk_upload_id"], name: "index_bulk_upload_errors_on_bulk_upload_id"
+  end
+
   create_table "bulk_uploads", force: :cascade do |t|
     t.bigint "user_id"
     t.text "log_type", null: false
@@ -236,7 +250,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_10_094518) do
     t.string "old_id"
     t.integer "joint"
     t.bigint "created_by_id"
-    t.integer "illness_type_0"
     t.integer "retirement_value_check"
     t.integer "tshortfall_known"
     t.integer "sheltered"
@@ -469,7 +482,21 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_10_094518) do
     t.integer "hoday"
     t.integer "homonth"
     t.integer "hoyear"
+    t.integer "fromprop"
+    t.integer "socprevten"
+    t.integer "mortgagelender"
+    t.string "mortgagelenderother"
     t.integer "mortlen"
+    t.integer "extrabor"
+    t.integer "hhmemb"
+    t.integer "totadult"
+    t.integer "totchild"
+    t.integer "hhtype"
+    t.string "pcode1"
+    t.string "pcode2"
+    t.integer "pcodenk"
+    t.string "postcode_full"
+    t.boolean "is_la_inferred"
     t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id"
     t.index ["owning_organisation_id"], name: "index_sales_logs_on_owning_organisation_id"
     t.index ["updated_by_id"], name: "index_sales_logs_on_updated_by_id"
@@ -498,13 +525,11 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_10_094518) do
     t.string "intended_stay"
     t.datetime "end_date"
     t.integer "has_other_client_group"
-    t.bigint "managing_organisation_id"
     t.string "arrangement_type"
     t.string "old_id"
     t.string "old_visible_id"
     t.integer "total_units"
     t.boolean "confirmed"
-    t.index ["managing_organisation_id"], name: "index_schemes_on_managing_organisation_id"
     t.index ["owning_organisation_id"], name: "index_schemes_on_owning_organisation_id"
   end
 
@@ -569,7 +594,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_10_094518) do
   add_foreign_key "organisation_relationships", "organisations", column: "child_organisation_id"
   add_foreign_key "organisation_relationships", "organisations", column: "parent_organisation_id"
   add_foreign_key "sales_logs", "organisations", column: "owning_organisation_id", on_delete: :cascade
-  add_foreign_key "schemes", "organisations", column: "managing_organisation_id"
   add_foreign_key "schemes", "organisations", column: "owning_organisation_id", on_delete: :cascade
   add_foreign_key "users", "organisations", on_delete: :cascade
 end

spec/components/log_summary_component_spec.rb

@@ -5,30 +5,49 @@ RSpec.describe LogSummaryComponent, type: :component do
   let(:coordinator_user) { FactoryBot.create(:user) }
   let(:propcode) { "P3647" }
   let(:tenancycode) { "T62863" }
-  let(:log) { FactoryBot.create(:lettings_log, needstype: 1, startdate: Time.utc(2022, 1, 1), tenancycode:, propcode:) }
+  let(:lettings_log) { FactoryBot.create(:lettings_log, needstype: 1, startdate: Time.utc(2022, 1, 1), tenancycode:, propcode:) }
+  let(:sales_log) { FactoryBot.create(:sales_log) }
 
-  context "when rendering log for a support user" do
+  context "when rendering lettings log for a support user" do
     it "show the log summary with organisational relationships" do
-      result = render_inline(described_class.new(current_user: support_user, log:))
+      result = render_inline(described_class.new(current_user: support_user, log: lettings_log))
 
-      expect(result).to have_link(log.id.to_s)
+      expect(result).to have_link(lettings_log.id.to_s)
-      expect(result).to have_text(log.tenancycode)
+      expect(result).to have_text(lettings_log.tenancycode)
-      expect(result).to have_text(log.propcode)
+      expect(result).to have_text(lettings_log.propcode)
       expect(result).to have_text("General needs")
       expect(result).to have_text("Tenancy starts 1 January 2022")
       expect(result).to have_text("Created 8 February 2022")
       expect(result).to have_text("by Danny Rojas")
       expect(result).to have_content("Owned by\n              DLUHC")
-      expect(result).not_to have_content("Managed by\n              DLUHC")
+      expect(result).to have_content("Managed by\n              DLUHC")
     end
   end
 
-  context "when rendering log for a data coordinator user" do
+  context "when rendering lettings log for a data coordinator user" do
     it "show the log summary" do
-      result = render_inline(described_class.new(current_user: coordinator_user, log:))
+      result = render_inline(described_class.new(current_user: coordinator_user, log: lettings_log))
 
-      expect(result).not_to have_content("Owned by\n              DLUHC")
+      expect(result).not_to have_content("Owned by")
-      expect(result).not_to have_content("Managed by\n              DLUHC")
+      expect(result).not_to have_content("Managed by")
+    end
+  end
+
+  context "when rendering sales log for a support user" do
+    it "show the log summary with organisational relationships" do
+      result = render_inline(described_class.new(current_user: support_user, log: sales_log))
+
+      expect(result).to have_content("Owned by\n              DLUHC")
+      expect(result).not_to have_content("Managed by")
+    end
+  end
+
+  context "when rendering sales log for a data coordinator user" do
+    it "show the log summary" do
+      result = render_inline(described_class.new(current_user: coordinator_user, log: sales_log))
+
+      expect(result).not_to have_content("Owned by")
+      expect(result).not_to have_content("Managed by")
     end
   end
 end

spec/models/sales_log_spec.rb

@@ -85,12 +85,6 @@ RSpec.describe SalesLog, type: :model do
       create(:sales_log, :completed, owning_organisation: organisation_2)
     end
 
-    it "filters by given organisation id" do
-      expect(described_class.filter_by_organisation([organisation_1.id]).count).to eq(2)
-      expect(described_class.filter_by_organisation([organisation_1.id, organisation_2.id]).count).to eq(3)
-      expect(described_class.filter_by_organisation([organisation_3.id]).count).to eq(0)
-    end
-
     it "filters by given organisation" do
       expect(described_class.filter_by_organisation([organisation_1]).count).to eq(2)
       expect(described_class.filter_by_organisation([organisation_1, organisation_2]).count).to eq(3)