Thanks for the feedback Mo. The problem was I failed to test after my
last merge which picked up changes for a provider type model.
Resubmitting with the fix.
On 03/03/2011 12:00 PM, Mo Morsi wrote:
Hey applied this and tried navigating to the new page but ran into
an
error. See below for comments
On 03/03/2011 12:42 PM, Richard Su wrote:
> Renamed "Images" tab to "Builds"
Would be nice to separate this out into a second patch if possible, fine
as is though.
> Table columns now matches cloud-engine-templates-details-images.png mockup.
> Table shows builds that have been completed and uploaded, and additional
> providers that a completed build can be uploaded to. The additional provider's
> provider type would have to match the build's target.
>
> Note the Arch column may be redundant because the Architecture is specified
> per template and not per build.
> ---
> .../controllers/image_factory/builds_controller.rb | 2 +-
> .../image_factory/templates_controller.rb | 27 +++++++++++++----
> src/app/views/image_factory/templates/_builds.haml | 31 ++++++++++++++++++++
> src/app/views/image_factory/templates/_images.haml | 20 -------------
> src/features/step_definitions/template_steps.rb | 6 +++-
> src/features/support/paths.rb | 2 +-
> src/features/template.feature | 13 ++++++++
> src/spec/factories/provider.rb | 8 +++++
> 8 files changed, 79 insertions(+), 30 deletions(-)
> create mode 100644 src/app/views/image_factory/templates/_builds.haml
> delete mode 100644 src/app/views/image_factory/templates/_images.haml
>
> diff --git a/src/app/controllers/image_factory/builds_controller.rb
b/src/app/controllers/image_factory/builds_controller.rb
> index ac0a3e2..01f05bd 100644
> --- a/src/app/controllers/image_factory/builds_controller.rb
> +++ b/src/app/controllers/image_factory/builds_controller.rb
> @@ -38,7 +38,7 @@ class ImageFactory::BuildsController< ApplicationController
> end
> flash[:warning] = 'Warning: ' + warnings.join unless warnings.empty?
> if errors.empty?
> - redirect_to image_factory_template_path(@tpl, :details_tab =>
'images')
> + redirect_to image_factory_template_path(@tpl, :details_tab =>
'builds')
> else
> flash_error('Error while trying to build image', errors)
> render :action => 'new'
> diff --git a/src/app/controllers/image_factory/templates_controller.rb
b/src/app/controllers/image_factory/templates_controller.rb
> index 5f905fa..a38fe0d 100644
> --- a/src/app/controllers/image_factory/templates_controller.rb
> +++ b/src/app/controllers/image_factory/templates_controller.rb
> @@ -25,9 +25,9 @@ class ImageFactory::TemplatesController<
ApplicationController
> @tpl = Template.find(params[:id])
> require_privilege(Privilege::VIEW, @tpl)
> @url_params = params.clone
> - @tab_captions = ['Properties', 'Images']
> + @tab_captions = ['Properties', 'Builds']
> @details_tab = params[:details_tab].blank? ? 'properties' :
params[:details_tab]
> - load_images(@tpl) if @details_tab == 'images'
> + load_images(@tpl) if @details_tab == 'builds'
> respond_to do |format|
> format.js do
> if @url_params.delete :details_pane
> @@ -196,12 +196,26 @@ class ImageFactory::TemplatesController<
ApplicationController
>
> def load_images(tpl)
> @images_header = [
> - {:name => 'NAME', :sort_attr => 'templates.name'},
> - {:name => 'OS', :sort_attr =>
'templates.platform'},
> - {:name => 'VERSION', :sort_attr =>
'templates.platform_version'},
> {:name => 'ARCH', :sort_attr =>
'templates.architecture'},
> + {:name => 'PROVIDER', :sort_attr => 'provider'},
> {:name => 'STATUS', :sort_attr => 'status'},
> - ]
> + {:name => 'UPLOADED?', :sort_attr =>
'images.uploaded'},
> + ]
> +
> + @imaged_provider_types = {}
> + tpl.images.each do |img|
> + img.provider_images.each do |pimg|
> + @imaged_provider_types[pimg.provider_id] = img.target
This line is throwing the error:
NoMethodError (undefined method `target' for #<Image:0x7f4f7bf16f90>):
app/controllers/image_factory/templates_controller.rb:208:in `load_images'
> + end
> + end
> +
> + @providers_not_uploaded =
Provider.find_all_by_provider_type((a)imaged_provider_types.keys.uniq)
@images_provider_types.keys is an array of provider id's. And here you
are searching for providers based on provider_type? Is this correct or
should this be @imagesd_provider_types.values? Am I misunderstanding this?
> + @providers_not_uploaded.each do |provider|
> + if @imaged_provider_types.has_key?(provider.id)
> + @providers_not_uploaded.delete(provider)
> + end
> + end
> +
Just to make sure I understand whats happening here. This is returning
this list of providers of the specified types (the provider types
associated w/ the template's images) that one of the images has not been
uploaded to.
> @images = tpl.images
> end
>
> @@ -283,4 +297,3 @@ class ImageFactory::TemplatesController<
ApplicationController
> end
>
> end
> -
> diff --git a/src/app/views/image_factory/templates/_builds.haml
b/src/app/views/image_factory/templates/_builds.haml
> new file mode 100644
> index 0000000..a9822f0
> --- /dev/null
> +++ b/src/app/views/image_factory/templates/_builds.haml
> @@ -0,0 +1,31 @@
> +Builds for
> += @tpl.platform
> += @tpl.platform_version
> +, template "#{(a)tpl.name}"
> +
> +- form_tag :action => "" do
> + /= restful_submit_tag "Cancel Job", :name =>
"job_details", :disabled => true, :class => "icon"
> + /= restful_submit_tag "Show Job Details", :name =>
"job_details", :disabled => true, :class => "icon"
> + /= restful_submit_tag "Show Job Log", :name => "job_log",
:disabled => true, :class => "icon"
> + /= restful_submit_tag "Clear Job History", :name =>
"job_history", :disabled => true, :class => "icon"
> +
> + %table
> + = sortable_table_header(@images_header)
> + %tbody
> + - if @images.empty?
> + %tr
> + %td{:colspan => 5} No Builds
> + - else
> + - @images.each do |img|
> + - img.provider_images.each do |pimg|
> + %tr
> + %td= img.template.architecture
> + %td= pimg.provider.nil? ? '' : pimg.provider.name
> + %td= img.status
> + %td= pimg.uploaded? ? 'yes' : 'no'
> + - @providers_not_uploaded.each do |provider|
> + %tr
> + %td= @tpl.architecture
> + %td= provider.name
> + %td= img.status
> + %td no
Should this last bit, the @providers_not_uploaded loop be moved over one
or two tab levels, eg to be underneath the 'else' or the '%tbody' not
the '(a)images.each' loop?
> \ No newline at end of file
> diff --git a/src/app/views/image_factory/templates/_images.haml
b/src/app/views/image_factory/templates/_images.haml
> deleted file mode 100644
> index 3fb1046..0000000
> --- a/src/app/views/image_factory/templates/_images.haml
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -- form_tag :action => "" do
> - /= restful_submit_tag "Cancel Job", :name =>
"job_details", :disabled => true, :class => "icon"
> - /= restful_submit_tag "Show Job Details", :name =>
"job_details", :disabled => true, :class => "icon"
> - /= restful_submit_tag "Show Job Log", :name => "job_log",
:disabled => true, :class => "icon"
> - /= restful_submit_tag "Clear Job History", :name =>
"job_history", :disabled => true, :class => "icon"
> -
> - %table
> - = sortable_table_header(@images_header)
> - %tbody
> - - if @images.empty?
> - %tr
> - %td{:colspan => 5} No Images
> - - else
> - - @images.each do |img|
> - %tr
> - %td= img.template.name
> - %td= img.template.platform
> - %td= img.template.platform_version
> - %td= img.template.architecture
> - %td= img.status
> diff --git a/src/features/step_definitions/template_steps.rb
b/src/features/step_definitions/template_steps.rb
> index 3491eb4..69a6928 100644
> --- a/src/features/step_definitions/template_steps.rb
> +++ b/src/features/step_definitions/template_steps.rb
> @@ -91,7 +91,6 @@ end
> When /^I choose this template$/ do
> click_link((a)template.name)
> end
> -
> Given /^there is Amazon AWS provider account$/ do
> account = Factory.build(:ec2_provider_account)
> account.save!
> @@ -101,6 +100,11 @@ Given /^there is Amazon AWS build for this template$/ do
> Image.build(@template, ProviderType.find_by_codename("ec2"))
> end
>
> +Given /^there is Amazon AWS provider with no builds$/ do
> + provider = Factory.build(:ec2_provider_no_builds)
> + provider.save!
> +end
> +
> Given /^there is an imported template$/ do
> @template = Factory.build :template, :name => name, :imported => true
> @template.save!
> diff --git a/src/features/support/paths.rb b/src/features/support/paths.rb
> index c123cba..5af9b34 100644
> --- a/src/features/support/paths.rb
> +++ b/src/features/support/paths.rb
> @@ -81,7 +81,7 @@ module NavigationHelpers
> url_for new_build_path
>
> when /the template builds page/
> - url_for image_factory_template_path(@template, :details_tab =>
'images')
> + url_for image_factory_template_path(@template, :details_tab =>
'builds')
>
> when /the templates page/
> templates_path
> diff --git a/src/features/template.feature b/src/features/template.feature
> index 336b194..4fe8e9a 100644
> --- a/src/features/template.feature
> +++ b/src/features/template.feature
> @@ -105,6 +105,19 @@ Feature: Manage Templates
> # test that we see a collection (collections are loaded by default)
> And I should see an input "conductor"
>
> + Scenario: See upload status
> + Given there is a "mock1" template
> + And there is Amazon AWS provider account
> + And there is Amazon AWS build for this template
> + And there is Amazon AWS provider with no builds
> + And I am on the image factory templates page
> + When I choose this template
> + And I follow "Builds"
> + Then I should see the following:
> + | ARCH | PROVIDERS | STATUS | UPLOADED? |
> + | x86_64 | amazon-ec2 | queued | no |
> + | x86_64 | amazon-ec2-no-builds | queued | no |
> +
Do we want to verify the uploaded column for templates / images that
have been uploaded? NBD for the time being, patch will be fine w/out this.
Other than the error and questions above the patch looks good. If you
send out another or send me a hotfix fixing the error I'll verify the
functionality.
-Mo
_______________________________________________
aeolus-devel mailing list
aeolus-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/aeolus-devel