[ARVADOS] updated: 69e4d608045afea775b77d1f41afddf0bdfcf5c6

git at public.curoverse.com git at public.curoverse.com
Thu Oct 16 17:22:12 EDT 2014


Summary of changes:
 apps/workbench/app/helpers/application_helper.rb   |  9 ++-----
 .../app/models/api_client_authorization.rb         |  4 +--
 apps/workbench/app/models/arvados_base.rb          | 13 +++++++--
 apps/workbench/app/models/authorized_key.rb        |  6 ++---
 apps/workbench/app/models/collection.rb            |  8 ++----
 apps/workbench/app/models/job.rb                   | 10 +++----
 apps/workbench/app/models/pipeline_instance.rb     | 10 ++++---
 apps/workbench/app/models/user.rb                  |  5 ++--
 apps/workbench/app/models/virtual_machine.rb       |  4 +--
 .../test/functional/projects_controller_test.rb    | 17 ++++++++++++
 apps/workbench/test/test_helper.rb                 | 14 +++++++---
 apps/workbench/test/unit/collection_test.rb        | 31 ++++++++++++++++++++++
 apps/workbench/test/unit/group_test.rb             | 12 +++++++++
 apps/workbench/test/unit/job_test.rb               | 30 ++++++++++++++++++---
 apps/workbench/test/unit/pipeline_instance_test.rb | 30 ++++++++++++++++++---
 doc/api/schema/Group.html.textile.liquid           |  1 +
 doc/api/schema/User.html.textile.liquid            |  1 +
 services/api/app/models/arvados_model.rb           |  3 ++-
 services/api/app/models/user.rb                    |  1 +
 .../test/fixtures/api_client_authorizations.yml    |  6 +++++
 services/api/test/fixtures/collections.yml         |  9 +++++++
 services/api/test/fixtures/jobs.yml                | 10 +++++++
 services/api/test/fixtures/links.yml               | 14 ++++++++++
 services/api/test/fixtures/pipeline_instances.yml  |  6 +++++
 services/api/test/fixtures/users.yml               | 14 ++++++++++
 .../functional/arvados/v1/users_controller_test.rb | 10 +++++++
 26 files changed, 233 insertions(+), 45 deletions(-)

       via  69e4d608045afea775b77d1f41afddf0bdfcf5c6 (commit)
       via  d7fdbbab1f67d696906afe7a6e17e17997e1c064 (commit)
       via  d825508721cb4e184d03d43965cee00caee21bd6 (commit)
       via  d532da29a2f954f6a24ccacd1142e61b299ea292 (commit)
       via  75fd0cb8bff8f0396168a8900eaba9d8e1edd65e (commit)
       via  84e64f6e81c241a6dbaabea3dcafa2bad4cb188a (commit)
       via  bc4c747ce6b4cda9928cd318fd050c1171bff475 (commit)
      from  ac4f4ab5c6d677096cc335af7cfc0d9b10043b93 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 69e4d608045afea775b77d1f41afddf0bdfcf5c6
Merge: ac4f4ab d7fdbba
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Oct 16 17:23:57 2014 -0400

    Merge branch '4189-workbench-project-admin-attr-editing-wip'
    
    Closes #4189, #4202.


commit d7fdbbab1f67d696906afe7a6e17e17997e1c064
Author: Brett Smith <brett at curoverse.com>
Date:   Wed Oct 15 14:04:12 2014 -0400

    4189: Workbench lets users with write permission rename objects in projects.
    
    The key part of this commit are the changes to ApplicationHelper.
    There's also some refactoring to more consistently answer questions
    about whether or not an attribute is editable.
    
    There are also some tests in here that I wrote while I was tracking
    down the problem.  They seem like a useful addition.

diff --git a/apps/workbench/app/helpers/application_helper.rb b/apps/workbench/app/helpers/application_helper.rb
index d343249..66b7ed6 100644
--- a/apps/workbench/app/helpers/application_helper.rb
+++ b/apps/workbench/app/helpers/application_helper.rb
@@ -150,9 +150,7 @@ module ApplicationHelper
 
   def render_editable_attribute(object, attr, attrvalue=nil, htmloptions={})
     attrvalue = object.send(attr) if attrvalue.nil?
