Commit 527a4b0d28e0814f04cf83b415cb77d90928245b
1 parent
b3fb1448
Exists in
master
and in
28 other branches
Moved logic from controller to model
- Also included new db/schema - Removed unused partial (ActionItem2513)
Showing
9 changed files
with
77 additions
and
14 deletions
Show diff stats
app/controllers/my_profile/cms_controller.rb
@@ -74,8 +74,6 @@ class CmsController < MyProfileController | @@ -74,8 +74,6 @@ class CmsController < MyProfileController | ||
74 | record_coming | 74 | record_coming |
75 | if request.post? | 75 | if request.post? |
76 | @article.image = nil if params[:remove_image] == 'true' | 76 | @article.image = nil if params[:remove_image] == 'true' |
77 | - @article.users_with_agreement.clear if (@article.forum? and params[:article][:has_terms_of_use] == "0") | ||
78 | - @article.users_with_agreement << user if (@article.forum? and params[:article][:has_terms_of_use] == "1" and !@article.users_with_agreement.include? user) | ||
79 | @article.last_changed_by = user | 77 | @article.last_changed_by = user |
80 | if @article.update_attributes(params[:article]) | 78 | if @article.update_attributes(params[:article]) |
81 | if !continue | 79 | if !continue |
@@ -132,7 +130,6 @@ class CmsController < MyProfileController | @@ -132,7 +130,6 @@ class CmsController < MyProfileController | ||
132 | 130 | ||
133 | continue = params[:continue] | 131 | continue = params[:continue] |
134 | if request.post? | 132 | if request.post? |
135 | - @article.users_with_agreement << user if (@article.forum? and params[:article][:has_terms_of_use] == "1") | ||
136 | if @article.save | 133 | if @article.save |
137 | if continue | 134 | if continue |
138 | redirect_to :action => 'edit', :id => @article | 135 | redirect_to :action => 'edit', :id => @article |
app/controllers/public/content_viewer_controller.rb
@@ -56,8 +56,7 @@ class ContentViewerController < ApplicationController | @@ -56,8 +56,7 @@ class ContentViewerController < ApplicationController | ||
56 | end | 56 | end |
57 | elsif !@page.parent.nil? && @page.parent.forum? | 57 | elsif !@page.parent.nil? && @page.parent.forum? |
58 | unless @page.parent.agrees_with_terms?(user) | 58 | unless @page.parent.agrees_with_terms?(user) |
59 | - params[:page].pop | ||
60 | - redirect_to :profile => params[:profile], :page => params[:page], :action => 'view_page' | 59 | + redirect_to @page.parent.url |
61 | end | 60 | end |
62 | end | 61 | end |
63 | 62 |
app/models/forum.rb
@@ -7,6 +7,17 @@ class Forum < Folder | @@ -7,6 +7,17 @@ class Forum < Folder | ||
7 | settings_items :has_terms_of_use, :type => :boolean, :default => false | 7 | settings_items :has_terms_of_use, :type => :boolean, :default => false |
8 | has_and_belongs_to_many :users_with_agreement, :class_name => 'Person', :join_table => 'terms_forum_people' | 8 | has_and_belongs_to_many :users_with_agreement, :class_name => 'Person', :join_table => 'terms_forum_people' |
9 | 9 | ||
10 | + before_save do |forum| | ||
11 | + if forum.has_terms_of_use | ||
12 | + last_editor = forum.profile.environment.people.find_by_id(forum.last_changed_by_id) | ||
13 | + if last_editor && !forum.users_with_agreement.exists?(last_editor) | ||
14 | + forum.users_with_agreement << last_editor | ||
15 | + end | ||
16 | + else | ||
17 | + forum.users_with_agreement.clear | ||
18 | + end | ||
19 | + end | ||
20 | + | ||
10 | def self.type_name | 21 | def self.type_name |
11 | _('Forum') | 22 | _('Forum') |
12 | end | 23 | end |
@@ -51,11 +62,8 @@ class Forum < Folder | @@ -51,11 +62,8 @@ class Forum < Folder | ||
51 | 62 | ||
52 | def agrees_with_terms?(user) | 63 | def agrees_with_terms?(user) |
53 | return true unless self.has_terms_of_use | 64 | return true unless self.has_terms_of_use |
54 | - if user | ||
55 | - self.users_with_agreement.find_by_id user.id | ||
56 | - else | ||
57 | - false | ||
58 | - end | 65 | + return false unless user |
66 | + self.users_with_agreement.exists? user | ||
59 | end | 67 | end |
60 | 68 | ||
61 | end | 69 | end |
app/views/content_viewer/_terms_of_use.rhtml
app/views/content_viewer/forum_page.rhtml
@@ -22,7 +22,7 @@ | @@ -22,7 +22,7 @@ | ||
22 | <% else %> | 22 | <% else %> |
23 | <%= button :save, _("Accept"), login_url %> | 23 | <%= button :save, _("Accept"), login_url %> |
24 | <% end %> | 24 | <% end %> |
25 | - <%= button "cancel", _("Cancel"), profile.url %> | 25 | + <%= button :cancel, _("Cancel"), profile.url %> |
26 | <% end %> | 26 | <% end %> |
27 | <% end %> | 27 | <% end %> |
28 | -<% end %> | ||
29 | \ No newline at end of file | 28 | \ No newline at end of file |
29 | +<% end %> |
db/schema.rb
@@ -9,7 +9,7 @@ | @@ -9,7 +9,7 @@ | ||
9 | # | 9 | # |
10 | # It's strongly recommended to check this file into your version control system. | 10 | # It's strongly recommended to check this file into your version control system. |
11 | 11 | ||
12 | -ActiveRecord::Schema.define(:version => 20131121162641) do | 12 | +ActiveRecord::Schema.define(:version => 20131128161159) do |
13 | 13 | ||
14 | create_table "abuse_reports", :force => true do |t| | 14 | create_table "abuse_reports", :force => true do |t| |
15 | t.integer "reporter_id" | 15 | t.integer "reporter_id" |
@@ -566,6 +566,13 @@ ActiveRecord::Schema.define(:version => 20131121162641) do | @@ -566,6 +566,13 @@ ActiveRecord::Schema.define(:version => 20131121162641) do | ||
566 | 566 | ||
567 | add_index "tasks", ["spam"], :name => "index_tasks_on_spam" | 567 | add_index "tasks", ["spam"], :name => "index_tasks_on_spam" |
568 | 568 | ||
569 | + create_table "terms_forum_people", :id => false, :force => true do |t| | ||
570 | + t.integer "forum_id" | ||
571 | + t.integer "person_id" | ||
572 | + end | ||
573 | + | ||
574 | + add_index "terms_forum_people", ["forum_id", "person_id"], :name => "index_terms_forum_people_on_forum_id_and_person_id" | ||
575 | + | ||
569 | create_table "thumbnails", :force => true do |t| | 576 | create_table "thumbnails", :force => true do |t| |
570 | t.integer "size" | 577 | t.integer "size" |
571 | t.string "content_type" | 578 | t.string "content_type" |
public/stylesheets/application.css
@@ -3338,6 +3338,9 @@ table.cms-articles .icon:hover { | @@ -3338,6 +3338,9 @@ table.cms-articles .icon:hover { | ||
3338 | div.with_media_panel .formfield input { | 3338 | div.with_media_panel .formfield input { |
3339 | width: 100%; | 3339 | width: 100%; |
3340 | } | 3340 | } |
3341 | +div.with_media_panel .formfield input[type="checkbox"] { | ||
3342 | + width: auto; | ||
3343 | +} | ||
3341 | 3344 | ||
3342 | .text-editor-sidebar { | 3345 | .text-editor-sidebar { |
3343 | position: absolute; | 3346 | position: absolute; |
test/functional/cms_controller_test.rb
@@ -1698,6 +1698,16 @@ class CmsControllerTest < ActionController::TestCase | @@ -1698,6 +1698,16 @@ class CmsControllerTest < ActionController::TestCase | ||
1698 | :attributes => { :value => article.id.to_s }} | 1698 | :attributes => { :value => article.id.to_s }} |
1699 | end | 1699 | end |
1700 | 1700 | ||
1701 | + should 'remove users that agreed with forum terms after removing terms' do | ||
1702 | + forum = Forum.create(:name => 'Forum test', :profile_id => profile.id, :has_terms_of_use => true) | ||
1703 | + person = fast_create(Person) | ||
1704 | + forum.users_with_agreement << person | ||
1705 | + | ||
1706 | + assert_difference Forum.find(forum.id).users_with_agreement, :count, -1 do | ||
1707 | + post :edit, :profile => profile.identifier, :id => forum.id, :article => { :has_terms_of_use => 'false' } | ||
1708 | + end | ||
1709 | + end | ||
1710 | + | ||
1701 | protected | 1711 | protected |
1702 | 1712 | ||
1703 | # FIXME this is to avoid adding an extra dependency for a proper JSON parser. | 1713 | # FIXME this is to avoid adding an extra dependency for a proper JSON parser. |
test/unit/forum_test.rb
@@ -133,4 +133,44 @@ class ForumTest < ActiveSupport::TestCase | @@ -133,4 +133,44 @@ class ForumTest < ActiveSupport::TestCase | ||
133 | assert_equal '', f.first_paragraph | 133 | assert_equal '', f.first_paragraph |
134 | end | 134 | end |
135 | 135 | ||
136 | + should 'include user that changes a forum as agreed with terms' do | ||
137 | + author = fast_create(Person) | ||
138 | + editor = fast_create(Person) | ||
139 | + forum = Forum.create(:profile => author, :name => 'Forum test', :body => 'Forum test', :has_terms_of_use => true, :last_changed_by => author) | ||
140 | + forum.last_changed_by = editor | ||
141 | + forum.save | ||
142 | + | ||
143 | + assert_equivalent [author, editor], forum.users_with_agreement | ||
144 | + end | ||
145 | + | ||
146 | + should 'not crash if forum has terms and last_changed does not exist' do | ||
147 | + profile = fast_create(Person) | ||
148 | + forum = Forum.create(:profile => profile, :name => 'Forum test', :body => 'Forum test', :has_terms_of_use => true) | ||
149 | + | ||
150 | + assert_equal [], forum.users_with_agreement | ||
151 | + end | ||
152 | + | ||
153 | + should 'agree with terms if forum doesn\'t have terms' do | ||
154 | + person = fast_create(Person) | ||
155 | + forum = fast_create(Forum) | ||
156 | + | ||
157 | + assert_equal true, forum.agrees_with_terms?(person) | ||
158 | + end | ||
159 | + | ||
160 | + should 'not agree with terms if user is logged in but did not accept it' do | ||
161 | + person = fast_create(Person) | ||
162 | + forum = Forum.create(:profile => person, :name => 'Forum test', :body => 'Forum test', :has_terms_of_use => true) | ||
163 | + | ||
164 | + assert_equal false, forum.agrees_with_terms?(person) | ||
165 | + end | ||
166 | + | ||
167 | + should 'agree with terms if user is logged in and accept it' do | ||
168 | + person = fast_create(Person) | ||
169 | + forum = Forum.create(:profile => person, :name => 'Forum test', :body => 'Forum test', :has_terms_of_use => true) | ||
170 | + forum.users_with_agreement << person | ||
171 | + forum.save | ||
172 | + | ||
173 | + assert_equal true, Forum.find(forum.id).agrees_with_terms?(person) | ||
174 | + end | ||
175 | + | ||
136 | end | 176 | end |