On 11/09/2011 07:56 AM, jzigmund(a)redhat.com wrote:
From: Jozef Zigmund<jzigmund(a)redhat.com>
URL field was removed from both views, upload button and checkbox for
"editing deployable XML file after save" were added to new form.
Also to catalog_entry#show view was added text area of deployable XML file,
with link to edit that XML file.
---
src/app/controllers/catalog_entries_controller.rb | 33 ++++++----------
src/app/controllers/catalogs_controller.rb | 2 +-
src/app/models/catalog_entry.rb | 20 +++++++--
src/app/views/catalog_entries/_form.html.haml | 42 +++++++++++++-------
src/app/views/catalog_entries/_list.html.haml | 2 +-
.../views/catalog_entries/_properties.html.haml | 8 +++-
src/app/views/catalog_entries/new.html.haml | 2 +-
src/app/views/catalogs/show.html.haml | 2 +-
src/config/locales/en.yml | 8 +++-
9 files changed, 72 insertions(+), 47 deletions(-)
diff --git a/src/app/controllers/catalog_entries_controller.rb
b/src/app/controllers/catalog_entries_controller.rb
index 5e36cc0..56bbfc2 100644
--- a/src/app/controllers/catalog_entries_controller.rb
+++ b/src/app/controllers/catalog_entries_controller.rb
@@ -58,18 +47,22 @@ class CatalogEntriesController< ApplicationController
end
require_privilege(Privilege::CREATE, CatalogEntry)
-
- @catalog = Catalog.find(params[:catalog_id])
require_privilege(Privilege::MODIFY, @catalog)
This isn't new for your
patch, but I just noticed that the permission
checks are wrong here. In this case, we're currently requiring that:
1) user has site-wide CatalogEntry create privileges
2) user has modify privilege on the containing catalog
So 1) is too strict and 2) is unnecessary. All we really need to verify
is that the user has CatalogEntry create privileges on the current
catalog -- since it's possible to grant a user privileges to create
deployables in only one catalog, and MODIFY on the catalog isn't
required. So I'd remove both require_privilege statements and replace
with the following:
require_privilege(Privilege::CREATE, CatalogEntry, @catalog)
The same modification (adding @catalog) should be added to the
permission check on the 'new' operation.
@catalog_entry = CatalogEntry.new(params[:catalog_entry])
- @catalog_entry.catalog = @catalog
@catalog_entry.owner = current_user
+
if @catalog_entry.save
flash[:notice] = t "catalog_entries.flash.notice.added"
- flash[:warning] = t("catalog_entries.flash.warning.not_valid") unless
@catalog_entry.accessible_and_valid_deployable_xml?((a)catalog_entry.url)
- redirect_to catalog_catalog_entries_path(@catalog)
+ flash[:warning] = t("catalog_entries.flash.warning.not_valid") unless
@catalog_entry.valid_deployable_xml?
I'm thinking that we should validate the
xml _before_ saving the entry
rather than after, otherwise the user may cancel the operation at this
point and we're stuck with an invalid Deployable that users will attempt
to launch. We should also make sure that we validate the user-edited XML
both here and in the 'edit' action.
If you add a model validation that calls valid_deployable_xml? and
populates flash[] as necessary then it will be checked automatically on
save.
+ if params[:edit_xml]
+ redirect_to edit_catalog_catalog_entry_path @catalog_entry.catalog.id,
@catalog_entry.id, :edit_xml =>true
+ else
+ redirect_to catalog_catalog_entries_path(@catalog)
+ end
else
+ @catalog = Catalog.find(params[:catalog_id])
load_catalogs
+ params.delete(:edit_xml) if params[:edit_xml]
render :new
end
end