diff --git a/lib/buildings_api.rb b/lib/buildings_api.rb index 2c6d7002..b54f1228 100644 --- a/lib/buildings_api.rb +++ b/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) diff --git a/lib/classroom_api.rb b/lib/classroom_api.rb index 5b98b8bd..192a7dd6 100644 --- a/lib/classroom_api.rb +++ b/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) diff --git a/spec/lib/buildings_api_spec.rb b/spec/lib/buildings_api_spec.rb index e12b7b03..bf3eb15d 100644 --- a/spec/lib/buildings_api_spec.rb +++ b/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 diff --git a/spec/lib/classroom_api_spec.rb b/spec/lib/classroom_api_spec.rb index 7135d105..95ab8a2f 100644 --- a/spec/lib/classroom_api_spec.rb +++ b/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