Bug #6880
closed
[Workbench] Remove delete functionality from Users table
Added by Ward Vandewege over 9 years ago.
Updated over 9 years ago.
Assigned To:
Radhika Chippada
Estimated time:
(Total: 0.00 h)
Description
Story¶
Removing users from Arvados is impractical. Workbench currently offers a delete ("trash") button in the users table, but it basically never works, because the API server refuses the request. Remove this entire column of buttons from the users index in Workbench.
Original bug report¶
There's a delete icon next to each user on the admin users page. Clicking it does nothing, because the ajax call fails (without reporting an error to the user):
#<ActiveRecord::DeleteRestrictionError: Cannot delete record because of dependent logs>
/var/www/arvados-api/shared/vendor_bundle/ruby/2.1.0/gems/activerecord-3.2.17/lib/active_record/associations/builder/has_many.rb:63:in `block in define_restrict_dependency_method'
/var/www/arvados-api/shared/vendor_bundle/ruby/2.1.0/gems/activesupport-3.2.17/lib/active_support/callbacks.rb:506:in `_run__3002657973961559811__destroy__2941283170573855482__callbacks'
/var/www/arvados-api/shared/vendor_bundle/ruby/2.1.0/gems/activesupport-3.2.17/lib/active_support/callbacks.rb:405:in `__run_callback'
/var/www/arvados-api/shared/vendor_bundle/ruby/2.1.0/gems/activesupport-3.2.17/lib/active_support/callbacks.rb:385:in `_run_destroy_callbacks'
/var/www/arvados-api/shared/vendor_bundle/ruby/2.1.0/gems/activesupport-3.2.17/lib/active_support/callbacks.rb:81:in `run_callbacks'
/var/www/arvados-api/shared/vendor_bundle/ruby/2.1.0/gems/activerecord-3.2.17/lib/active_record/callbacks.rb:254:in `destroy'
/var/www/arvados-api/shared/vendor_bundle/ruby/2.1.0/gems/activerecord-3.2.17/lib/active_record/transactions.rb:254:in `block in destroy'
/var/www/arvados-api/shared/vendor_bundle/ruby/2.1.0/gems/activerecord-3.2.17/lib/active_record/transactions.rb:313:in `block in with_transaction_returning_status'
/var/www/arvados-api/shared/vendor_bundle/ruby/2.1.0/gems/activerecord-3.2.17/lib/active_record/connection_adapters/abstract/database_statements.rb:192:in `transaction'
/var/www/arvados-api/shared/vendor_bundle/ruby/2.1.0/gems/activerecord-3.2.17/lib/active_record/transactions.rb:208:in `transaction'
/var/www/arvados-api/shared/vendor_bundle/ruby/2.1.0/gems/activerecord-3.2.17/lib/active_record/transactions.rb:311:in `with_transaction_returning_status'
/var/www/arvados-api/shared/vendor_bundle/ruby/2.1.0/gems/activerecord-3.2.17/lib/active_record/transactions.rb:254:in `destroy'
/var/www/arvados-api/current/app/controllers/application_controller.rb:133:in `destroy'
- Description updated (diff)
- Description updated (diff)
So what do you want?
- The Workbench delete interface gets removed
- The Workbench delete interface reports the error clearly
- The Workbench delete interface does more work to make the delete succeed (this might be difficult or even impossible, haven't dug into it)
- The API server does more work to make the delete succeed
- We change the semantics of API deletes (and a "deleted" attribute to users or whatever)
When I originally identified this issue #3628, Tom suggested that we should not provide the delete button in the users page.
- Subject changed from [API] deleting users from the admin interface is broken to [Workbench] Remove delete functionality from Users table
- Description updated (diff)
- Category set to Workbench
- Target version changed from Bug Triage to 2015-08-19 sprint
- Story points set to 0.5
Adjusting per Radhika's comment.
- Assigned To set to Radhika Chippada
- Status changed from New to In Progress
While testing this very simple "remove Delete button" implementation, I noticed that the "Home" link is shown on "all" pages in admin settings. Filed #6916.
Fixing this also as part of this issue. And adding controller tests, which should have been written when #6234 was initially implemented.
Tom said in IRC:
11:14 radhika: we really need to kick the "writing logic and local variables in views" habit. This is how copy-and-paste starts. Perhaps we can have a "deletable?" instance method (defaulting to calling editable?, and override in user to return false), and call that from the delete_object_button partial? Then the generic view can go back to being generic.
Branch 6880-remove-user-delete-button at 467b636f7d1b34f7695f55af972ae90132fc8063
- Contains fix for this issue as well as 6916. The tests and the logic were too close and hence was productive to in one stretch
- Added a deletable? to arvados_base.rb per Tom's suggestion and used it in delete_object_button partial
- While at it, also created a show_home_button partial and moved the logic to show the Home button into it
I think this
if object.editable? and object.deletable?
should just be
if object.deletable?
(if a model someday claims that it is deletable but not editable, then it would be appropriate for the view to offer a "delete" button)
The "=false" part of expect_home_link=false and expect_delete_link=false in the tests seems to be unused (which is a good thing) so could be dropped.
With those, LGTM, thanks!
- Status changed from In Progress to Resolved
- % Done changed from 0 to 100
Applied in changeset arvados|commit:427d9052d59ca7819acba9fb2e5f381d3e44a53e.
Also available in: Atom
PDF
Updated by 
Updated by 
Updated by Radhika Chippada 
Updated by