Bug #5186

[Workbench] Collection viewing broken for collections with empty (but non-nil) properties attribute

Added by Ward Vandewege over 5 years ago. Updated over 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
-
Target version:
Start date:
02/15/2015
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
0.5

Description

I have a collection (su92l-4zz18-kf7cytopq8suzi0) with a properties attribute that has the value {}. I was able to set the properties attribute to {} with the Python SDK.

Viewing this collection leads to a fiddlesticks error:

Started GET "/collections/su92l-4zz18-kf7cytopq8suzi0" for 207.172.212.131 at 2015-02-11 01:01:51 +0000
Processing by CollectionsController#show as HTML
  Parameters: {"id"=>"su92l-4zz18-kf7cytopq8suzi0"}
  Rendered collections/_show_source_summary.html.erb (0.1ms)
  Rendered collections/_sharing_button.html.erb (0.6ms)
  Rendered application/_title_and_buttons.html.erb (1.7ms)
  Rendered application/show.html.erb (2.9ms)
  Rendered collections/show.html.erb within layouts/application (12.7ms)
#<TypeError: no implicit conversion of Symbol into Integer>
/var/www/workbench.su92l.arvadosapi.com/releases/20150211005436/app/views/application/_title_and_buttons.html.erb:3:in `[]'
/var/www/workbench.su92l.arvadosapi.com/releases/20150211005436/app/views/application/_title_and_buttons.html.erb:3:in `block in _app_views_application__title_and_buttons_html_erb__42221069618551823_60261440'
/var/www/workbench.su92l.arvadosapi.com/releases/20150211005436/vendor/bundle/ruby/2.1.0/gems/actionview-4.1.9/lib/action_view/helpers/capture_helper.rb:38:in `block in capture'
/var/www/workbench.su92l.arvadosapi.com/releases/20150211005436/vendor/bundle/ruby/2.1.0/gems/actionview-4.1.9/lib/action_view/helpers/capture_helper.rb:200:in `with_output_buffer'
/var/www/workbench.su92l.arvadosapi.com/releases/20150211005436/vendor/bundle/ruby/2.1.0/gems/actionview-4.1.9/lib/action_view/helpers/capture_helper.rb:38:in `capture'
/var/www/workbench.su92l.arvadosapi.com/releases/20150211005436/vendor/bundle/ruby/2.1.0/gems/actionview-4.1.9/lib/action_view/helpers/capture_helper.rb:152:in `content_for'
/var/www/workbench.su92l.arvadosapi.com/releases/20150211005436/app/views/application/_title_and_buttons.html.erb:2:in `_app_views_application__title_and_buttons_html_erb__42221069618551823_60261440'

This is line 3 in app/views/application/_title_and_buttons.html.erb:

    <%= (@object.respond_to?(:properties) and !@object.properties.nil? ? @object.properties[:page_title] : nil) ||

Subtasks

Task #5221: Review branch: 5186-collection-with-empty-propertiesResolvedRadhika Chippada

Associated revisions

Revision 0cefa4c0
Added by Radhika Chippada over 5 years ago

closes #5186
Merge branch '5186-collection-with-empty-properties'

History

#1 Updated by Ward Vandewege over 5 years ago

  • Description updated (diff)

#2 Updated by Radhika Chippada over 5 years ago

  • Status changed from New to In Progress
  • Assigned To set to Radhika Chippada
  • Target version changed from Bug Triage to 2015-02-18 sprint

#3 Updated by Radhika Chippada over 5 years ago

  • This error is happening because the properties attribute is not serialized as hash on the API server.
  • I updated collection model and controller on api server for this, similar to "prefs" attribute in "user"
  • My tests passed with or without "accept_attribute_as_json :properties, Hash" in controller though.

#4 Updated by Brett Smith over 5 years ago

At 7ef2780

Radhika Chippada wrote:

  • My tests passed with or without "accept_attribute_as_json :properties, Hash" in controller though.

That's because ApplicationController declares it for you. Declaring it again in CollectionsController is redundant. I suggest removing that.

I'm also not sure the new API server integration test in this branch gets us much. It seems like the new controller test gets us almost all the same benefit, and anything left could be obtained by a couple of get-only tests against the fixtures.

But I appreciate you being thorough with the tests in general, especially testing on both the API server and Workbench ends. Whether or not you remove the integration test, I'm happy to see this merged. Thanks.

#5 Updated by Radhika Chippada over 5 years ago

Thanks for the review feedback Brett. I removed the unnecessary statement from the collection_controller. I added a get test in controller tests and expanded the integration test to do an update instead of create with properties. Thanks.

#6 Updated by Radhika Chippada over 5 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:0cefa4c0f3c1b16884b04d6273bd8730166d69ba.

Also available in: Atom PDF