Hi,
I'm attaching what probably should have been a simple patch to show the username + a link for deployments. This also required making User a PermissionedObject, as well as making the DeploymentsController actually show Deployments in the first place. I didn't aim to fully finish the Deployments page, since it was outside the scope of this task. (You will note that none of the buttons up top work, for example.)
I found myself writing silly lines in views like:
= deployment.user.has_privilege(@current_user, Privilege::VIEW) ? link_to(deployment.user.name, admin_user_path(deployment.user)) : deployment.user.name
It also occurred to me that we're probably going to want to follow this pattern (link to an object if the user has permission to see it; just show its name or some other attribute in a table otherwise) elsewhere as we move forward, so I elected to create a conditional_link helper method.
Unfortunately, this required the ability to link_to an object without knowing its path, e.g., link_to(User.first). This works ordinarily, but because we use namespaces, it is not possible[1]. Since we don't duplicate routes within namespaces (there is not an /admin/users and a /resources/users), I wrote a fairly-ugly find_link_to(object) method to try to determine the appropriate path in support of the above.
I'm sending these as three separate patches in case the above changes are controversial. You'll need all three patches applied in order to test, however.
-- Matt
[1] http://stackoverflow.com/questions/731487/rails-correct-routing-for-namespac...
--- src/app/models/user.rb | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/app/models/user.rb b/src/app/models/user.rb index ead459a..25422eb 100644 --- a/src/app/models/user.rb +++ b/src/app/models/user.rb @@ -48,6 +48,7 @@
require 'sunspot_rails' class User < ActiveRecord::Base + include PermissionedObject non_solr_attributes = [:single_access_token, :last_request_at, :created_at, :quota_id, :crypted_password, :updated_at, :perishable_token, :failed_login_count, :current_login_ip, :password_salt, :current_login_at, :persistence_token, :login_count, :last_login_ip, :last_login_at] searchable :ignore_attribute_changes_of => non_solr_attributes do text :login, :as => :code_substring
Adds a conditional_link method to ApplicationHelper to link to an object only if @current_user has permission to view it. Also adds a requisite find_link_to method to emulate link_to(object) functionality lost when using namespaces in routes. --- src/app/helpers/application_helper.rb | 29 +++++++++++++++++++++++++++ src/spec/helpers/application_helper_spec.rb | 28 ++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 0 deletions(-)
diff --git a/src/app/helpers/application_helper.rb b/src/app/helpers/application_helper.rb index c536a81..c8e42bd 100644 --- a/src/app/helpers/application_helper.rb +++ b/src/app/helpers/application_helper.rb @@ -194,4 +194,33 @@ module ApplicationHelper title.split(' ').join('_').downcase end
+ # Link to an object only if it can be viewed by the current user + def conditional_link(attr, object) + if object.present? && object.has_privilege(@current_user, Privilege::VIEW) + link_to object.send(attr), find_link_to(object) + else + object.present? ? object.send(attr) : '' + end + end + + # Try to allow simple link_to even though we use namespaces in routes + def find_link_to(object) + ns_resources = ['Pool', 'Deployment', 'Instance'] + ns_image_factory = ['Assembly', 'Deployable', 'Template'] + ns_admin = ['HardwareProfile', 'Provider', 'User', 'ProviderAccount', 'Role', 'PoolFamily', 'Realm'] + + class_path = object.class.name.underscore + link = nil + if ns_resources.include?(object.class.name) + link = "resources_#{class_path}_path" + elsif ns_image_factory.include?(object.class.name) + link = "image_factory_#{class_path}_path" + elsif ns_admin.include?(object.class.name) + link = "admin_#{class_path}_path" + else + raise "No route for #{class_path}" + end + ActionController::Integration::Session.new.send(link, object) + end + end diff --git a/src/spec/helpers/application_helper_spec.rb b/src/spec/helpers/application_helper_spec.rb index fe5c956..947df05 100644 --- a/src/spec/helpers/application_helper_spec.rb +++ b/src/spec/helpers/application_helper_spec.rb @@ -34,4 +34,32 @@ describe ApplicationHelper do
end
+ context "find_link_to helper" do + it "has route for admin_users" do + find_link_to(Factory(:user)).match('^/admin/users/').should be_true + end + it "has route for resources_pool" do + find_link_to(Factory(:pool)).match('^/resources/pools/').should be_true + end + it "should raise error for bogus routes" do + lambda { find_link_to(Factory(:quota)) }.should raise_error(RuntimeError) + end + end + + context "conditional_link helper" do + it "links to viewable pool" do + @pool_user_permission = Factory(:pool_user_permission) + @pool = @pool_user_permission.permission_object + @current_user = @pool_user_permission.user + conditional_link(:name, @pool).match('^<a href="/resources/pools').should be_true + end + + it "does not link to non-viewable pool" do + @pool_user_permission = Factory(:pool_user_permission) + @pool = @pool_user_permission.permission_object + @current_user = Factory(:pool_user2_permission).user + conditional_link(:name, @pool).match("^mypool").should be_true + end + end + end
--- .../resources/deployments_controller.rb | 7 ++++- src/app/views/resources/deployments/_list.haml | 12 ++++++++++ src/features/deployment.feature | 22 ++++++++++++++++++++ src/features/step_definitions/deployment_steps.rb | 5 ++++ 4 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 src/features/deployment.feature create mode 100644 src/features/step_definitions/deployment_steps.rb
diff --git a/src/app/controllers/resources/deployments_controller.rb b/src/app/controllers/resources/deployments_controller.rb index 3e76bb0..7592fa4 100644 --- a/src/app/controllers/resources/deployments_controller.rb +++ b/src/app/controllers/resources/deployments_controller.rb @@ -22,12 +22,15 @@ class Resources::DeploymentsController < ApplicationController private def load_deployments @url_params = params + @deployments = Deployment.paginate(:all, + :page => params[:page] || 1, + :order => (params[:order_field] || 'name') +' '+ (params[:order_dir] || 'asc') + ) @header = [ { :name => "Deployment name", :sort_attr => :name }, { :name => "Deployable", :sortable => false }, - { :name => "Deployment Owner", :sort_attr => "owner.last_name"}, + { :name => "Owner", :sort_attr => "owner.last_name"}, { :name => "Running Since", :sort_attr => :running_since }, - { :name => "Heath Metric", :sort_attr => :health }, { :name => "Pool", :sort_attr => "pool.name" } ] end diff --git a/src/app/views/resources/deployments/_list.haml b/src/app/views/resources/deployments/_list.haml index 6f62520..e7272bb 100644 --- a/src/app/views/resources/deployments/_list.haml +++ b/src/app/views/resources/deployments/_list.haml @@ -18,3 +18,15 @@
%table = sortable_table_header @header + - @deployments.each do |deployment| + %tr + %td + = conditional_link(:name, deployment) + %td + = conditional_link(:name, deployment.deployable) + %td + = conditional_link(:login, deployment.owner) + %td + = deployment.created_at + %td + = conditional_link(:name, deployment.pool) \ No newline at end of file diff --git a/src/features/deployment.feature b/src/features/deployment.feature new file mode 100644 index 0000000..4736306 --- /dev/null +++ b/src/features/deployment.feature @@ -0,0 +1,22 @@ +Feature: Manage Deployments + In order to manage my cloud infrastructure + As a user + I want to manage my deployments + + Background: + Given I am an authorised user + And I am logged in + + Scenario: List deployments + Given I am on the homepage + And there is a deployment named "MySQL Cluster" belonging to "Databases" owned by "bob" + When I go to the resources deployments page + Then I should see "MySQL Cluster" + And I should see "bob" + + Scenario: View user from deployment list + Given I am on the homepage + And there is a deployment named "Postgres" belonging to "Databases" owned by "joe" + When I go to the resources deployments page + And I follow "joe" + Then I should see "John Smith (joe)" diff --git a/src/features/step_definitions/deployment_steps.rb b/src/features/step_definitions/deployment_steps.rb new file mode 100644 index 0000000..32d5e89 --- /dev/null +++ b/src/features/step_definitions/deployment_steps.rb @@ -0,0 +1,5 @@ +Given /^there is a deployment named "([^"]*)" belonging to "([^"]*)" owned by "([^"]*)"$/ do |deployment_name, deployable_name, owner_name| + user = Factory.build(:user, :login => owner_name) + deployable = Deployable.create!(:name => deployable_name) + deployable.deployments.create!({:name => deployment_name, :pool => Pool.first, :owner => user}) +end \ No newline at end of file
On Tue, Apr 19, 2011 at 04:49:28PM -0400, Matt Wagner wrote:
Hi,
I'm attaching what probably should have been a simple patch to show the username + a link for deployments.
Please disregard this series for the time being. I'm going to send out a simpler version momentarily.
-- Matt
aeolus-devel@lists.fedorahosted.org