Commit 56e1b119593f2eab0ba6efb3e5c3546121a3dd17
Exists in
master
and in
23 other branches
Merge branch 'move-article-folder' into features
Conflicts: app/helpers/forms_helper.rb app/sweepers/article_sweeper.rb
Showing
12 changed files
with
134 additions
and
20 deletions
Show diff stats
app/controllers/my_profile/cms_controller.rb
| @@ -149,7 +149,6 @@ class CmsController < MyProfileController | @@ -149,7 +149,6 @@ class CmsController < MyProfileController | ||
| 149 | @uploaded_files = [] | 149 | @uploaded_files = [] |
| 150 | @article = @parent = check_parent(params[:parent_id]) | 150 | @article = @parent = check_parent(params[:parent_id]) |
| 151 | @target = @parent ? ('/%s/%s' % [profile.identifier, @parent.full_name]) : '/%s' % profile.identifier | 151 | @target = @parent ? ('/%s/%s' % [profile.identifier, @parent.full_name]) : '/%s' % profile.identifier |
| 152 | - @folders = Folder.find(:all, :conditions => { :profile_id => profile }) | ||
| 153 | if @article | 152 | if @article |
| 154 | record_coming | 153 | record_coming |
| 155 | end | 154 | end |
app/helpers/application_helper.rb
| @@ -653,19 +653,6 @@ module ApplicationHelper | @@ -653,19 +653,6 @@ module ApplicationHelper | ||
| 653 | content_tag('div', result) | 653 | content_tag('div', result) |
| 654 | end | 654 | end |
| 655 | 655 | ||
| 656 | - def select_folder(label, object, method, collection, html_options = {}, js_options = {}) | ||
| 657 | - root = profile ? profile.identifier : _("root") | ||
| 658 | - labelled_form_field(label, select(object, method, | ||
| 659 | - collection.map {|f| [ root + '/' + f.full_name, f.id ]}, | ||
| 660 | - {:include_blank => root}, html_options.merge(js_options))) | ||
| 661 | - end | ||
| 662 | - | ||
| 663 | - def select_profile_folder(label, object, method, profile, html_options = {}, js_options = {}) | ||
| 664 | - labelled_form_field(label, select(object, method, | ||
| 665 | - profile.folders.map {|f| [ profile.identifier + '/' + f.full_name, f.id ]}, | ||
| 666 | - {:include_blank => profile.identifier}, html_options.merge(js_options))) | ||
| 667 | - end | ||
| 668 | - | ||
| 669 | def theme_option(opt = nil) | 656 | def theme_option(opt = nil) |
| 670 | conf = RAILS_ROOT.to_s() + | 657 | conf = RAILS_ROOT.to_s() + |
| 671 | '/public' + theme_path + | 658 | '/public' + theme_path + |
app/helpers/forms_helper.rb
| @@ -142,6 +142,38 @@ module FormsHelper | @@ -142,6 +142,38 @@ module FormsHelper | ||
| 142 | content_tag('table',rows.join("\n")) | 142 | content_tag('table',rows.join("\n")) |
| 143 | end | 143 | end |
| 144 | 144 | ||
| 145 | + def select_folder(label_text, field_id, collection, default_value=nil, html_options = {}, js_options = {}) | ||
| 146 | + root = profile ? profile.identifier : _("root") | ||
| 147 | + labelled_form_field( | ||
| 148 | + label_text, | ||
| 149 | + select_tag( | ||
| 150 | + field_id, | ||
| 151 | + options_for_select( | ||
| 152 | + [[root, '']] + | ||
| 153 | + collection.collect {|f| [ root + '/' + f.full_name, f.id ] }, | ||
| 154 | + default_value | ||
| 155 | + ), | ||
| 156 | + html_options.merge(js_options) | ||
| 157 | + ) | ||
| 158 | + ) | ||
| 159 | + end | ||
| 160 | + | ||
| 161 | + def select_profile_folder(label_text, field_id, profile, default_value='', html_options = {}, js_options = {}) | ||
| 162 | + result = labelled_form_field( | ||
| 163 | + label_text, | ||
| 164 | + select_tag( | ||
| 165 | + field_id, | ||
| 166 | + options_for_select( | ||
| 167 | + [[profile.identifier, '']] + | ||
| 168 | + profile.folders.collect {|f| [ profile.identifier + '/' + f.full_name, f.id ] }, | ||
| 169 | + default_value | ||
| 170 | + ), | ||
| 171 | + html_options.merge(js_options) | ||
| 172 | + ) | ||
| 173 | + ) | ||
| 174 | + return result | ||
| 175 | + end | ||
| 176 | + | ||
| 145 | def date_field(name, value, format = '%Y-%m-%d', datepicker_options = {}, html_options = {}) | 177 | def date_field(name, value, format = '%Y-%m-%d', datepicker_options = {}, html_options = {}) |
| 146 | datepicker_options[:disabled] ||= false | 178 | datepicker_options[:disabled] ||= false |
| 147 | datepicker_options[:alt_field] ||= '' | 179 | datepicker_options[:alt_field] ||= '' |
app/models/article.rb
| @@ -79,6 +79,25 @@ class Article < ActiveRecord::Base | @@ -79,6 +79,25 @@ class Article < ActiveRecord::Base | ||
| 79 | validate :native_translation_must_have_language | 79 | validate :native_translation_must_have_language |
| 80 | validate :translation_must_have_language | 80 | validate :translation_must_have_language |
| 81 | 81 | ||
| 82 | + validate :no_self_reference | ||
| 83 | + validate :no_cyclical_reference, :if => 'parent_id.present?' | ||
| 84 | + | ||
| 85 | + def no_self_reference | ||
| 86 | + errors.add(:parent_id, _('self-reference is not allowed.')) if id && parent_id == id | ||
| 87 | + end | ||
| 88 | + | ||
| 89 | + def no_cyclical_reference | ||
| 90 | + current_parent = Article.find(parent_id) | ||
| 91 | + while current_parent | ||
| 92 | + if current_parent == self | ||
| 93 | + errors.add(:parent_id, _('cyclical reference is not allowed.')) | ||
| 94 | + break | ||
| 95 | + end | ||
| 96 | + current_parent = current_parent.parent | ||
| 97 | + end | ||
| 98 | + end | ||
| 99 | + | ||
| 100 | + | ||
| 82 | def is_trackable? | 101 | def is_trackable? |
| 83 | self.published? && self.notifiable? && self.advertise? && self.profile.public_profile | 102 | self.published? && self.notifiable? && self.advertise? && self.profile.public_profile |
| 84 | end | 103 | end |
| @@ -154,6 +173,12 @@ class Article < ActiveRecord::Base | @@ -154,6 +173,12 @@ class Article < ActiveRecord::Base | ||
| 154 | article.advertise = true | 173 | article.advertise = true |
| 155 | end | 174 | end |
| 156 | 175 | ||
| 176 | + before_save do |article| | ||
| 177 | + article.parent = article.parent_id ? Article.find(article.parent_id) : nil | ||
| 178 | + parent_path = article.parent ? article.parent.path : nil | ||
| 179 | + article.path = [parent_path, article.slug].compact.join('/') | ||
| 180 | + end | ||
| 181 | + | ||
| 157 | # retrieves all articles belonging to the given +profile+ that are not | 182 | # retrieves all articles belonging to the given +profile+ that are not |
| 158 | # sub-articles of any other article. | 183 | # sub-articles of any other article. |
| 159 | named_scope :top_level_for, lambda { |profile| | 184 | named_scope :top_level_for, lambda { |profile| |
app/sweepers/article_sweeper.rb
| @@ -10,11 +10,17 @@ class ArticleSweeper < ActiveRecord::Observer | @@ -10,11 +10,17 @@ class ArticleSweeper < ActiveRecord::Observer | ||
| 10 | expire_caches(article) | 10 | expire_caches(article) |
| 11 | end | 11 | end |
| 12 | 12 | ||
| 13 | + def before_update(article) | ||
| 14 | + if article.parent_id_change | ||
| 15 | + Article.find(article.parent_id_was).touch if article.parent_id_was | ||
| 16 | + end | ||
| 17 | + end | ||
| 18 | + | ||
| 13 | protected | 19 | protected |
| 14 | 20 | ||
| 15 | def expire_caches(article) | 21 | def expire_caches(article) |
| 16 | return if !article.environment | 22 | return if !article.environment |
| 17 | - article.hierarchy.each { |a| a.touch if a != article } | 23 | + article.hierarchy(true).each { |a| a.touch if a != article } |
| 18 | blocks = article.profile.blocks | 24 | blocks = article.profile.blocks |
| 19 | blocks += article.profile.environment.blocks if article.profile.environment | 25 | blocks += article.profile.environment.blocks if article.profile.environment |
| 20 | blocks = blocks.select{|b|[RecentDocumentsBlock, BlogArchivesBlock].any?{|c| b.kind_of?(c)}} | 26 | blocks = blocks.select{|b|[RecentDocumentsBlock, BlogArchivesBlock].any?{|c| b.kind_of?(c)}} |
app/views/cms/_general_fields.html.erb
| 1 | +<%= select_profile_folder(_('Parent folder:'), 'article[parent_id]', profile, @article.parent_id) %> | ||
| 1 | <%= labelled_form_field(_('License'), select(:article, :license_id, options_for_select_with_title([[_('None'), nil]] + profile.environment.licenses.map {|license| [license.name, license.id]}, @article.license ? @article.license.id : nil))) %> | 2 | <%= labelled_form_field(_('License'), select(:article, :license_id, options_for_select_with_title([[_('None'), nil]] + profile.environment.licenses.map {|license| [license.name, license.id]}, @article.license ? @article.license.id : nil))) %> |
app/views/cms/_text_editor_sidebar.rhtml
| @@ -9,8 +9,7 @@ | @@ -9,8 +9,7 @@ | ||
| 9 | <div id='media-upload-form'> | 9 | <div id='media-upload-form'> |
| 10 | <% form_tag({ :action => 'media_upload' }, :multipart => true) do %> | 10 | <% form_tag({ :action => 'media_upload' }, :multipart => true) do %> |
| 11 | <div class='formfield'> | 11 | <div class='formfield'> |
| 12 | - <%# TODO duplicated from partial upload_file_form %> | ||
| 13 | - <%= labelled_form_field(_('Choose folder to upload files:'), select_tag('parent_id', options_for_select([[profile.identifier, '']] + profile.folders.collect {|f| [ profile.identifier + '/' + f.full_name, f.id ] }))) %> | 12 | + <%= select_profile_folder(_('Choose folder to upload files:'), :parent_id, profile) %> |
| 14 | </div> | 13 | </div> |
| 15 | <p><%= file_field_tag('file1') %></p> | 14 | <p><%= file_field_tag('file1') %></p> |
| 16 | <p><%= file_field_tag('file2') %></p> | 15 | <p><%= file_field_tag('file2') %></p> |
app/views/cms/_upload_file_form.rhtml
| 1 | <% if @parent %> | 1 | <% if @parent %> |
| 2 | <%= hidden_field_tag('parent_id', @parent.id) %> | 2 | <%= hidden_field_tag('parent_id', @parent.id) %> |
| 3 | <% else %> | 3 | <% else %> |
| 4 | - <%= labelled_form_field(_('Choose folder to upload files:'), select_tag('parent_id', options_for_select([[profile.identifier, '']] + @folders.collect {|f| [ profile.identifier + '/' + f.full_name, f.id ] }))) %> | 4 | + <%= select_profile_folder(_('Choose folder to upload files:'), :parent_id, profile) %> |
| 5 | <% end %> | 5 | <% end %> |
| 6 | 6 | ||
| 7 | <div id='uploaded_files'> | 7 | <div id='uploaded_files'> |
app/views/cms/_uploaded_file.rhtml
| 1 | <%= labelled_form_field(_('Title'), text_field(:article, :title, :maxlength => 60)) %> | 1 | <%= labelled_form_field(_('Title'), text_field(:article, :title, :maxlength => 60)) %> |
| 2 | + | ||
| 3 | +<%= render :partial => 'general_fields' %> | ||
| 4 | + | ||
| 2 | <%= labelled_form_field(_('Description'), text_area(:article, :abstract, :rows => 3, :cols => 64)) %> | 5 | <%= labelled_form_field(_('Description'), text_area(:article, :abstract, :rows => 3, :cols => 64)) %> |
| 3 | <% if @article.image? %> | 6 | <% if @article.image? %> |
| 4 | <%= f.text_field(:external_link, :size => 64) %> | 7 | <%= f.text_field(:external_link, :size => 64) %> |
test/factories.rb
| @@ -424,7 +424,7 @@ module Noosfero::Factory | @@ -424,7 +424,7 @@ module Noosfero::Factory | ||
| 424 | 424 | ||
| 425 | def defaults_for_forum(params = {}) | 425 | def defaults_for_forum(params = {}) |
| 426 | name = "forum_#{rand(1000)}" | 426 | name = "forum_#{rand(1000)}" |
| 427 | - { :profile_id => 1, :path => name, :name => name, :slug => name.to_slug }.merge(params) | 427 | + { :profile_id => 1, :path => name.to_slug, :name => name, :slug => name.to_slug }.merge(params) |
| 428 | end | 428 | end |
| 429 | 429 | ||
| 430 | ############################################### | 430 | ############################################### |
| @@ -433,7 +433,7 @@ module Noosfero::Factory | @@ -433,7 +433,7 @@ module Noosfero::Factory | ||
| 433 | 433 | ||
| 434 | def defaults_for_gallery(params = {}) | 434 | def defaults_for_gallery(params = {}) |
| 435 | name = "gallery_#{rand(1000)}" | 435 | name = "gallery_#{rand(1000)}" |
| 436 | - { :profile_id => 1, :path => name, :name => name, :slug => name.to_slug }.merge(params) | 436 | + { :profile_id => 1, :path => name.to_slug, :name => name, :slug => name.to_slug }.merge(params) |
| 437 | end | 437 | end |
| 438 | 438 | ||
| 439 | def defaults_for_suggest_article | 439 | def defaults_for_suggest_article |
test/functional/cms_controller_test.rb
| @@ -1557,6 +1557,32 @@ class CmsControllerTest < ActionController::TestCase | @@ -1557,6 +1557,32 @@ class CmsControllerTest < ActionController::TestCase | ||
| 1557 | assert_equal license, article.license | 1557 | assert_equal license, article.license |
| 1558 | end | 1558 | end |
| 1559 | 1559 | ||
| 1560 | + should 'list folders options to move content' do | ||
| 1561 | + article = fast_create(Article, :profile_id => profile.id) | ||
| 1562 | + f1 = fast_create(Folder, :profile_id => profile.id) | ||
| 1563 | + f2 = fast_create(Folder, :profile_id => profile.id) | ||
| 1564 | + f3 = fast_create(Folder, :profile_id => profile, :parent_id => f2.id) | ||
| 1565 | + login_as(profile.identifier) | ||
| 1566 | + | ||
| 1567 | + get :edit, :profile => profile.identifier, :id => article.id | ||
| 1568 | + | ||
| 1569 | + assert_tag :tag => 'option', :attributes => {:value => f1.id}, :content => "#{profile.identifier}/#{f1.name}" | ||
| 1570 | + assert_tag :tag => 'option', :attributes => {:value => f2.id}, :content => "#{profile.identifier}/#{f2.name}" | ||
| 1571 | + assert_tag :tag => 'option', :attributes => {:value => f3.id}, :content => "#{profile.identifier}/#{f2.name}/#{f3.name}" | ||
| 1572 | + end | ||
| 1573 | + | ||
| 1574 | + should 'be able to move content' do | ||
| 1575 | + f1 = fast_create(Folder, :profile_id => profile.id) | ||
| 1576 | + f2 = fast_create(Folder, :profile_id => profile.id) | ||
| 1577 | + article = fast_create(Article, :profile_id => profile.id, :parent_id => f1) | ||
| 1578 | + login_as(profile.identifier) | ||
| 1579 | + | ||
| 1580 | + post :edit, :profile => profile.identifier, :id => article.id, :article => {:parent_id => f2.id} | ||
| 1581 | + article.reload | ||
| 1582 | + | ||
| 1583 | + assert_equal f2, article.parent | ||
| 1584 | + end | ||
| 1585 | + | ||
| 1560 | protected | 1586 | protected |
| 1561 | 1587 | ||
| 1562 | # FIXME this is to avoid adding an extra dependency for a proper JSON parser. | 1588 | # FIXME this is to avoid adding an extra dependency for a proper JSON parser. |
test/unit/article_test.rb
| @@ -1782,4 +1782,40 @@ class ArticleTest < ActiveSupport::TestCase | @@ -1782,4 +1782,40 @@ class ArticleTest < ActiveSupport::TestCase | ||
| 1782 | assert_equal license, article.license | 1782 | assert_equal license, article.license |
| 1783 | end | 1783 | end |
| 1784 | 1784 | ||
| 1785 | + should 'update path if parent is changed' do | ||
| 1786 | + f1 = Folder.create!(:name => 'Folder 1', :profile => profile) | ||
| 1787 | + f2 = Folder.create!(:name => 'Folder 2', :profile => profile) | ||
| 1788 | + article = TinyMceArticle.create!(:name => 'Sample Article', :parent_id => f1.id, :profile => profile) | ||
| 1789 | + assert_equal [f1.path,article.slug].join('/'), article.path | ||
| 1790 | + | ||
| 1791 | + article.parent = f2 | ||
| 1792 | + article.save! | ||
| 1793 | + assert_equal [f2.path,article.slug].join('/'), article.path | ||
| 1794 | + | ||
| 1795 | + article.parent = nil | ||
| 1796 | + article.save! | ||
| 1797 | + assert_equal article.slug, article.path | ||
| 1798 | + | ||
| 1799 | + article.update_attributes({:parent_id => f2.id}) | ||
| 1800 | + assert_equal [f2.path,article.slug].join('/'), article.path | ||
| 1801 | + end | ||
| 1802 | + | ||
| 1803 | + should 'not allow parent as itself' do | ||
| 1804 | + article = Article.create!(:name => 'Sample Article', :profile => profile) | ||
| 1805 | + article.parent = article | ||
| 1806 | + article.valid? | ||
| 1807 | + | ||
| 1808 | + assert article.errors.invalid?(:parent_id) | ||
| 1809 | + end | ||
| 1810 | + | ||
| 1811 | + should 'not allow cyclical paternity' do | ||
| 1812 | + a1 = Article.create!(:name => 'Sample Article 1', :profile => profile) | ||
| 1813 | + a2 = Article.create!(:name => 'Sample Article 2', :profile => profile, :parent => a1) | ||
| 1814 | + a3 = Article.create!(:name => 'Sample Article 3', :profile => profile, :parent => a2) | ||
| 1815 | + a1.parent = a3 | ||
| 1816 | + a1.valid? | ||
| 1817 | + | ||
| 1818 | + assert a1.errors.invalid?(:parent_id) | ||
| 1819 | + end | ||
| 1820 | + | ||
| 1785 | end | 1821 | end |