Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deleting a show can leave orphaned people #677

Open
CHTJonas opened this issue Jun 7, 2019 · 3 comments

Comments

Projects
None yet
2 participants
@CHTJonas
Copy link
Member

commented Jun 7, 2019

Under certain circumstances, deleting a show can leave behind orphaned people records that don't have any shows listed in their profile pages.

Steps to reproduce:

  1. Create a new show
  2. Add a new person (eg. with a silly name)
  3. Open link to said person in a new tab
  4. Delete show
  5. Refresh person page

To this end I have added 0 3 * * * /usr/local/bin/camdram-console camdram:people:remove-orphaned --env=prod > /dev/null to the camdram user's crontab.

@CHTJonas CHTJonas added the bug label Jun 7, 2019

@philosophicles

This comment has been minimized.

Copy link
Member

commented Jun 15, 2019

Is this crontab addition a sufficient fix, @CHTJonas, or were you envisioning additional changes in this codebase to trigger clean up of orphan entities when the show is deleted?

Amusingly it looks like we meant to add this crontab entry as part of #195 but looks like it fell in the cracks :)

@CHTJonas

This comment has been minimized.

Copy link
Member Author

commented Jun 16, 2019

I think that the remove-orphaned console task is a somewhat blunt albeit extremely useful instrument - that is to say it solves the problem without really fixing the source. Ideally what we want is some kind of callback that gets triggered when a show is deleted and that examines each person in the show and deletes individuals who are orphans there and then.

It's not a huge issue and I haven't looked at the code but I imagine that console task essentially scans an entire table in the database and enumerates the person-show relationship which probably isn't great efficiency-wise. Then again it's happening at 3am so...

@philosophicles

This comment has been minimized.

Copy link
Member

commented Jun 20, 2019

Just for reference:

src/Acts/CamdramAdminBundle/Command/PeopleRemoveOrphanedCommand.php

It's doing it all through the ORM but it looks like it's getting the list of orphans pretty efficiently (simple left-join SQL query). However the resolution is then done row-by-agonizing-row in a loop; it's sometimes a delete and sometimes an update. This probably could be optimized into just two slightly more complex set-based SQL queries (an update and a delete), with no separate select query beforehand and no php loop, if desired. That would be super quick I think.

But that all said, completely agree that event based action when a show is deleted is probably best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.