-    if !object.attribute_editable?(attr, :ever) or
-        (!object.editable? and
-         !object.owner_uuid.in?(my_projects.collect(&:uuid)))
+    if not object.attribute_editable?(attr)
       if attrvalue && attrvalue.length > 0
         return render_attribute_as_textile( object, attr, attrvalue, false )
       else
@@ -241,10 +239,7 @@ module ApplicationHelper
       preconfigured_search_str = value_info[:search_for]
     end
 
-    if !object or
-        !object.attribute_editable?(attr, :ever) or
-        (!object.editable? and
-         !object.owner_uuid.in?(my_projects.collect(&:uuid)))
+    if not object.andand.attribute_editable?(attr)
       return link_to_if_arvados_object attrvalue
     end
 
diff --git a/apps/workbench/app/models/api_client_authorization.rb b/apps/workbench/app/models/api_client_authorization.rb
index ac3a9bf..6d1558c 100644
--- a/apps/workbench/app/models/api_client_authorization.rb
+++ b/apps/workbench/app/models/api_client_authorization.rb
@@ -1,6 +1,6 @@
 class ApiClientAuthorization < ArvadosBase
-  def attribute_editable? attr, *args
-    ['expires_at', 'default_owner_uuid'].index attr
+  def editable_attributes
+    %w(expires_at default_owner_uuid)
   end
   def self.creatable?
     false
diff --git a/apps/workbench/app/models/arvados_base.rb b/apps/workbench/app/models/arvados_base.rb
index 31a5b58..f5be0e1 100644
--- a/apps/workbench/app/models/arvados_base.rb
+++ b/apps/workbench/app/models/arvados_base.rb
@@ -334,8 +334,15 @@ class ArvadosBase < ActiveRecord::Base
        (ArvadosBase.find(owner_uuid).writable_by.include? current_user.uuid rescue false)))) or false
   end
 
+  # Array of strings that are the names of attributes that can be edited
+  # with X-Editable.
+  def editable_attributes
+    self.class.columns.map(&:name) -
+      %w(created_at modified_at modified_by_user_uuid modified_by_client_uuid updated_at)
+  end
+
   def attribute_editable?(attr, ever=nil)
-    if %w(created_at modified_at modified_by_user_uuid modified_by_client_uuid updated_at).include? attr.to_s
+    if not editable_attributes.include?(attr.to_s)
       false
     elsif not (current_user.andand.is_active)
       false
diff --git a/apps/workbench/app/models/authorized_key.rb b/apps/workbench/app/models/authorized_key.rb
index 724c996..2d804e1 100644
--- a/apps/workbench/app/models/authorized_key.rb
+++ b/apps/workbench/app/models/authorized_key.rb
@@ -1,7 +1,7 @@
 class AuthorizedKey < ArvadosBase
-  def attribute_editable? attr, *args
-    if attr.to_s == 'authorized_user_uuid'
-      current_user and current_user.is_admin
+  def attribute_editable?(attr, ever=nil)
+    if (attr.to_s == 'authorized_user_uuid') and (not ever)
+      current_user.andand.is_admin
     else
       super
     end
diff --git a/apps/workbench/app/models/collection.rb b/apps/workbench/app/models/collection.rb
index 87a083e..b5347dc 100644
--- a/apps/workbench/app/models/collection.rb
+++ b/apps/workbench/app/models/collection.rb
@@ -66,12 +66,8 @@ class Collection < ArvadosBase
     dir_to_tree.call('.')
   end
 
-  def attribute_editable? attr, *args
-    if %w(name description manifest_text).include? attr.to_s
-      true
-    else
-      super
-    end
+  def editable_attributes
+    %w(name description manifest_text)
   end
 
   def self.creatable?
diff --git a/apps/workbench/app/models/job.rb b/apps/workbench/app/models/job.rb
index 977eef9..c59bb89 100644
--- a/apps/workbench/app/models/job.rb
+++ b/apps/workbench/app/models/job.rb
@@ -7,12 +7,8 @@ class Job < ArvadosBase
     "#{script} job"
   end
 
-  def attribute_editable? attr, *args
-    if attr.to_sym == :description
-      super && attr.to_sym == :description
-    else
-      false
-    end
+  def editable_attributes
+    %w(description)
   end
 
   def self.creatable?
