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.

Sign up for GitHub

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

Jump to bottom

Todos linked to projects aren't deleted when user is deleted #1447

Closed
dnrce opened this issue Jun 25, 2014 · 9 comments
Closed

Todos linked to projects aren't deleted when user is deleted #1447

dnrce opened this issue Jun 25, 2014 · 9 comments
Labels
Database Problems related to the database
Milestone
2.3.0

Comments

@dnrce
Copy link
Member

dnrce commented Jun 25, 2014

Migrated from the original issue at https://www.assembla.com/spaces/tracks-tickets/tickets/1447

When a project is deleted using an action such as UsersController::destroy, the user instance gets destroyed and propagates deletion to related projects. However, the todos of this project do not get deleted as there is a :delete_all dependency between users and projects.

  • Ivan

Originally reported by Bocete on September 9, 2013 at 15:10:29 (-0700) against version 2.0
@dnrce dnrce added this to the 2.2.3 milestone Jun 25, 2014
@dnrce
Copy link
Member Author

dnrce commented Jun 25, 2014

On September 9, 2013 at 01:14:29 (-0700), lrbalt commented:

Is it still present in 2.2?

@dnrce
Copy link
Member Author

dnrce commented Jun 25, 2014

On September 9, 2013 at 10:45:41 (-0700), Bocete commented:

Looks like it. Users -> :delete_all Projects, and Projects -> :delete_all Todos, but when a User is destroyed :delete_all does not propagate further than the deleted user's projects. The dependency between Users and Projects would have to be :destroy_all, but I'm not suggesting that as a fix as that may break other things (possibly deleting too eagerly)

@dnrce
Copy link
Member Author

dnrce commented Jun 25, 2014

On September 9, 2013 at 00:31:19 (-0700), lrbalt commented:

true. Need to investigate why we reverted to delete_all from destroy_all. Perhaps performance was an issue, but deleting a user is not really done often IMHO.

Ref: http://stackoverflow.com/questions/2797339/rails-dependent-destroy-vs-dependent-delete-all

@dnrce dnrce changed the title Todos without projects Todos linked to projects aren't deleted when user is deleted Jul 1, 2014
@dnrce
Copy link
Member Author

dnrce commented Jul 12, 2014

@lrbalt I went to have a look at the history and it has been :delete_all since being migrated to git seven years ago. Do you have a copy of the old SVN repository available to find the actual change?

https://github.com/TracksApp/tracks/blame/0bc9b0397a144a10c99486f084519d2d9c946f22/app/models/user.rb#L30

@lrbalt
Copy link
Member

lrbalt commented Jul 12, 2014

The svn history was migrated to github too. So I guess we do not have the rationale behind the :delete_all versus :destroy. We probably need to test the effect of the change. I do not think we have unit tests that check if deleting a user cleans up all stuff of that user.

@dnrce
Copy link
Member Author

dnrce commented Jul 12, 2014

Ah, I didn't go far enough back. The file was renamed. 26c7a1e shows that it has always been :delete_all.

@dnrce
Copy link
Member Author

dnrce commented Jul 16, 2014

Related to #1400

@dnrce
Copy link
Member Author

dnrce commented Sep 8, 2014

This is too deep a change for a patch release. Moving to 2.3.

@dnrce dnrce modified the milestones: 2.3, 2.2.3 Sep 8, 2014
@lrbalt
Copy link
Member

lrbalt commented Sep 23, 2014

I have looked into this and I find this in my log if I delete a user:

DELETE FROM `contexts` WHERE `contexts`.`user_id` = 1
DELETE FROM `projects` WHERE `projects`.`user_id` = 1
DELETE FROM `todos` WHERE `todos`.`user_id` = 1
DELETE FROM `recurring_todos` WHERE `recurring_todos`.`user_id` = 1
DELETE FROM `notes` WHERE `notes`.`user_id` = 1
SELECT  `preferences`.* FROM `preferences`  WHERE `preferences`.`user_id` = 1 LIMIT 1
DELETE FROM `preferences` WHERE `preferences`.`id` = 1
DELETE FROM `users` WHERE `users`.`id` = 1

AFAICS, deleting a project does not delete all todos in that project (otherwise there should be a DELETE FROM 'todos' WHERE project_id = xx in there). But this is handled by one big DELETE on user_id in line 3

This is probably why delete_all was chosen over destroy, so that both deletes do not bother each other.

I conclude that deleting a user will delete all his/her stuff, so this ticket is not valid. I'll add a test and close this one.

@lrbalt lrbalt closed this as completed in f8d4f85 Sep 23, 2014
errinlarsen added a commit to errinlarsen/tracks that referenced this issue Dec 24, 2014
@errinlarsen
Merge branch 'upgrade_irkeninvader.net' into irkeninvader.net
444fdc5 
* upgrade_irkeninvader.net: (570 commits)
  [v2.3.rc1] add postgresql & bundle install
  [v2.3.rc1] add vendor/bundle to .gitignore
  Version 2.3.rc1
  Add pessimistic version constraints for all gems
  Use badges targeting 2.3_branch
  Fix links on same line
  Specify pages for MkDocs
  Minor changelog polish
  Convert changelog to Markdown
  Recommend upgrading from 2.2.3
  Pull changelog entry from Tracks 2.2.3
  Remove metadata from top of changelog
  Update rspec-expectations gem
  Update capybara gem
  Update codeclimate-test-reporter gem
  Use latest Tolk from Rubygems
  fix TracksApp#1400 where deleting a user will clean up tags and dependencies too
  add test for deleting all stuff of a user when this users is deleted. Fixes TracksApp#1447
  create secrets.yml per rails upgrade instructions. It uses the secret key from site.yml
  make drag_action_title unambiguous
  ...

Resolved Conflicts:
	.gitignore
	.travis.yml
	Gemfile
	Gemfile.lock
	README.md
	app/assets/javascripts/tracks.js.erb
	app/controllers/contexts_controller.rb
	app/controllers/data_controller.rb
	app/controllers/integrations_controller.rb
	app/controllers/projects_controller.rb
	app/controllers/recurring_todos_controller.rb
	app/controllers/todos_controller.rb
	app/helpers/todos_helper.rb
	app/models/message_gateway.rb
	app/models/project.rb
	app/models/recurring_todo.rb
	app/models/todo.rb
	app/views/contexts/create.js.erb
	app/views/projects/update.js.erb
	app/views/projects/update_project_name.js.erb
	app/views/todos/_successor.html.erb
	app/views/todos/create.js.erb
	app/views/todos/update.js.erb
	config/application.rb
	config/initializers/secret_token.rb
	config/initializers/tracks.rb
	config/locales/es.yml
	config/locales/fr.yml
	config/routes.rb
	config/site.yml.tmpl
	db/tracks-blank.sqlite3.db
	db/tracks-example.sqlite3.db
	db/tracks-test.sqlite3.db
	doc/CHANGELOG.md
	features/context_list.feature
	features/manage_users.feature
	features/step_definitions/context_list_steps.rb
	features/step_definitions/project_list_steps.rb
	features/step_definitions/project_steps.rb
	features/step_definitions/todo_edit_steps.rb
	features/step_definitions/todo_steps.rb
	features/support/world.rb
	features/tickler.feature
	lib/tracks/done_todos.rb
	test/controllers/stats_controller_test.rb
	test/controllers/todos_controller_test.rb
	test/functional/recurring_todos_controller_test.rb
	test/models/project_test.rb
	test/unit/tagging_test.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Database Problems related to the database
Projects
None yet
Milestone
2.3.0
Development

No branches or pull requests
2 participants
@lrbalt @dnrce