search

MushroomObserver / mushroom-observer

A website for sharing observations of mushrooms.
https://mushroomobserver.org
MIT License
77 stars 25 forks source link

erase_user personal herbarium #2082

Open JoeCohen opened 3 months ago

JoeCohen commented 3 months ago

erase_user fails to destroy the user's personal herbarium This causes that herbarium to appear the broken references that are emailed to webmaster.

Cron mo@mushroomobserver /var/web/mo/bin/run script/check_for_broken_references Cron Daemon [2024-03-31] 12:14 AM (10 hours ago) [PST] to webmaster ... ALERT!! herbaria.personal_user_id = [5859, 5980, 6353, 6779] ...

The bug is easy to fix, but we need a test. The bug happens because: A. erase_user tries to delete the herbarium in delete_own_records, which executes

      Herbarium.where(personal_id: <user_id>).delete_all

B. But before doing that, erase_user does blank_out_public_references which executes

     Herbarium.where(personal_id: <user_id>).update_all(personal_id: 0)

So when erase_user gets to step A, there is no herbarium with personal_id: .

I think the fix is to omit step B. (There's no need to blank out the reference, as the herbarium will be destroyed.) And there should be a test.

pellaea commented 3 months ago

Are we sure there will be no herbarium_records referring to that personal herbarium after the user is deleted?

Hold on a second. Step B should prevent the ALERT check_for_broken_references detects. If it sets herbaria.personal_user_id = 0, then the script should no longer consider it broken. Or maybe it should be setting it to nil instead?

I appreciate you looking into these ALERTs. I was intending to get there myself someday, but of course hadn't found time to do so yet.

There may be insufficient forensic to decide how these herbaria got into this state, unfortunately. But I appreciate you trying!

I would personally be okay with checking to see if any of these personal herbaria with a deleted user have any herbarium records, and if not, just deleting the herbaria, as you say.

But I don't think User.erase_user needs to be fixed. Or if you do want to clean up unused personal herbaria, you should probably make sure you aren't creating even more broken references in the process! :) (That is, make sure no herbarium_records refer to the herbarium before you delete it. If there are, simply breaking the reference to the soon-no-longer-to-exist user record is sufficient, I think, which is exactly what it already does.)

If you are feeling particularly OCD, I bet there are "correctly stranded" herbaria already in the database that could also be culled. That is, personal herbaria for users that have been deleted and which have no herbarium_records associated, but which aren't being caught by the check_for_broken_references script because herbaria.personal_user has been cleared. But I doubt there are enough to warrant even a few minutes of work, in my opinion at least.

PS. Also note that herarium_curators also references herbaria. That's another reference which could potentially be broken if you delete an herbarium.

On Sun, Mar 31, 2024 at 2:28 PM Joseph D. Cohen @.***> wrote:

erase_user fails to destroy the user's personal herbarium This causes that herbarium to appear the broken references that are emailed to webmaster.

Cron @.*** /var/web/mo/bin/run script/check_for_broken_references Cron Daemon [2024-03-31] 12:14 AM (10 hours ago) [PST] to webmaster ... ALERT!! herbaria.personal_user_id = [5859, 5980, 6353, 6779] ...

The bug is easy to fix, but we need a test. The bug happens because: A. erase_user tries to delete the herbarium in delete_own_records, which executes

  Herbarium.where(personal_id: <user_id>).delete_all

B. But before doing that, erase_user does blank_out_public_references which executes

 Herbarium.where(personal_id: <user_id>).update_all(personal_id: 0)

So when erase_user gets to step A, there is no herbarium with personal_id: .

I think the fix is to omit step B. (There's no need to blank out the reference, as the herbarium will be destroyed.) And there should be a test.

— Reply to this email directly, view it on GitHub https://github.com/MushroomObserver/mushroom-observer/issues/2082, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYTNNNV45R3C6LBJDBX5DTY3BIVJAVCNFSM6AAAAABFQTU5ZKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGIYTOMJSGU4DAMY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

JoeCohen commented 3 months ago

@pellaea Thank you, Thank you.

  1. I don't have time to deal with those particular herbaria today. I obviously should check for references to them.
  2. You're right about the ALERT means that herbarium.personal_id cannot be zero. And I need to understand how delete_all works. And double-check what I said about erase_user.