[ARVADOS] updated: 00f628861c69a1faef520b651b2f733d3981672d
git at public.curoverse.com
git at public.curoverse.com
Thu Oct 16 12:01:12 EDT 2014
Summary of changes:
.../app/controllers/application_controller.rb | 9 -------
apps/workbench/app/helpers/application_helper.rb | 4 +--
.../app/models/api_client_authorization.rb | 4 +--
apps/workbench/app/models/arvados_base.rb | 9 ++++++-
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/application_controller_test.rb | 29 --------------------
apps/workbench/test/test_helper.rb | 12 ++++++---
apps/workbench/test/unit/collection_test.rb | 31 ++++++++++++++++++++++
apps/workbench/test/unit/job_test.rb | 30 ++++++++++++++++++---
apps/workbench/test/unit/pipeline_instance_test.rb | 30 ++++++++++++++++++---
services/api/test/fixtures/jobs.yml | 10 +++++++
services/api/test/fixtures/pipeline_instances.yml | 6 +++++
17 files changed, 141 insertions(+), 76 deletions(-)
via 00f628861c69a1faef520b651b2f733d3981672d (commit)
from c422cbe72db43dc133a45cc8d7da6ea0f87967ca (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 00f628861c69a1faef520b651b2f733d3981672d
Author: Brett Smith <brett at curoverse.com>
Date: Thu Oct 16 12:03:10 2014 -0400
4189: Break the last commit.
This tries to use #attribute_editable? more consistently as Tom
suggested in feedback. The tests don't pass. See ticket for
discussion.
diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index a454822..4f5d8fd 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -337,15 +337,6 @@ class ApplicationController < ActionController::Base
Thread.current[:user]
end
- helper_method :object_attribute_editable?
- def object_attribute_editable?(object, attr)
- object and object.attribute_editable?(attr, :ever) and
- (object.editable? or
- (object.owner_uuid == current_user.uuid) or
- (Group.find(object.owner_uuid).writable_by
- .include?(current_user.uuid) rescue false))
- end
-
def model_class
controller_name.classify.constantize
end
diff --git a/apps/workbench/app/helpers/application_helper.rb b/apps/workbench/app/helpers/application_helper.rb
index d7578ca..66b7ed6 100644
--- a/apps/workbench/app/helpers/application_helper.rb
+++ b/apps/workbench/app/helpers/application_helper.rb
@@ -150,7 +150,7 @@ module ApplicationHelper
def render_editable_attribute(object, attr, attrvalue=nil, htmloptions={})
attrvalue = object.send(attr) if attrvalue.nil?
- if not object_attribute_editable?(object, attr)
+ if not object.attribute_editable?(attr)
if attrvalue && attrvalue.length > 0
return render_attribute_as_textile( object, attr, attrvalue, false )
else
@@ -239,7 +239,7 @@ module ApplicationHelper
preconfigured_search_str = value_info[:search_for]
end
- if not object_attribute_editable?(object, attr)
+ 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 e0e93b9..cbc65f3 100644
--- a/apps/workbench/app/models/arvados_base.rb
+++ b/apps/workbench/app/models/arvados_base.rb
@@ -332,8 +332,15 @@ class ArvadosBase < ActiveRecord::Base
(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/application_controller_test.rb b/apps/workbench/test/functional/application_controller_test.rb
index ec15518..c282802 100644
--- a/apps/workbench/test/functional/application_controller_test.rb
+++ b/apps/workbench/test/functional/application_controller_test.rb
@@ -321,33 +321,4 @@ class ApplicationControllerTest < ActionController::TestCase
Rails.configuration.arvados_v1_base = orig_api_server
end
end
-
- def user_can_edit_collection_name?(token_sym, collection_key)
- use_token token_sym
- coll = Collection.find(api_fixture("collections")[collection_key]["uuid"])
- @controller.object_attribute_editable?(coll, "name")
- end
-
- test "admin can edit object attributes" do
- assert(user_can_edit_collection_name?(:admin, "foo_file"),
- "admin not allowed to edit collection name")
- end
-
- test "project owner can edit object attributes" do
- assert(user_can_edit_collection_name?(:active,
- "foo_collection_in_aproject"),
- "project owner not allowed to edit collection name")
- end
-
- test "project admin can edit object attributes" do
- assert(user_can_edit_collection_name?(:subproject_admin,
- "baz_file_in_asubproject"),
- "project admin not allowed to edit collection name")
- end
-
- test "project viewer cannot edit object attributes" do
- refute(user_can_edit_collection_name?(:project_viewer,
- "foo_collection_in_aproject"),
- "project viewer allowed to edit collection name")
- end
end
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/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/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/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
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list