Deleting a subject set removes subjects that are still members of other sets

[adammcmaster]

The subject remover (

panoptes/lib/subjects/remover.rb

Line 31 in 377fc00

def can_be_removed?

) does not check if the subject is a member of multiple sets before removing it, so it will delete the subject even if it is still in use in other subject sets.

[zwolf]

The subject remover doesn't reflect on the subject's linked sets. It only asks

panoptes/lib/subjects/remover.rb

Lines 31 to 40 in 377fc00

	def can_be_removed?
	return false if has_been_collected_or_classified?
	return false if has_been_talked_about?
	return false if has_been_counted_or_retired?
	# subject has no record of use in zooniverse
	true
	end

and then removes all the SetMemberSubjects entries if true. If a subject has been acted upon (classified, retired, commented, collected) on any workflow, on any project, then that subject is not orphaned and will be left. Just being linked to any set anywhere doesn't prevent a subject from being removed.

So in order to make this work, I'd modify the SubjectRemover to take a subject set id (if applicable) as an additional param along with the SubjectRemovalWorker that uses it and everywhere that calls that worker. Wouldn't affect subjects for which the removal worker was called directly (via SubjectsController) but would work for calls from SubjectSetsController and SetMemberSubjects controller.

I'll add it to the list!

[lcjohnso]

Note -- this is also an issue when cross-project subjects are in use. Example: one project team might delete a subject set and that could have the possible result of deleting the subject from all projects. Especially because the deletion could be / would be unexpected, best if we do not allow deletion if subject is linked elsewhere.

[yuenmichelle1]

Wouldn't affect subjects for which the removal worker was called directly (via SubjectsController) but would work for calls from SubjectSetsController and SetMemberSubjects controller.

@zwolf @lcjohnso I'm not quite sure I follow this ^ quoted part. If the removal call was called via SubjectsController, wouldn't we still want this additional check of "don't remove subject if subject belongs to a different subject set"? (Especially with Cliff's followup note)

I feel like I'm missing something so hopefully either of you can explain.

MAIN QUESTION: I'm wondering why adding optional subject_set_id parameter that will be sent down from SubjectRemovalWorker -> Subjects::Remover is more preferable vs checking the count of the subject's set_member_subjects (see remover.rb line 13)?

(I.e. if set_member_subjects.length > 1 [meaning the subject belongs to more than 1 subject_set] then do not remove)

panoptes/lib/subjects/remover.rb

Line 13 in 377fc00

set_member_subjects = orphan_subject.set_member_subjects

[yuenmichelle1]

Follow up on above ^ and posting follow up discussion had by BE team (Mar 21, 2024):

• One reason why checking length of set_member_subjects of the subject might be trickier is because that within SetMemberSubjectsController and SubjectSetController before it hits the SubjectRemovalWorker and Subjects::Remover the related set_member_subject already gets deleted.
  • If we had a check for set_member_subjects.length > 0 then there's an assumption that associated set_member_subject already gets deleted, and this delete would not work when being directly called from SubjectsController

Decision:

We decided to implement fix proposed in Zach's comment: #4250 (comment)
(i.e. updating SubjectRemovalWorker and Subjects::Remover to take in optional subject_set_id parameter that will be called from SubjectSetsController. and adding a check on remover to check that if there is a subject_set_id, check if subject belongs to another subject_set)

This will take care of this current issue. BUT what it does not take care of is if remover is called from SubjectsController or SetMemberSubjectsController, which if needed, will require a separate issue.

[lcjohnso]

Update: specific issue detailed here was addressed in #4306 and #4321.

Remaining open questions re: subject deletion are documented here: #4346