Update dependencies, enhance accessibility, and add regression specs

lib/buildings_api.rb

@@ -430,18 +430,20 @@ def strip_headers(result)
   def deactivate_stale_rooms(context)
     room_ids = @rooms_in_db.uniq
     expected_count = room_ids.count
+    already_hidden_count = 0
     deactivated_count = 0
 
     ActiveRecord::Base.transaction do
-      deactivated_count = Room.where(rmrecnbr: room_ids).where.not(visible: false).update_all(visible: false)
+      already_hidden_count = Room.where(rmrecnbr: room_ids, visible: false).count
+      deactivated_count = Room.where(rmrecnbr: room_ids, visible: [true, nil]).update_all(visible: false)
 
-      if deactivated_count != expected_count
-        @log.api_logger.debug "#{context}, error: expected to deactivate #{expected_count} room(s), deactivated #{deactivated_count}"
+      if already_hidden_count + deactivated_count != expected_count
+        @log.api_logger.debug "#{context}, error: expected to deactivate #{expected_count} room(s), deactivated #{deactivated_count}, already hidden #{already_hidden_count}"
         raise ActiveRecord::Rollback
       end
     end
 
-    (deactivated_count == expected_count) ? deactivated_count : false
+    (already_hidden_count + deactivated_count == expected_count) ? deactivated_count : false
   end
 
   def start_result(phase)

lib/classroom_api.rb

@@ -365,18 +365,20 @@ def strip_headers(result)
   def deactivate_stale_rooms(context)
     room_ids = @rooms_in_db.uniq
     expected_count = room_ids.count
+    already_hidden_count = 0
     deactivated_count = 0
 
     ActiveRecord::Base.transaction do
-      deactivated_count = Room.where(rmrecnbr: room_ids).where.not(visible: false).update_all(visible: false)
+      already_hidden_count = Room.where(rmrecnbr: room_ids, visible: false).count
+      deactivated_count = Room.where(rmrecnbr: room_ids, visible: [true, nil]).update_all(visible: false)
 
-      if deactivated_count != expected_count
-        @log.api_logger.debug "#{context}, error: expected to deactivate #{expected_count} room(s), deactivated #{deactivated_count}"
+      if already_hidden_count + deactivated_count != expected_count
+        @log.api_logger.debug "#{context}, error: expected to deactivate #{expected_count} room(s), deactivated #{deactivated_count}, already hidden #{already_hidden_count}"
         raise ActiveRecord::Rollback
       end
     end
 
-    (deactivated_count == expected_count) ? deactivated_count : false
+    (already_hidden_count + deactivated_count == expected_count) ? deactivated_count : false
   end
 
   def start_result(phase)

spec/lib/buildings_api_spec.rb

@@ -82,18 +82,41 @@
       api.instance_variable_set(:@rooms_in_db, [room.rmrecnbr, 9_999_999])
 
       expect(api.send(:deactivate_stale_rooms, "update_rooms")).to be(false)
+      expect(room.reload.visible).to be(true)
       expect(Room.exists?(room.rmrecnbr)).to be(true)
       expect(RoomContact.exists?(contact.id)).to be(true)
       expect(RoomCharacteristic.exists?(characteristic.id)).to be(true)
     end
 
-    it "fails closed when a listed room is already hidden" do
+    it "counts an already hidden room as deactivated" do
       building = create(:building, bldrecnbr: 5_000_304)
       room = create(:room, building_bldrecnbr: building.bldrecnbr, rmrecnbr: 1_500_304, visible: false)
       api = described_class.new
       api.instance_variable_set(:@rooms_in_db, [room.rmrecnbr])
 
