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