@@ -42,7 +38,7 @@ class Job < ArvadosBase
     arvados_api_client.api("jobs/", "queue_size", {"_method"=> "GET"})[:queue_size] rescue 0
   end
 
-  def self.queue 
+  def self.queue
     arvados_api_client.unpack_api_response arvados_api_client.api("jobs/", "queue", {"_method"=> "GET"})
   end
 
diff --git a/apps/workbench/app/models/pipeline_instance.rb b/apps/workbench/app/models/pipeline_instance.rb
index 9369057..83328b9 100644
--- a/apps/workbench/app/models/pipeline_instance.rb
+++ b/apps/workbench/app/models/pipeline_instance.rb
@@ -47,10 +47,12 @@ class PipelineInstance < ArvadosBase
     end
   end
 
-  def attribute_editable? attr, *args
-    super && (attr.to_sym == :name || attr.to_sym == :description ||
-              (attr.to_sym == :components and
-               (self.state == 'New' || self.state == 'Ready')))
+  def editable_attributes
+    %w(name description components)
+  end
+
+  def attribute_editable?(name, ever=nil)
+    (ever or %w(New Ready).include?(state)) and super
   end
 
   def attributes_for_display
diff --git a/apps/workbench/app/models/user.rb b/apps/workbench/app/models/user.rb
index 967ea2a..7aaa4fe 100644
--- a/apps/workbench/app/models/user.rb
+++ b/apps/workbench/app/models/user.rb
@@ -35,8 +35,9 @@ class User < ArvadosBase
     super.reject { |k,v| %w(owner_uuid default_owner_uuid identity_url prefs).index k }
   end
 
-  def attribute_editable? attr, *args
-    (not (self.uuid.andand.match(/000000000000000$/) and self.is_admin)) and super
+  def attribute_editable?(attr, ever=nil)
+    (ever or not (self.uuid.andand.match(/000000000000000$/) and
+                  self.is_admin)) and super
   end
 
   def friendly_link_name lookup=nil
diff --git a/apps/workbench/app/models/virtual_machine.rb b/apps/workbench/app/models/virtual_machine.rb
index 083aae3..3b44397 100644
--- a/apps/workbench/app/models/virtual_machine.rb
+++ b/apps/workbench/app/models/virtual_machine.rb
@@ -6,8 +6,8 @@ class VirtualMachine < ArvadosBase
   def attributes_for_display
     super.append ['current_user_logins', @current_user_logins]
   end
-  def attribute_editable? attr, *args
-    attr != 'current_user_logins' and super
+  def editable_attributes
+    super - %w(current_user_logins)
   end
   def self.attribute_info
     merger = ->(k,a,b) { a.merge(b, &merger) }
diff --git a/apps/workbench/test/functional/projects_controller_test.rb b/apps/workbench/test/functional/projects_controller_test.rb
index d76430c..8eb0cdc 100644
--- a/apps/workbench/test/functional/projects_controller_test.rb
+++ b/apps/workbench/test/functional/projects_controller_test.rb
@@ -93,6 +93,23 @@ class ProjectsControllerTest < ActionController::TestCase
     refute user_can_manage(:project_viewer, "asubproject")
   end
 
+  test "subproject_admin can_manage asubproject" do
+    assert user_can_manage(:subproject_admin, "asubproject")
+  end
+
+  test "project admin can remove items from the project" do
+    coll_key = "collection_to_remove_from_subproject"
+    coll_uuid = api_fixture("collections")[coll_key]["uuid"]
+    delete(:remove_item,
+           { id: api_fixture("groups")["asubproject"]["uuid"],
+             item_uuid: coll_uuid,
+             format: "js" },
+           session_for(:subproject_admin))
+    assert_response :success
+    assert_match(/\b#{coll_uuid}\b/, @response.body,
+                 "removed object not named in response")
+  end
+
   test 'projects#show tab infinite scroll partial obeys limit' do
     get_contents_rows(limit: 1, filters: [['uuid','is_a',['arvados#job']]])
     assert_response :success
diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb
index f99144a..01a79bb 100644
--- a/apps/workbench/test/test_helper.rb
+++ b/apps/workbench/test/test_helper.rb
@@ -57,7 +57,7 @@ module ApiFixtureLoader
 
   module ClassMethods
     @@api_fixtures = {}
-    def api_fixture(name)
+    def api_fixture(name, *keys)
       # Returns the data structure from the named API server test fixture.
       @@api_fixtures[name] ||= \
       begin
@@ -65,10 +65,16 @@ module ApiFixtureLoader
                          'test', 'fixtures', "#{name}.yml")
         YAML.load(IO.read(path))
       end
+      keys.inject(@@api_fixtures[name]) { |hash, key| hash[key] }
     end
   end
-  def api_fixture name
-    self.class.api_fixture name
+  def api_fixture(name, *keys)
+    self.class.api_fixture(name, *keys)
+  end
+
+  def find_fixture(object_class, name)
+    object_class.find(api_fixture(object_class.to_s.pluralize.underscore,
+                                  name, "uuid"))
   end
 end
 
diff --git a/apps/workbench/test/unit/collection_test.rb b/apps/workbench/test/unit/collection_test.rb
index 512ad47..e71f966 100644
--- a/apps/workbench/test/unit/collection_test.rb
+++ b/apps/workbench/test/unit/collection_test.rb
@@ -40,4 +40,35 @@ class CollectionTest < ActiveSupport::TestCase
                  get_files_tree('multilevel_collection_2'),
                  "Collection file tree was malformed")
   end
+
+  test "portable_data_hash never editable" do
+    refute(Collection.new.attribute_editable?("portable_data_hash", :ever))
+  end
+
+  test "admin can edit name" do
+    use_token :admin
+    assert(find_fixture(Collection, "foo_file").attribute_editable?("name"),
+           "admin not allowed to edit collection name")
+  end
+
+  test "project owner can edit name" do
+    use_token :active
+    assert(find_fixture(Collection, "foo_collection_in_aproject")
+             .attribute_editable?("name"),
+           "project owner not allowed to edit collection name")
+  end
+
+  test "project admin can edit name" do
+    use_token :subproject_admin
+    assert(find_fixture(Collection, "baz_file_in_asubproject")
+             .attribute_editable?("name"),
+           "project admin not allowed to edit collection name")
+  end
+
+  test "project viewer cannot edit name" do
+    use_token :project_viewer
+    refute(find_fixture(Collection, "foo_collection_in_aproject")
+             .attribute_editable?("name"),
+           "project viewer allowed to edit collection name")
+  end
 end
diff --git a/apps/workbench/test/unit/group_test.rb b/apps/workbench/test/unit/group_test.rb
index 3f5cebc..4a4530c 100644
--- a/apps/workbench/test/unit/group_test.rb
+++ b/apps/workbench/test/unit/group_test.rb
@@ -25,4 +25,16 @@ class GroupTest < ActiveSupport::TestCase
       assert_nil user.owner_uuid
     end
   end
+
+  test "project editable by its admin" do
+    use_token :subproject_admin
+    project = Group.find(api_fixture("groups")["asubproject"]["uuid"])
+    assert(project.editable?, "project not editable by admin")
+  end
+
+  test "project not editable by reader" do
+    use_token :project_viewer
+    project = Group.find(api_fixture("groups")["aproject"]["uuid"])
+    refute(project.editable?, "project editable by reader")
+  end
 end
diff --git a/apps/workbench/test/unit/job_test.rb b/apps/workbench/test/unit/job_test.rb
index 5079316..add4c0f 100644
--- a/apps/workbench/test/unit/job_test.rb
+++ b/apps/workbench/test/unit/job_test.rb
@@ -1,7 +1,31 @@
 require 'test_helper'
 
 class JobTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  test "admin can edit description" do
+    use_token :admin
+    assert(find_fixture(Job, "job_in_subproject")
+             .attribute_editable?("description"),
+           "admin not allowed to edit job description")
+  end
+
+  test "project owner can edit description" do
+    use_token :active
+    assert(find_fixture(Job, "job_in_subproject")
+             .attribute_editable?("description"),
+           "project owner not allowed to edit job description")
+  end
+
+  test "project admin can edit description" do
+    use_token :subproject_admin
+    assert(find_fixture(Job, "job_in_subproject")
+             .attribute_editable?("description"),
+           "project admin not allowed to edit job description")
+  end
+
+  test "project viewer cannot edit description" do
+    use_token :project_viewer
+    refute(find_fixture(Job, "job_in_subproject")
+             .attribute_editable?("description"),
+           "project viewer allowed to edit job description")
+  end
 end
