From cdb34d51d53ba7e445ecd4ce04dcd7a6fa6536e6 Mon Sep 17 00:00:00 2001 From: Arthur Del Esposte Date: Tue, 10 Feb 2015 13:54:34 +0000 Subject: [PATCH] Replace fixed option to introduce new edit modes for blocks --- app/controllers/my_profile/profile_design_controller.rb | 12 ++++++++++-- app/helpers/boxes_helper.rb | 28 ++++++++++++++++------------ app/models/block.rb | 35 +++++++++++++++++++++++++++++++---- app/views/box_organizer/edit.html.erb | 15 +++++++++------ test/functional/profile_design_controller_test.rb | 6 +++--- test/unit/block_test.rb | 18 ++++++++++++++++++ test/unit/boxes_helper_test.rb | 24 ++++++++++++++++++------ 7 files changed, 105 insertions(+), 33 deletions(-) diff --git a/app/controllers/my_profile/profile_design_controller.rb b/app/controllers/my_profile/profile_design_controller.rb index fc93c7f..7a5ee5a 100644 --- a/app/controllers/my_profile/profile_design_controller.rb +++ b/app/controllers/my_profile/profile_design_controller.rb @@ -4,11 +4,19 @@ class ProfileDesignController < BoxOrganizerController protect 'edit_profile_design', :profile - before_filter :protect_fixed_block, :only => [:save, :move_block] + before_filter :protect_uneditable_block, :only => [:save] + before_filter :protect_fixed_block, :only => [:move_block] + + def protect_uneditable_block + block = boxes_holder.blocks.find(params[:id].gsub(/^block-/, '')) + if !current_person.is_admin? && !block.editable? + render_access_denied + end + end def protect_fixed_block block = boxes_holder.blocks.find(params[:id].gsub(/^block-/, '')) - if block.fixed && !current_person.is_admin? + if !current_person.is_admin? && !block.movable? render_access_denied end end diff --git a/app/helpers/boxes_helper.rb b/app/helpers/boxes_helper.rb index e1fd4a1..bef9e61 100644 --- a/app/helpers/boxes_helper.rb +++ b/app/helpers/boxes_helper.rb @@ -190,7 +190,7 @@ module BoxesHelper else "before-block-#{block.id}" end - if block.nil? or modifiable?(block) + if block.nil? || movable?(block) content_tag('div', ' ', :id => id, :class => 'block-target' ) + drop_receiving_element(id, :url => { :action => 'move_block', :target => id }, :accept => box.acceptable_blocks, :hoverclass => 'block-target-hover') else "" @@ -199,14 +199,14 @@ module BoxesHelper # makes the given block draggable so it can be moved away. def block_handle(block) - modifiable?(block) ? draggable_element("block-#{block.id}", :revert => true) : "" + movable?(block) ? draggable_element("block-#{block.id}", :revert => true) : "" end def block_edit_buttons(block) buttons = [] nowhere = 'javascript: return false;' - if modifiable?(block) + if movable?(block) if block.first? buttons << icon_button('up-disabled', _("Can't move up anymore."), nowhere) else @@ -229,15 +229,15 @@ module BoxesHelper buttons << icon_button('left', _('Move to the opposite side'), { :action => 'move_block', :target => 'end-of-box-' + holder.boxes[1].id.to_s, :id => block.id }, :method => 'post' ) end end + end - if block.editable? - buttons << modal_icon_button(:edit, _('Edit'), { :action => 'edit', :id => block.id }) - end + if editable?(block) + buttons << modal_icon_button(:edit, _('Edit'), { :action => 'edit', :id => block.id }) + end - if !block.main? - buttons << icon_button(:delete, _('Remove block'), { :action => 'remove', :id => block.id }, { :method => 'post', :confirm => _('Are you sure you want to remove this block?')}) - buttons << icon_button(:clone, _('Clone'), { :action => 'clone_block', :id => block.id }, { :method => 'post' }) - end + if movable?(block) && !block.main? + buttons << icon_button(:delete, _('Remove block'), { :action => 'remove', :id => block.id }, { :method => 'post', :confirm => _('Are you sure you want to remove this block?')}) + buttons << icon_button(:clone, _('Clone'), { :action => 'clone_block', :id => block.id }, { :method => 'post' }) end if block.respond_to?(:help) @@ -273,7 +273,11 @@ module BoxesHelper classes end - def modifiable?(block) - return !block.fixed || environment.admins.include?(user) + def movable?(block) + return block.movable? || user.is_admin? + end + + def editable?(block) + return block.editable? || user.is_admin? end end diff --git a/app/models/block.rb b/app/models/block.rb index 524d197..38217f0 100644 --- a/app/models/block.rb +++ b/app/models/block.rb @@ -1,6 +1,8 @@ class Block < ActiveRecord::Base - attr_accessible :title, :display, :limit, :box_id, :posts_per_page, :visualization_format, :language, :display_user, :box, :fixed + attr_accessible :title, :display, :limit, :box_id, :posts_per_page, + :visualization_format, :language, :display_user, + :box, :edit_modes, :move_modes # to be able to generate HTML include ActionView::Helpers::UrlHelper @@ -110,8 +112,14 @@ class Block < ActiveRecord::Base # * 'all': the block is always displayed settings_items :language, :type => :string, :default => 'all' - # The block can be configured to be fixed. Only can be edited by environment admins - settings_items :fixed, :type => :boolean, :default => false + # The block can be configured to define the edition modes options. Only can be edited by environment admins + # It can assume the following values: + # + # * 'all': the block owner has all edit options for this block + # * 'only_edit': the block owner can only edit the block content, but can't move it + # * 'none': the block owner can't do anything with the block + settings_items :edit_modes, :type => :string, :default => 'all' + settings_items :move_modes, :type => :string, :default => 'all' # returns the description of the block, used when the user sees a list of # blocks to choose one to include in the design. @@ -148,7 +156,11 @@ class Block < ActiveRecord::Base # Is this block editable? (Default to false) def editable? - true + self.edit_modes == "all" + end + + def movable? + self.move_modes == "all" end # must always return false, except on MainBlock clas. @@ -228,6 +240,21 @@ class Block < ActiveRecord::Base } end + def edit_block_options + @edit_options ||= { + 'all' => _('Can be modified'), + 'none' => _('Cannot be modified') + } + end + + def move_block_options + @move_options ||= { + 'all' => _('Can be moved'), + 'none' => _('Cannot be moved') + } + end + + def duplicate duplicated_block = self.dup duplicated_block.display = 'never' diff --git a/app/views/box_organizer/edit.html.erb b/app/views/box_organizer/edit.html.erb index 03f6687..9fa35eb 100644 --- a/app/views/box_organizer/edit.html.erb +++ b/app/views/box_organizer/edit.html.erb @@ -5,12 +5,6 @@ <%= labelled_form_field(_('Custom title for this block: '), text_field(:block, :title, :maxlength => 20)) %> - <% if environment.admins.include?(user) %> -
- <%= labelled_check_box(_("Fixed"), "block[fixed]", value = "1", checked = @block.fixed) %> -
- <% end %> - <%= render :partial => partial_for_class(@block.class) %>
@@ -25,6 +19,15 @@ <%= labelled_form_field(_('Show for:'), select(:block, :language, [ [ _('all languages'), 'all']] + environment.locales.map {|key, value| [value, key]} )) %> + <% if user.is_admin? %> +
+ <%= labelled_form_field _('Edit options:'), select_tag('block[edit_modes]', options_from_collection_for_select(@block.edit_block_options, :first, :last, @block.edit_modes)) %> +
+
+ <%= labelled_form_field _('Move options:'), select_tag('block[move_modes]', options_from_collection_for_select(@block.move_block_options, :first, :last, @block.move_modes)) %> +
+ <% end %> + <% button_bar do %> <%= submit_button(:save, _('Save')) %> <%= modal_close_button(_('Cancel')) %> diff --git a/test/functional/profile_design_controller_test.rb b/test/functional/profile_design_controller_test.rb index bbe55ac..3770477 100644 --- a/test/functional/profile_design_controller_test.rb +++ b/test/functional/profile_design_controller_test.rb @@ -737,9 +737,9 @@ class ProfileDesignControllerTest < ActionController::TestCase end end - test 'should forbid POST to save for fixed blocks' do + test 'should forbid POST to save for uneditable blocks' do block = profile.blocks.last - block.fixed = true + block.edit_modes = "none" block.save! post :save, id: block.id, profile: profile.identifier @@ -748,7 +748,7 @@ class ProfileDesignControllerTest < ActionController::TestCase test 'should forbid POST to move_block for fixed blocks' do block = profile.blocks.last - block.fixed = true + block.move_modes = "none" block.save! post :move_block, id: block.id, profile: profile.identifier, target: "end-of-box-#{@box3.id}" diff --git a/test/unit/block_test.rb b/test/unit/block_test.rb index e23e84f..ce67ec9 100644 --- a/test/unit/block_test.rb +++ b/test/unit/block_test.rb @@ -35,6 +35,24 @@ class BlockTest < ActiveSupport::TestCase assert Block.new.editable? end + should 'be editable if edit modes is all' do + block = Block.new + block.edit_modes = 'all' + + assert block.editable? + end + + should 'be movable by default' do + assert Block.new.movable? + end + + should 'be movable if move modes is all' do + block = Block.new + block.move_modes = 'all' + + assert block.movable? + end + should 'have default titles' do b = Block.new b.expects(:default_title).returns('my title') diff --git a/test/unit/boxes_helper_test.rb b/test/unit/boxes_helper_test.rb index b7c06fe..7787303 100644 --- a/test/unit/boxes_helper_test.rb +++ b/test/unit/boxes_helper_test.rb @@ -122,24 +122,24 @@ class BoxesHelperTest < ActionView::TestCase display_box_content(box, '') end - should 'not show move options on block when block is fixed' do + should 'not show move options on block when block has no permission to edit' do p = create_user_with_blocks b = p.blocks.select{|bk| !bk.kind_of?(MainBlock) }[0] - b.fixed = true + b.move_modes = "none" b.save! stubs(:environment).returns(p.environment) stubs(:user).returns(p) - assert_equal false, modifiable?(b) + assert_equal false, movable?(b) end - should 'show move options on block when block is fixed and user is admin' do + should 'show move options on block when block has no permission to edit and user is admin' do p = create_user_with_blocks b = p.blocks.select{|bk| !bk.kind_of?(MainBlock) }[0] - b.fixed = true + b.edit_modes = "none" b.save! p.environment.add_admin(p) @@ -147,7 +147,7 @@ class BoxesHelperTest < ActionView::TestCase stubs(:environment).returns(p.environment) stubs(:user).returns(p) - assert_equal true, modifiable?(b) + assert_equal true, movable?(b) end should 'consider boxes_limit without custom_design' do @@ -181,4 +181,16 @@ class BoxesHelperTest < ActionView::TestCase display_box_content(box, '') end + should 'only show edit option on block' do + p = create_user_with_blocks + + b = p.blocks.select{|bk| !bk.kind_of?(MainBlock) }[0] + b.edit_modes = "only_edit" + b.save! + + stubs(:environment).returns(p.environment) + stubs(:user).returns(p) + + assert_equal false, b.editable? + end end -- libgit2 0.21.2