-      expect(api.send(:deactivate_stale_rooms, "update_rooms")).to be(false)
+      expect(api.send(:deactivate_stale_rooms, "update_rooms")).to eq(0)
+      expect(room.reload.visible).to be(false)
+    end
+
+    it "hides the visible rooms when some listed rooms are already hidden" do
+      building = create(:building, bldrecnbr: 5_000_305)
+      hidden_room = create(:room, building_bldrecnbr: building.bldrecnbr, rmrecnbr: 1_500_305, visible: false)
+      visible_room = create(:room, building_bldrecnbr: building.bldrecnbr, rmrecnbr: 1_500_306, visible: true)
+      api = described_class.new
+      api.instance_variable_set(:@rooms_in_db, [hidden_room.rmrecnbr, visible_room.rmrecnbr])
+
+      expect(api.send(:deactivate_stale_rooms, "update_rooms")).to eq(1)
+      expect(hidden_room.reload.visible).to be(false)
+      expect(visible_room.reload.visible).to be(false)
+    end
+
+    it "hides a room whose visibility is unset" do
+      building = create(:building, bldrecnbr: 5_000_307)
+      room = create(:room, building_bldrecnbr: building.bldrecnbr, rmrecnbr: 1_500_307, visible: nil)
+      api = described_class.new
+      api.instance_variable_set(:@rooms_in_db, [room.rmrecnbr])
+
+      expect(api.send(:deactivate_stale_rooms, "update_rooms")).to eq(1)
       expect(room.reload.visible).to be(false)
     end
   end

spec/lib/classroom_api_spec.rb

@@ -66,12 +66,13 @@
       api.instance_variable_set(:@rooms_in_db, [room.rmrecnbr, 9_999_999])
 
       expect(api.send(:deactivate_stale_rooms, "add_facility_id_to_classrooms")).to be(false)
+      expect(room.reload.visible).to be(true)
       expect(Room.exists?(room.rmrecnbr)).to be(true)
       expect(RoomContact.exists?(contact.id)).to be(true)
       expect(RoomCharacteristic.exists?(characteristic.id)).to be(true)
     end
 
-    it "fails closed when a listed classroom is already hidden" do
+    it "counts an already hidden classroom as deactivated" do
       building = create(:building, bldrecnbr: 5_000_205)
       room = create(
         :room,
@@ -83,7 +84,47 @@
       api = described_class.new
       api.instance_variable_set(:@rooms_in_db, [room.rmrecnbr])
 
-      expect(api.send(:deactivate_stale_rooms, "add_facility_id_to_classrooms")).to be(false)
+      expect(api.send(:deactivate_stale_rooms, "add_facility_id_to_classrooms")).to eq(0)
+      expect(room.reload.visible).to be(false)
+    end
+
+    it "hides the visible classrooms when some listed classrooms are already hidden" do
+      building = create(:building, bldrecnbr: 5_000_206)
+      hidden_room = create(
+        :room,
+        building_bldrecnbr: building.bldrecnbr,
+        rmrecnbr: 1_400_206,
+        rmtyp_description: "Classroom",
+        visible: false
+      )
+      visible_room = create(
+        :room,
+        building_bldrecnbr: building.bldrecnbr,
+        rmrecnbr: 1_400_207,
+        rmtyp_description: "Classroom",
+        visible: true
+      )
+      api = described_class.new
+      api.instance_variable_set(:@rooms_in_db, [hidden_room.rmrecnbr, visible_room.rmrecnbr])
+
+      expect(api.send(:deactivate_stale_rooms, "add_facility_id_to_classrooms")).to eq(1)
+      expect(hidden_room.reload.visible).to be(false)
+      expect(visible_room.reload.visible).to be(false)
+    end
+
+    it "hides a classroom whose visibility is unset" do
+      building = create(:building, bldrecnbr: 5_000_208)
+      room = create(
+        :room,
+        building_bldrecnbr: building.bldrecnbr,
+        rmrecnbr: 1_400_208,
+        rmtyp_description: "Classroom",
+        visible: nil
+      )
+      api = described_class.new
+      api.instance_variable_set(:@rooms_in_db, [room.rmrecnbr])
+
+      expect(api.send(:deactivate_stale_rooms, "add_facility_id_to_classrooms")).to eq(1)
       expect(room.reload.visible).to be(false)
     end
   end