diff --git a/apps/workbench/test/unit/pipeline_instance_test.rb b/apps/workbench/test/unit/pipeline_instance_test.rb
index 9b4c7c3..95ad8fa 100644
--- a/apps/workbench/test/unit/pipeline_instance_test.rb
+++ b/apps/workbench/test/unit/pipeline_instance_test.rb
@@ -1,7 +1,31 @@
 require 'test_helper'
 
 class PipelineInstanceTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  test "admin can edit name" do
+    use_token :admin
+    assert(find_fixture(PipelineInstance, "new_pipeline_in_subproject")
+             .attribute_editable?("name"),
+           "admin not allowed to edit pipeline instance name")
+  end
+
+  test "project owner can edit name" do
+    use_token :active
+    assert(find_fixture(PipelineInstance, "new_pipeline_in_subproject")
+             .attribute_editable?("name"),
+           "project owner not allowed to edit pipeline instance name")
+  end
+
+  test "project admin can edit name" do
+    use_token :subproject_admin
+    assert(find_fixture(PipelineInstance, "new_pipeline_in_subproject")
+             .attribute_editable?("name"),
+           "project admin not allowed to edit pipeline instance name")
+  end
+
+  test "project viewer cannot edit name" do
+    use_token :project_viewer
+    refute(find_fixture(PipelineInstance, "new_pipeline_in_subproject")
+             .attribute_editable?("name"),
+           "project viewer allowed to edit pipeline instance name")
+  end
 end
diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index 32560c3..8f7b125 100644
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -49,6 +49,12 @@ project_viewer:
   api_token: projectviewertoken1234567890abcdefghijklmnopqrstuv
   expires_at: 2038-01-01 00:00:00
 
+subproject_admin:
+  api_client: untrusted
+  user: subproject_admin
+  api_token: subprojectadmintoken1234567890abcdefghijklmnopqrst
+  expires_at: 2038-01-01 00:00:00
+
 admin_vm:
   api_client: untrusted
   user: admin
diff --git a/services/api/test/fixtures/collections.yml b/services/api/test/fixtures/collections.yml
index 2fb235c..e1e568a 100644
--- a/services/api/test/fixtures/collections.yml
+++ b/services/api/test/fixtures/collections.yml
@@ -257,3 +257,12 @@ collection_owned_by_foo:
   owner_uuid: zzzzz-tpzed-81hsbo6mk8nl05c
   created_at: 2014-02-03T17:22:54Z
   name: collection_owned_by_foo
+
+collection_to_remove_from_subproject:
+  # The Workbench tests remove this from subproject.
+  uuid: zzzzz-4zz18-subprojgonecoll
+  portable_data_hash: 2386ca6e3fffd4be5e197a72c6c80fb2+51
+  manifest_text: ". 8258b505536a9ab47baa2f4281cb932a+9 0:9:missingno\n"
+  owner_uuid: zzzzz-j7d0g-axqo7eu9pwvna1x
+  created_at: 2014-10-15T10:45:00
+  name: Collection to remove from subproject
diff --git a/services/api/test/fixtures/jobs.yml b/services/api/test/fixtures/jobs.yml
index 5836809..5e383ed 100644
--- a/services/api/test/fixtures/jobs.yml
+++ b/services/api/test/fixtures/jobs.yml
@@ -284,3 +284,13 @@ cancelled:
     done: 1
   runtime_constraints: {}
   state: Cancelled
+
+job_in_subproject:
+  uuid: zzzzz-8i9sb-subprojectjob01
+  created_at: 2014-10-15 12:00:00
+  owner_uuid: zzzzz-j7d0g-axqo7eu9pwvna1x
+  log: ~
+  repository: foo
+  script: hash
+  script_version: 4fe459abe02d9b365932b8f5dc419439ab4e2577
+  state: Complete
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 9e13971..1771fcc 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -377,6 +377,20 @@ project_viewer_can_read_project:
   head_uuid: zzzzz-j7d0g-v955i6s2oi1cbso
   properties: {}
 
