Hi Tomas, Just to reply to your comments,
The Opaque HardwareProfile. I know this causes issues with this patch, this is because the Opaque HWP has nil values where it shouldn't. This is a use case that does not occur in real providers and as such I decided to ignore it. In my opinion if at all possible this should be removed from the Mock driver and we should have validation in the HardwareProfile model to catch any other abnormal cases like. Other comment in line Thanks mate Martyn ----- Original Message ----- From: "Tomas Sedovic" <[email protected]> To: [email protected] Sent: Monday, January 3, 2011 4:07:26 PM Subject: Re: [deltacloud-devel] [PATCH aeolus 2/2] Added new views/controllers for admin, hardware profiles Hey Martyn, Good stuff. I've found 3 things. The first: the page that lists the HWPs contains "Opaque" HWP as well. When I click on it, I get the following error: Internal Server Error undefined method `name' for nil:NilClass The other two are inline. Could you please resend it afterwards? Thanks, Thomas On 12/28/2010 05:43 PM, [email protected] wrote: > From: Martyn Taylor<[email protected]> > > --- > .../admin/hardware_profiles_controller.rb | 79 > +++++++++++++++++++- > src/app/models/hardware_profile_property.rb | 13 +++ > .../views/admin/hardware_profiles/_history.haml | 1 + > src/app/views/admin/hardware_profiles/_list.haml | 28 +++++++ > .../_matching_provider_hardware_profiles.haml | 18 +++++ > .../views/admin/hardware_profiles/_properties.haml | 20 +++++ > src/app/views/admin/hardware_profiles/index.haml | 3 +- > src/app/views/admin/hardware_profiles/show.haml | 5 + > src/features/support/paths.rb | 3 + > 9 files changed, 167 insertions(+), 3 deletions(-) > create mode 100644 src/app/views/admin/hardware_profiles/_history.haml > create mode 100644 src/app/views/admin/hardware_profiles/_list.haml > create mode 100644 > src/app/views/admin/hardware_profiles/_matching_provider_hardware_profiles.haml > create mode 100644 src/app/views/admin/hardware_profiles/_properties.haml > create mode 100644 src/app/views/admin/hardware_profiles/show.haml > > diff --git a/src/app/controllers/admin/hardware_profiles_controller.rb > b/src/app/controllers/admin/hardware_profiles_controller.rb > index e0d2a0e..67c655d 100644 > --- a/src/app/controllers/admin/hardware_profiles_controller.rb > +++ b/src/app/controllers/admin/hardware_profiles_controller.rb > @@ -1,6 +1,81 @@ > class Admin::HardwareProfilesController< ApplicationController > before_filter :require_user > - > + before_filter :load_hardware_profiles, :only => [:index, :show] > + before_filter :load_hardware_profile, :only => [:show] > def index > end > -end > + > + def show > + @tab_captions = ['Properties', 'History', 'Matching Provider Hardware > Profiles'] > + @details_tab = params[:details_tab].blank? ? 'properties' : > params[:details_tab] Thinking aloud here: could we use something like this instead? @details_tab = params[:details_tab] or 'properties' I know my code has is the same way as yours, but this looks like a better way to do it. What do you think? I'm not sure we can replace: @details_tab = params[:details_tab].blank? ? 'properties' : params[:details_tab] with @details_tab = params[:details_tab] or 'properties' Since we're not just checking 'nil', 'false' here for params[:details_tab] we're checking .blank? which could be a number of things, mainly empty string. > + case @details_tab > + when 'properties' > + properties > + when 'matching_provider_hardware_profiles' > + matching_provider_hardware_profiles > + end > + respond_to do |format| > + format.js do > + if @url_params.delete :details_pane > + render :partial => 'layouts/details_pane' and return > + end > + render :partial => @details_tab and return > + end > + format.html { render :action => 'show'} > + end > + end > + > + def new > + end > + > + def create > + end > + > + def delete > + end > + > + private > + def properties > + @properties_header = [ > + { :name => "Name", :sort_attr => :name}, > + { :name => "Kind", :sort_attr => :kind }, > + { :name => "Range First", :sort_attr => :range_first}, > + { :name => "Range Last", :sort_attr => :range_last }, > + { :name => "Enum Entries", :sort_attr => :false }, > + { :name => "Default Value", :sort_attr => :value}, > + { :name => "Unit", :sort_attr => :unit} > + ] > + @hwp_properties = [...@hardware_profile.memory, @hardware_profile.cpu, > @hardware_profile.storage, @hardware_profile.architecture] > + end > + > + #TODO Update this method when moving to new HWP Model > + def matching_provider_hardware_profiles > + @provider_hwps_header = [ > + { :name => "Provider Name", :sort_attr => "provider.name" }, > + { :name => "Hardware Profile Name", :sort_attr => :name }, > + { :name => "Architecture", :sort_attr => :architecture }, > + { :name => "Memory", :sort_attr => :memory}, > + { :name => "Storage", :sort_attr => :storage }, > + { :name => "Virtual CPU", :sort_attr => :cpus} > + ] > + @matching_hwps = HardwareProfile.all(:include => > "aggregator_hardware_profiles", > + :conditions => > {:hardware_profile_map => { :aggregator_hardware_profile_id => params[:id] > }}) > + end > + > + def load_hardware_profiles > + @hardware_profiles = HardwareProfile.all(:conditions => 'provider_id IS > NULL') > + @url_params = params > + @header = [ > + { :name => "Hardware Profile Name", :sort_attr => :name }, > + { :name => "Architecture", :sort_attr => :architecture }, > + { :name => "Memory", :sort_attr => :memory}, > + { :name => "Storage", :sort_attr => :storage }, > + { :name => "Virtual CPU", :sort_attr => :cpus} > + ] > + end > + > + def load_hardware_profile > + @hardware_profile = HardwareProfile.find((params[:id] || []).first) > + end > + > +end > \ No newline at end of file > diff --git a/src/app/models/hardware_profile_property.rb > b/src/app/models/hardware_profile_property.rb > index 1291122..46e4b8e 100644 > --- a/src/app/models/hardware_profile_property.rb > +++ b/src/app/models/hardware_profile_property.rb > @@ -97,5 +97,18 @@ class HardwareProfileProperty< ActiveRecord::Base > end > end > end > + > + def to_s > + case kind > + when FIXED > + value > + when RANGE > + range_first.to_s + " - " + range_last.to_s > + when ENUM > + (property_enum_entries.collect { |enum| enum.value }).join(", ") > + else > + "undefined" > + end > + end > end > > diff --git a/src/app/views/admin/hardware_profiles/_history.haml > b/src/app/views/admin/hardware_profiles/_history.haml > new file mode 100644 > index 0000000..894e9cd > --- /dev/null > +++ b/src/app/views/admin/hardware_profiles/_history.haml > @@ -0,0 +1 @@ > +%h3 History > diff --git a/src/app/views/admin/hardware_profiles/_list.haml > b/src/app/views/admin/hardware_profiles/_list.haml > new file mode 100644 > index 0000000..7fa2c2b > --- /dev/null > +++ b/src/app/views/admin/hardware_profiles/_list.haml > @@ -0,0 +1,28 @@ > +- form_tag do |f| You can drop the `do |f|` part. Plain `-form_tag` will suffice. The `do |f|` is used for the `form_for` helper. I've made that same mistake earlier in a view -- it's possible you copied that. If so, I'm sorry. > + #object-actions > + = restful_submit_tag "Create", "create", admin_hardware_profiles_path, > "PUT" > + = restful_submit_tag "Delete", "delete", admin_hardware_profiles_path, > "DELETE" > + > + #selections > + %p > + Select: > + = link_to "All", @url_params.merge(:select => 'all') > + %span> , > + = link_to "None", @url_params.merge(:select => 'none') > + > +%table > + = sortable_table_header @header > + - @hardware_profiles.each do |hwp| > + %tr > + %td > + - selected = @url_params[:select] == 'all' > + = check_box(:pool, "selected[#{hwp.id}]", :checked => selected) > + = link_to hwp.name, admin_hardware_profile_path(hwp) > + %td > + =hwp.architecture.to_s > + %td > + =hwp.memory.to_s > + %td > + =hwp.storage.to_s > + %td > + =hwp.cpu.to_s > \ No newline at end of file > diff --git > a/src/app/views/admin/hardware_profiles/_matching_provider_hardware_profiles.haml > > b/src/app/views/admin/hardware_profiles/_matching_provider_hardware_profiles.haml > new file mode 100644 > index 0000000..cfbb856 > --- /dev/null > +++ > b/src/app/views/admin/hardware_profiles/_matching_provider_hardware_profiles.haml > @@ -0,0 +1,18 @@ > +%h3 > + = @hardware_profile.name > +%table > + = sortable_table_header @provider_hwps_header > + - @matching_hwps.each do |hwp| > + %tr > + %td > + = link_to hwp.provider.name, admin_provider_path(hwp.provider) > + %td > + = link_to hwp.name, admin_hardware_profile_path(hwp) > + %td > + =hwp.architecture.to_s > + %td > + =hwp.memory.to_s > + %td > + =hwp.storage.to_s > + %td > + =hwp.cpu.to_s > \ No newline at end of file > diff --git a/src/app/views/admin/hardware_profiles/_properties.haml > b/src/app/views/admin/hardware_profiles/_properties.haml > new file mode 100644 > index 0000000..590edc5 > --- /dev/null > +++ b/src/app/views/admin/hardware_profiles/_properties.haml > @@ -0,0 +1,20 @@ > +%h3 > + = @hardware_profile.name + "(" + (@hardware_profile.provider_id.nil? ? > "Front End" : "Provider" ) + ")" > +%table > + = sortable_table_header @properties_header > + - @hwp_properties.each do |hwpp| > + %tr > + %td > + =hwpp.name.nil? ? "n/a" : hwpp.name > + %td > + =hwpp.kind.nil? ? "n/a" : hwpp.kind > + %td > + =hwpp.range_first.nil? ? "n/a" : hwpp.range_first > + %td > + =hwpp.range_last.nil? ? "n/a" : hwpp.range_last > + %td > + =hwpp.property_enum_entries.empty? ? "n/a" : > (hwpp.property_enum_entries.collect { |enum| enum.value.to_s }).join(", ") > + %td > + =hwpp.value.nil? ? "n/a" : hwpp.value > + %td > + =hwpp.unit.nil? ? "n/a" : hwpp.unit > diff --git a/src/app/views/admin/hardware_profiles/index.haml > b/src/app/views/admin/hardware_profiles/index.haml > index dd779f7..62ccbc6 100644 > --- a/src/app/views/admin/hardware_profiles/index.haml > +++ b/src/app/views/admin/hardware_profiles/index.haml > @@ -1 +1,2 @@ > -admin/hardware_profiles/index.haml > +- content_for :list do > + = render :partial => 'list' > diff --git a/src/app/views/admin/hardware_profiles/show.haml > b/src/app/views/admin/hardware_profiles/show.haml > new file mode 100644 > index 0000000..a5a777d > --- /dev/null > +++ b/src/app/views/admin/hardware_profiles/show.haml > @@ -0,0 +1,5 @@ > +- content_for :list do > + = render :partial => 'list' > + > +- content_for :details do > + = render :partial => 'layouts/details_pane' > \ No newline at end of file > diff --git a/src/features/support/paths.rb b/src/features/support/paths.rb > index c3309fe..ceeba17 100644 > --- a/src/features/support/paths.rb > +++ b/src/features/support/paths.rb > @@ -92,6 +92,9 @@ module NavigationHelpers > when /the settings update page/ > url_for :action => 'update', :controller => 'settings', :only_path > => true > > + when /the hardware profiles page/ > + url_for admin_hardware_profiles_path > + > # Add more mappings here. > # Here is an example that pulls values out of the Regexp: > # _______________________________________________ deltacloud-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/deltacloud-devel _______________________________________________ deltacloud-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/deltacloud-devel
