Skip to content

Commit

Permalink
switch order within can_be_removed to ensure we have orphan subject b…
Browse files Browse the repository at this point in the history
…efore querying for attributes of orphan subject (#4321)
  • Loading branch information
yuenmichelle1 authored Apr 30, 2024
1 parent 990fe05 commit eac5139
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 10 deletions.
4 changes: 2 additions & 2 deletions lib/subjects/remover.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ def cleanup
private

def can_be_removed?
return false if belongs_to_other_subject_set?

return false if has_been_collected_or_classified?

return false if belongs_to_other_subject_set?

return false if has_been_talked_about?

return false if has_been_counted_or_retired?
Expand Down
32 changes: 24 additions & 8 deletions spec/lib/subjects/remover_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,30 @@
expect { SetMemberSubject.find(sms_ids) }.to raise_error(ActiveRecord::RecordNotFound)
end

context 'with multiple subject sets' do
let(:alternate_subject_set) { create(:subject_set) }
let(:remover) { described_class.new(subject.id, panoptes_client, alternate_subject_set.id) }
let(:new_sms) { create(:set_member_subject, subject: subject, subject_set: alternate_subject_set) }

it 'does not remove subjects associated with multiple set_member_subjects' do
remover.cleanup
expect(Subject.where(id: subject.id)).to exist
context 'when subject_set_id is param in init' do
let(:remover_with_subject_set) {
described_class.new(subject.id, panoptes_client, subject_set.id)
}

it 'removes a subject that has not been used' do
remover_with_subject_set.cleanup
expect { Subject.find(subject.id) }.to raise_error(ActiveRecord::RecordNotFound)
end

it 'does not remove a subject that has been classified' do
create(:classification, subjects: [subject])
expect(remover_with_subject_set.cleanup).to be_falsey
end

context 'with multiple subject sets' do
let(:alternate_subject_set) { create(:subject_set) }
let(:remover) { described_class.new(subject.id, panoptes_client, alternate_subject_set.id) }
let(:new_sms) { create(:set_member_subject, subject: subject, subject_set: alternate_subject_set) }

it 'does not remove subjects associated with multiple set_member_subjects' do
remover.cleanup
expect(Subject.where(id: subject.id)).to exist
end
end
end

Expand Down

0 comments on commit eac5139

Please sign in to comment.