+subproject_admin_can_manage_subproject:
+  uuid: zzzzz-o0j2j-subprojadminlnk
+  owner_uuid: zzzzz-tpzed-000000000000000
+  created_at: 2014-10-15 10:00:00 -0000
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  modified_at: 2014-10-15 10:00:00 -0000
+  updated_at: 2014-10-15 10:00:00 -0000
+  tail_uuid: zzzzz-tpzed-subprojectadmin
+  link_class: permission
+  name: can_manage
+  head_uuid: zzzzz-j7d0g-axqo7eu9pwvna1x
+  properties: {}
+
 foo_collection_tag:
   uuid: zzzzz-o0j2j-eedahfaho8aphiv
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
diff --git a/services/api/test/fixtures/pipeline_instances.yml b/services/api/test/fixtures/pipeline_instances.yml
index a3d372b..2fc28ec 100644
--- a/services/api/test/fixtures/pipeline_instances.yml
+++ b/services/api/test/fixtures/pipeline_instances.yml
@@ -4,6 +4,12 @@ new_pipeline:
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
   created_at: <%= 1.minute.ago.to_s(:db) %>
 
+new_pipeline_in_subproject:
+  state: New
+  uuid: zzzzz-d1hrv-subprojpipeline
+  owner_uuid: zzzzz-j7d0g-axqo7eu9pwvna1x
+  created_at: <%= 1.minute.ago.to_s(:db) %>
+
 has_component_with_no_script_parameters:
   state: Ready
   uuid: zzzzz-d1hrv-1xfj6xkicf2muk2
diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index 2dd72ab..9cd2db8 100644
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -85,6 +85,20 @@ future_project_user:
       organization: example.com
       role: Computational biologist
 
+subproject_admin:
+  owner_uuid: zzzzz-tpzed-000000000000000
+  uuid: zzzzz-tpzed-subprojectadmin
+  email: subproject-admin at arvados.local
+  first_name: Subproject
+  last_name: Admin
+  identity_url: https://subproject-admin.openid.local
+  is_active: true
+  is_admin: false
+  prefs:
+    profile:
+      organization: example.com
+      role: Computational biologist
+
 spectator:
   owner_uuid: zzzzz-tpzed-000000000000000
   uuid: zzzzz-tpzed-l1s2piq4t4mps8r

commit d825508721cb4e184d03d43965cee00caee21bd6
Author: Brett Smith <brett at curoverse.com>
Date:   Wed Oct 15 13:29:48 2014 -0400

    4189: Workbench tests clear more state during teardown.
    
    I'm working on some new tests that pick up on stale state without
    these changes.

diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb
index 5253676..f99144a 100644
--- a/apps/workbench/test/test_helper.rb
+++ b/apps/workbench/test/test_helper.rb
@@ -38,7 +38,9 @@ class ActiveSupport::TestCase
 
   teardown do
     Thread.current[:arvados_api_token] = nil
+    Thread.current[:user] = nil
     Thread.current[:reader_tokens] = nil
+    Rails.cache.clear
     # Restore configuration settings changed during tests
     $application_config.each do |k,v|
       if k.match /^[^.]*$/

commit d532da29a2f954f6a24ccacd1142e61b299ea292
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Oct 16 17:00:39 2014 -0400

    4189: Document the API server's writable_by field.

diff --git a/doc/api/schema/Group.html.textile.liquid b/doc/api/schema/Group.html.textile.liquid
index 8a54265..2bf67eb 100644
--- a/doc/api/schema/Group.html.textile.liquid
+++ b/doc/api/schema/Group.html.textile.liquid
@@ -22,3 +22,4 @@ table(table table-bordered table-condensed).
 |group_class|string|Type of group. This does not affect behavior, but determines how the group is presented in the user interface. For example, @project@ indicates that the group should be displayed by Workbench and arv-mount as a project for organizing and naming objects.|@"project"@
 null|
 |description|text|||
