Commit cdb34d51d53ba7e445ecd4ce04dcd7a6fa6536e6
1 parent
d50ac31c
Exists in
master
and in
26 other branches
Replace fixed option to introduce new edit modes for blocks
It resolves issue #38 Signed-off-by: Arthur Del Esposte <arthurmde@gmail.com> Signed-off-by: David Carlos <ddavidcarlos1392@gmail.com> Signed-off-by: Gabriela Navarro <navarro1703@gmail.com>
Showing
7 changed files
with
105 additions
and
33 deletions
Show diff stats
app/controllers/my_profile/profile_design_controller.rb
@@ -4,11 +4,19 @@ class ProfileDesignController < BoxOrganizerController | @@ -4,11 +4,19 @@ class ProfileDesignController < BoxOrganizerController | ||
4 | 4 | ||
5 | protect 'edit_profile_design', :profile | 5 | protect 'edit_profile_design', :profile |
6 | 6 | ||
7 | - before_filter :protect_fixed_block, :only => [:save, :move_block] | 7 | + before_filter :protect_uneditable_block, :only => [:save] |
8 | + before_filter :protect_fixed_block, :only => [:move_block] | ||
9 | + | ||
10 | + def protect_uneditable_block | ||
11 | + block = boxes_holder.blocks.find(params[:id].gsub(/^block-/, '')) | ||
12 | + if !current_person.is_admin? && !block.editable? | ||
13 | + render_access_denied | ||
14 | + end | ||
15 | + end | ||
8 | 16 | ||
9 | def protect_fixed_block | 17 | def protect_fixed_block |
10 | block = boxes_holder.blocks.find(params[:id].gsub(/^block-/, '')) | 18 | block = boxes_holder.blocks.find(params[:id].gsub(/^block-/, '')) |
11 | - if block.fixed && !current_person.is_admin? | 19 | + if !current_person.is_admin? && !block.movable? |
12 | render_access_denied | 20 | render_access_denied |
13 | end | 21 | end |
14 | end | 22 | end |
app/helpers/boxes_helper.rb
@@ -190,7 +190,7 @@ module BoxesHelper | @@ -190,7 +190,7 @@ module BoxesHelper | ||
190 | else | 190 | else |
191 | "before-block-#{block.id}" | 191 | "before-block-#{block.id}" |
192 | end | 192 | end |
193 | - if block.nil? or modifiable?(block) | 193 | + if block.nil? || movable?(block) |
194 | 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') | 194 | 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') |
195 | else | 195 | else |
196 | "" | 196 | "" |
@@ -199,14 +199,14 @@ module BoxesHelper | @@ -199,14 +199,14 @@ module BoxesHelper | ||
199 | 199 | ||
200 | # makes the given block draggable so it can be moved away. | 200 | # makes the given block draggable so it can be moved away. |
201 | def block_handle(block) | 201 | def block_handle(block) |
202 | - modifiable?(block) ? draggable_element("block-#{block.id}", :revert => true) : "" | 202 | + movable?(block) ? draggable_element("block-#{block.id}", :revert => true) : "" |
203 | end | 203 | end |
204 | 204 | ||
205 | def block_edit_buttons(block) | 205 | def block_edit_buttons(block) |
206 | buttons = [] | 206 | buttons = [] |
207 | nowhere = 'javascript: return false;' | 207 | nowhere = 'javascript: return false;' |
208 | 208 | ||
209 | - if modifiable?(block) | 209 | + if movable?(block) |
210 | if block.first? | 210 | if block.first? |
211 | buttons << icon_button('up-disabled', _("Can't move up anymore."), nowhere) | 211 | buttons << icon_button('up-disabled', _("Can't move up anymore."), nowhere) |
212 | else | 212 | else |
@@ -229,15 +229,15 @@ module BoxesHelper | @@ -229,15 +229,15 @@ module BoxesHelper | ||
229 | 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' ) | 229 | 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' ) |
230 | end | 230 | end |
231 | end | 231 | end |
232 | + end | ||
232 | 233 | ||
233 | - if block.editable? | ||
234 | - buttons << modal_icon_button(:edit, _('Edit'), { :action => 'edit', :id => block.id }) | ||
235 | - end | 234 | + if editable?(block) |
235 | + buttons << modal_icon_button(:edit, _('Edit'), { :action => 'edit', :id => block.id }) | ||
236 | + end | ||
236 | 237 | ||
237 | - if !block.main? | ||
238 | - buttons << icon_button(:delete, _('Remove block'), { :action => 'remove', :id => block.id }, { :method => 'post', :confirm => _('Are you sure you want to remove this block?')}) | ||
239 | - buttons << icon_button(:clone, _('Clone'), { :action => 'clone_block', :id => block.id }, { :method => 'post' }) | ||
240 | - end | 238 | + if movable?(block) && !block.main? |
239 | + buttons << icon_button(:delete, _('Remove block'), { :action => 'remove', :id => block.id }, { :method => 'post', :confirm => _('Are you sure you want to remove this block?')}) | ||
240 | + buttons << icon_button(:clone, _('Clone'), { :action => 'clone_block', :id => block.id }, { :method => 'post' }) | ||
241 | end | 241 | end |
242 | 242 | ||
243 | if block.respond_to?(:help) | 243 | if block.respond_to?(:help) |
@@ -273,7 +273,11 @@ module BoxesHelper | @@ -273,7 +273,11 @@ module BoxesHelper | ||
273 | classes | 273 | classes |
274 | end | 274 | end |
275 | 275 | ||
276 | - def modifiable?(block) | ||
277 | - return !block.fixed || environment.admins.include?(user) | 276 | + def movable?(block) |
277 | + return block.movable? || user.is_admin? | ||
278 | + end | ||
279 | + | ||
280 | + def editable?(block) | ||
281 | + return block.editable? || user.is_admin? | ||
278 | end | 282 | end |
279 | end | 283 | end |
app/models/block.rb
1 | class Block < ActiveRecord::Base | 1 | class Block < ActiveRecord::Base |
2 | 2 | ||
3 | - attr_accessible :title, :display, :limit, :box_id, :posts_per_page, :visualization_format, :language, :display_user, :box, :fixed | 3 | + attr_accessible :title, :display, :limit, :box_id, :posts_per_page, |
4 | + :visualization_format, :language, :display_user, | ||
5 | + :box, :edit_modes, :move_modes | ||
4 | 6 | ||
5 | # to be able to generate HTML | 7 | # to be able to generate HTML |
6 | include ActionView::Helpers::UrlHelper | 8 | include ActionView::Helpers::UrlHelper |
@@ -110,8 +112,14 @@ class Block < ActiveRecord::Base | @@ -110,8 +112,14 @@ class Block < ActiveRecord::Base | ||
110 | # * <tt>'all'</tt>: the block is always displayed | 112 | # * <tt>'all'</tt>: the block is always displayed |
111 | settings_items :language, :type => :string, :default => 'all' | 113 | settings_items :language, :type => :string, :default => 'all' |
112 | 114 | ||
113 | - # The block can be configured to be fixed. Only can be edited by environment admins | ||
114 | - settings_items :fixed, :type => :boolean, :default => false | 115 | + # The block can be configured to define the edition modes options. Only can be edited by environment admins |
116 | + # It can assume the following values: | ||
117 | + # | ||
118 | + # * <tt>'all'</tt>: the block owner has all edit options for this block | ||
119 | + # * <tt>'only_edit'</tt>: the block owner can only edit the block content, but can't move it | ||
120 | + # * <tt>'none'</tt>: the block owner can't do anything with the block | ||
121 | + settings_items :edit_modes, :type => :string, :default => 'all' | ||
122 | + settings_items :move_modes, :type => :string, :default => 'all' | ||
115 | 123 | ||
116 | # returns the description of the block, used when the user sees a list of | 124 | # returns the description of the block, used when the user sees a list of |
117 | # blocks to choose one to include in the design. | 125 | # blocks to choose one to include in the design. |
@@ -148,7 +156,11 @@ class Block < ActiveRecord::Base | @@ -148,7 +156,11 @@ class Block < ActiveRecord::Base | ||
148 | 156 | ||
149 | # Is this block editable? (Default to <tt>false</tt>) | 157 | # Is this block editable? (Default to <tt>false</tt>) |
150 | def editable? | 158 | def editable? |
151 | - true | 159 | + self.edit_modes == "all" |
160 | + end | ||
161 | + | ||
162 | + def movable? | ||
163 | + self.move_modes == "all" | ||
152 | end | 164 | end |
153 | 165 | ||
154 | # must always return false, except on MainBlock clas. | 166 | # must always return false, except on MainBlock clas. |
@@ -228,6 +240,21 @@ class Block < ActiveRecord::Base | @@ -228,6 +240,21 @@ class Block < ActiveRecord::Base | ||
228 | } | 240 | } |
229 | end | 241 | end |
230 | 242 | ||
243 | + def edit_block_options | ||
244 | + @edit_options ||= { | ||
245 | + 'all' => _('Can be modified'), | ||
246 | + 'none' => _('Cannot be modified') | ||
247 | + } | ||
248 | + end | ||
249 | + | ||
250 | + def move_block_options | ||
251 | + @move_options ||= { | ||
252 | + 'all' => _('Can be moved'), | ||
253 | + 'none' => _('Cannot be moved') | ||
254 | + } | ||
255 | + end | ||
256 | + | ||
257 | + | ||
231 | def duplicate | 258 | def duplicate |
232 | duplicated_block = self.dup | 259 | duplicated_block = self.dup |
233 | duplicated_block.display = 'never' | 260 | duplicated_block.display = 'never' |
app/views/box_organizer/edit.html.erb
@@ -5,12 +5,6 @@ | @@ -5,12 +5,6 @@ | ||
5 | 5 | ||
6 | <%= labelled_form_field(_('Custom title for this block: '), text_field(:block, :title, :maxlength => 20)) %> | 6 | <%= labelled_form_field(_('Custom title for this block: '), text_field(:block, :title, :maxlength => 20)) %> |
7 | 7 | ||
8 | - <% if environment.admins.include?(user) %> | ||
9 | - <div class="fixed_block"> | ||
10 | - <%= labelled_check_box(_("Fixed"), "block[fixed]", value = "1", checked = @block.fixed) %> | ||
11 | - </div> | ||
12 | - <% end %> | ||
13 | - | ||
14 | <%= render :partial => partial_for_class(@block.class) %> | 8 | <%= render :partial => partial_for_class(@block.class) %> |
15 | 9 | ||
16 | <div class="display"> | 10 | <div class="display"> |
@@ -25,6 +19,15 @@ | @@ -25,6 +19,15 @@ | ||
25 | 19 | ||
26 | <%= labelled_form_field(_('Show for:'), select(:block, :language, [ [ _('all languages'), 'all']] + environment.locales.map {|key, value| [value, key]} )) %> | 20 | <%= labelled_form_field(_('Show for:'), select(:block, :language, [ [ _('all languages'), 'all']] + environment.locales.map {|key, value| [value, key]} )) %> |
27 | 21 | ||
22 | + <% if user.is_admin? %> | ||
23 | + <div class="edit-modes"> | ||
24 | + <%= labelled_form_field _('Edit options:'), select_tag('block[edit_modes]', options_from_collection_for_select(@block.edit_block_options, :first, :last, @block.edit_modes)) %> | ||
25 | + </div> | ||
26 | + <div class="move-modes"> | ||
27 | + <%= labelled_form_field _('Move options:'), select_tag('block[move_modes]', options_from_collection_for_select(@block.move_block_options, :first, :last, @block.move_modes)) %> | ||
28 | + </div> | ||
29 | + <% end %> | ||
30 | + | ||
28 | <% button_bar do %> | 31 | <% button_bar do %> |
29 | <%= submit_button(:save, _('Save')) %> | 32 | <%= submit_button(:save, _('Save')) %> |
30 | <%= modal_close_button(_('Cancel')) %> | 33 | <%= modal_close_button(_('Cancel')) %> |
test/functional/profile_design_controller_test.rb
@@ -737,9 +737,9 @@ class ProfileDesignControllerTest < ActionController::TestCase | @@ -737,9 +737,9 @@ class ProfileDesignControllerTest < ActionController::TestCase | ||
737 | end | 737 | end |
738 | end | 738 | end |
739 | 739 | ||
740 | - test 'should forbid POST to save for fixed blocks' do | 740 | + test 'should forbid POST to save for uneditable blocks' do |
741 | block = profile.blocks.last | 741 | block = profile.blocks.last |
742 | - block.fixed = true | 742 | + block.edit_modes = "none" |
743 | block.save! | 743 | block.save! |
744 | 744 | ||
745 | post :save, id: block.id, profile: profile.identifier | 745 | post :save, id: block.id, profile: profile.identifier |
@@ -748,7 +748,7 @@ class ProfileDesignControllerTest < ActionController::TestCase | @@ -748,7 +748,7 @@ class ProfileDesignControllerTest < ActionController::TestCase | ||
748 | 748 | ||
749 | test 'should forbid POST to move_block for fixed blocks' do | 749 | test 'should forbid POST to move_block for fixed blocks' do |
750 | block = profile.blocks.last | 750 | block = profile.blocks.last |
751 | - block.fixed = true | 751 | + block.move_modes = "none" |
752 | block.save! | 752 | block.save! |
753 | 753 | ||
754 | post :move_block, id: block.id, profile: profile.identifier, target: "end-of-box-#{@box3.id}" | 754 | post :move_block, id: block.id, profile: profile.identifier, target: "end-of-box-#{@box3.id}" |
test/unit/block_test.rb
@@ -35,6 +35,24 @@ class BlockTest < ActiveSupport::TestCase | @@ -35,6 +35,24 @@ class BlockTest < ActiveSupport::TestCase | ||
35 | assert Block.new.editable? | 35 | assert Block.new.editable? |
36 | end | 36 | end |
37 | 37 | ||
38 | + should 'be editable if edit modes is all' do | ||
39 | + block = Block.new | ||
40 | + block.edit_modes = 'all' | ||
41 | + | ||
42 | + assert block.editable? | ||
43 | + end | ||
44 | + | ||
45 | + should 'be movable by default' do | ||
46 | + assert Block.new.movable? | ||
47 | + end | ||
48 | + | ||
49 | + should 'be movable if move modes is all' do | ||
50 | + block = Block.new | ||
51 | + block.move_modes = 'all' | ||
52 | + | ||
53 | + assert block.movable? | ||
54 | + end | ||
55 | + | ||
38 | should 'have default titles' do | 56 | should 'have default titles' do |
39 | b = Block.new | 57 | b = Block.new |
40 | b.expects(:default_title).returns('my title') | 58 | b.expects(:default_title).returns('my title') |
test/unit/boxes_helper_test.rb
@@ -122,24 +122,24 @@ class BoxesHelperTest < ActionView::TestCase | @@ -122,24 +122,24 @@ class BoxesHelperTest < ActionView::TestCase | ||
122 | display_box_content(box, '') | 122 | display_box_content(box, '') |
123 | end | 123 | end |
124 | 124 | ||
125 | - should 'not show move options on block when block is fixed' do | 125 | + should 'not show move options on block when block has no permission to edit' do |
126 | p = create_user_with_blocks | 126 | p = create_user_with_blocks |
127 | 127 | ||
128 | b = p.blocks.select{|bk| !bk.kind_of?(MainBlock) }[0] | 128 | b = p.blocks.select{|bk| !bk.kind_of?(MainBlock) }[0] |
129 | - b.fixed = true | 129 | + b.move_modes = "none" |
130 | b.save! | 130 | b.save! |
131 | 131 | ||
132 | stubs(:environment).returns(p.environment) | 132 | stubs(:environment).returns(p.environment) |
133 | stubs(:user).returns(p) | 133 | stubs(:user).returns(p) |
134 | 134 | ||
135 | - assert_equal false, modifiable?(b) | 135 | + assert_equal false, movable?(b) |
136 | end | 136 | end |
137 | 137 | ||
138 | - should 'show move options on block when block is fixed and user is admin' do | 138 | + should 'show move options on block when block has no permission to edit and user is admin' do |
139 | p = create_user_with_blocks | 139 | p = create_user_with_blocks |
140 | 140 | ||
141 | b = p.blocks.select{|bk| !bk.kind_of?(MainBlock) }[0] | 141 | b = p.blocks.select{|bk| !bk.kind_of?(MainBlock) }[0] |
142 | - b.fixed = true | 142 | + b.edit_modes = "none" |
143 | b.save! | 143 | b.save! |
144 | 144 | ||
145 | p.environment.add_admin(p) | 145 | p.environment.add_admin(p) |
@@ -147,7 +147,7 @@ class BoxesHelperTest < ActionView::TestCase | @@ -147,7 +147,7 @@ class BoxesHelperTest < ActionView::TestCase | ||
147 | stubs(:environment).returns(p.environment) | 147 | stubs(:environment).returns(p.environment) |
148 | stubs(:user).returns(p) | 148 | stubs(:user).returns(p) |
149 | 149 | ||
150 | - assert_equal true, modifiable?(b) | 150 | + assert_equal true, movable?(b) |
151 | end | 151 | end |
152 | 152 | ||
153 | should 'consider boxes_limit without custom_design' do | 153 | should 'consider boxes_limit without custom_design' do |
@@ -181,4 +181,16 @@ class BoxesHelperTest < ActionView::TestCase | @@ -181,4 +181,16 @@ class BoxesHelperTest < ActionView::TestCase | ||
181 | display_box_content(box, '') | 181 | display_box_content(box, '') |
182 | end | 182 | end |
183 | 183 | ||
184 | + should 'only show edit option on block' do | ||
185 | + p = create_user_with_blocks | ||
186 | + | ||
187 | + b = p.blocks.select{|bk| !bk.kind_of?(MainBlock) }[0] | ||
188 | + b.edit_modes = "only_edit" | ||
189 | + b.save! | ||
190 | + | ||
191 | + stubs(:environment).returns(p.environment) | ||
192 | + stubs(:user).returns(p) | ||
193 | + | ||
194 | + assert_equal false, b.editable? | ||
195 | + end | ||
184 | end | 196 | end |