+|writable_by|array|List of UUID strings identifying Users and other Groups that have write permission for this Group.  Only users who are allowed to administer the Group will receive a full list.  Other users will receive a partial list that includes the Group's owner_uuid and (if applicable) their own user UUID.||
diff --git a/doc/api/schema/User.html.textile.liquid b/doc/api/schema/User.html.textile.liquid
index c95a243..9a1b056 100644
--- a/doc/api/schema/User.html.textile.liquid
+++ b/doc/api/schema/User.html.textile.liquid
@@ -26,3 +26,4 @@ table(table table-bordered table-condensed).
 |prefs|hash|||
 |default_owner_uuid|string|||
 |is_active|boolean|||
+|writable_by|array|List of UUID strings identifying Groups and other Users that can modify this User object.  This will include the user's owner_uuid and, for administrators and users requesting their own User object, the requesting user's UUID.||

commit 75fd0cb8bff8f0396168a8900eaba9d8e1edd65e
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Oct 16 16:17:51 2014 -0400

    4189: Add writable_by to User API response, so writable_by is available for every owner_uuid.

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 422ce01..13ccd70 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -110,7 +110,8 @@ class ArvadosModel < ActiveRecord::Base
     unless (owner_uuid == current_user.uuid or
             current_user.is_admin or
             (current_user.groups_i_can(:manage) & [uuid, owner_uuid]).any?)
-      if (current_user.groups_i_can(:write) & [uuid, owner_uuid]).any?
+      if ((current_user.groups_i_can(:write) + [current_user.uuid]) &
+          [uuid, owner_uuid]).any?
         return [owner_uuid, current_user.uuid]
       else
         return [owner_uuid]
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 6e7facd..ecd50cc 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -29,6 +29,7 @@ class User < ArvadosModel
     t.add :is_admin
     t.add :is_invited
     t.add :prefs
+    t.add :writable_by
   end
 
   ALL_PERMISSIONS = {read: true, write: true, manage: true}
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index 9c4d18b..2d26370 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -779,6 +779,16 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     assert_equal false, found_email, 'Expected no email after updating profile'
   end
 
+  test "user API response includes writable_by" do
+    authorize_with :active
+    get :current
+    assert_response :success
+    assert_includes(json_response["writable_by"], users(:active).uuid,
+                    "user's writable_by should include self")
+    assert_includes(json_response["writable_by"], users(:active).owner_uuid,
+                    "user's writable_by should include its owner_uuid")
+  end
+
 
   NON_ADMIN_USER_DATA = ["uuid", "kind", "is_active", "email", "first_name",
                          "last_name"].sort

commit 84e64f6e81c241a6dbaabea3dcafa2bad4cb188a
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Oct 16 16:11:13 2014 -0400

    4189: Admit in writable_by that an object is writable when its owner_uuid is writable.

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 823fd55..422ce01 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -110,7 +110,7 @@ class ArvadosModel < ActiveRecord::Base
     unless (owner_uuid == current_user.uuid or
             current_user.is_admin or
             (current_user.groups_i_can(:manage) & [uuid, owner_uuid]).any?)
-      if current_user.groups_i_can(:write).index(uuid)
+      if (current_user.groups_i_can(:write) & [uuid, owner_uuid]).any?
         return [owner_uuid, current_user.uuid]
       else
         return [owner_uuid]

commit bc4c747ce6b4cda9928cd318fd050c1171bff475
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Oct 16 16:09:16 2014 -0400

    4189: Look up owner's writable_by when API did not provide writable_by for the object in question.

diff --git a/apps/workbench/app/models/arvados_base.rb b/apps/workbench/app/models/arvados_base.rb
index e0e93b9..31a5b58 100644
--- a/apps/workbench/app/models/arvados_base.rb
+++ b/apps/workbench/app/models/arvados_base.rb
@@ -329,7 +329,9 @@ class ArvadosBase < ActiveRecord::Base
      (current_user.is_admin or
       current_user.uuid == self.owner_uuid or
       new_record? or
-      (writable_by.include? current_user.uuid rescue false))) or false
+      (respond_to?(:writable_by) ?
+       writable_by.include?(current_user.uuid) :
+       (ArvadosBase.find(owner_uuid).writable_by.include? current_user.uuid rescue false)))) or false
   end
 
   def attribute_editable?(attr, ever=nil)

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list