Merge Request #4
New feature: Restore a rejected or finished proposal
Added new button into processed tasks page to allow the admin restore
1 or N proposals to initial status.
In the last commit, was made a refactor to add a relation between tasks
and articles
entities. The article related with a proposal task is disabled when the task returns to their initial status!!
@@ -78,4 +78,14 @@ class ProposalsDiscussionPluginTasksController < TasksController | @@ -78,4 +78,14 @@ class ProposalsDiscussionPluginTasksController < TasksController | ||
78 | 78 | ||
79 | end | 79 | end |
80 | 80 | ||
81 | + def undo_flags | ||
82 | + if params[:tasks].present? | ||
83 | + result = ProposalsDiscussionPlugin::ProposalTask.undo_flags params[:tasks], current_user | ||
84 | + unless result | ||
85 | + session[:notice] = _("Error to undo flags. Please, verify with the admin") | ||
86 | + end | ||
87 | + end | ||
88 | + redirect_to :controller => 'tasks', :action => 'processed', :filter => {type: 'ProposalsDiscussionPlugin::ProposalTask'} | ||
89 | + end | ||
90 | + | ||
81 | end | 91 | end |
@@ -0,0 +1,28 @@ | @@ -0,0 +1,28 @@ | ||
1 | +class AddArticleRefToTasks < ActiveRecord::Migration | ||
2 | + | ||
3 | + def up | ||
4 | + add_reference :tasks, :article, index: true, foreign_key: true, on_delete: :nullify | ||
5 | + ProposalsDiscussionPlugin::ProposalTask.find_each do |task| | ||
6 | + | ||
7 | + if task.data[:article] | ||
8 | + field = {name: task.data[:article][:name]} | ||
9 | + if task.data[:article][:id] | ||
10 | + field = {id: task.data[:article][:id]} | ||
11 | + end | ||
12 | + article = Article.find_by field | ||
13 | + | ||
14 | + if article.nil? | ||
15 | + data = task.data[:article].merge({profile: task.target}).except(:type) | ||
16 | + article = Article.create!(data) | ||
17 | + end | ||
18 | + task.update_column(:article_id, article.id) | ||
19 | + end | ||
20 | + | ||
21 | + end | ||
22 | + end | ||
23 | + | ||
24 | + def down | ||
25 | + remove_reference :tasks, :article, index: true, foreign_key: true | ||
26 | + end | ||
27 | + | ||
28 | +end |
@@ -2,6 +2,8 @@ require_dependency 'article' | @@ -2,6 +2,8 @@ require_dependency 'article' | ||
2 | 2 | ||
3 | class Article | 3 | class Article |
4 | 4 | ||
5 | + has_one :task | ||
6 | + | ||
5 | def ranking_position | 7 | def ranking_position |
6 | self.kind_of?(ProposalsDiscussionPlugin::Proposal) && self.ranking_item.present? ? self.ranking_item.position : nil | 8 | self.kind_of?(ProposalsDiscussionPlugin::Proposal) && self.ranking_item.present? ? self.ranking_item.position : nil |
7 | end | 9 | end |
@@ -11,7 +11,7 @@ module Noosfero | @@ -11,7 +11,7 @@ module Noosfero | ||
11 | end | 11 | end |
12 | end | 12 | end |
13 | 13 | ||
14 | - class RankingItem < Entity | 14 | + class RankingItem < Grape::Entity |
15 | root :proposals, :proposal | 15 | root :proposals, :proposal |
16 | expose :id, :position, :abstract, :body, :votes_for, :votes_against | 16 | expose :id, :position, :abstract, :body, :votes_for, :votes_against |
17 | expose :hits, :effective_support, :proposal_id, :created_at | 17 | expose :hits, :effective_support, :proposal_id, :created_at |
@@ -7,6 +7,7 @@ class ProposalsDiscussionPlugin::ProposalTask < Task | @@ -7,6 +7,7 @@ class ProposalsDiscussionPlugin::ProposalTask < Task | ||
7 | association_foreign_key: :category_id | 7 | association_foreign_key: :category_id |
8 | 8 | ||
9 | has_one :proposal_evaluation | 9 | has_one :proposal_evaluation |
10 | + belongs_to :article_obj, class_name: 'Article', foreign_key:'article_id' | ||
10 | 11 | ||
11 | validates_presence_of :requestor_id, :target_id | 12 | validates_presence_of :requestor_id, :target_id |
12 | validates_associated :article_object | 13 | validates_associated :article_object |
@@ -20,6 +21,8 @@ class ProposalsDiscussionPlugin::ProposalTask < Task | @@ -20,6 +21,8 @@ class ProposalsDiscussionPlugin::ProposalTask < Task | ||
20 | settings_items :article, :type => Hash, :default => {} | 21 | settings_items :article, :type => Hash, :default => {} |
21 | settings_items :closing_statment, :article_parent_id | 22 | settings_items :closing_statment, :article_parent_id |
22 | 23 | ||
24 | + attr_accessible :requestor, :target, :article, :article_obj, :article_parent_id, :status, :end_date, :closed_by | ||
25 | + | ||
23 | 26 | ||
24 | scope :pending_evaluated, lambda { |profile, filter_type, filter_text| | 27 | scope :pending_evaluated, lambda { |profile, filter_type, filter_text| |
25 | self | 28 | self |
@@ -128,6 +131,30 @@ class ProposalsDiscussionPlugin::ProposalTask < Task | @@ -128,6 +131,30 @@ class ProposalsDiscussionPlugin::ProposalTask < Task | ||
128 | end | 131 | end |
129 | end | 132 | end |
130 | 133 | ||
134 | + def self.undo_flags(tasks, user) | ||
135 | + self.disable_article(tasks) | ||
136 | + | ||
137 | + fields = "status = #{Task::Status::ACTIVE}, end_date = NULL, closed_by_id = #{user.id}" | ||
138 | + conditions = "status != #{Task::Status::ACTIVE}" | ||
139 | + | ||
140 | + result = self.where(conditions).update_all(fields, ["id IN (?)",tasks]) | ||
141 | + | ||
142 | + end | ||
143 | + | ||
144 | + def undo_flags(user) | ||
145 | + if flagged? | ||
146 | + if flagged_for_approval? | ||
147 | + self.article_obj.published = false | ||
148 | + self.article_obj.save! | ||
149 | + end | ||
150 | + | ||
151 | + self.status = Task::Status::ACTIVE | ||
152 | + self.end_date = nil | ||
153 | + self.closed_by = user | ||
154 | + | ||
155 | + self.save! | ||
156 | + end | ||
157 | + end | ||
131 | 158 | ||
132 | def schedule_spam_checking | 159 | def schedule_spam_checking |
133 | self.delay.check_for_spam | 160 | self.delay.check_for_spam |
@@ -137,6 +164,13 @@ class ProposalsDiscussionPlugin::ProposalTask < Task | @@ -137,6 +164,13 @@ class ProposalsDiscussionPlugin::ProposalTask < Task | ||
137 | requestor.name if requestor | 164 | requestor.name if requestor |
138 | end | 165 | end |
139 | 166 | ||
167 | + def self.disable_article(tasks,id = nil) | ||
168 | + | ||
169 | + conditions = "tasks.status = #{Status::FLAGGED_FOR_APPROVAL} OR tasks.status = #{Task::Status::FINISHED}" | ||
170 | + Article.joins(:task).where(conditions).update_all('published = false, published_at = NULL', ["tasks.id IN (?)",tasks]) | ||
171 | + | ||
172 | + end | ||
173 | + | ||
140 | def article_parent=(parent) | 174 | def article_parent=(parent) |
141 | @article_parent = parent | 175 | @article_parent = parent |
142 | end | 176 | end |
@@ -146,12 +180,15 @@ class ProposalsDiscussionPlugin::ProposalTask < Task | @@ -146,12 +180,15 @@ class ProposalsDiscussionPlugin::ProposalTask < Task | ||
146 | end | 180 | end |
147 | 181 | ||
148 | def article_object | 182 | def article_object |
149 | - if @article_object.nil? | ||
150 | - @article_object = article_type.new(article.merge(target.present? ? {:profile => target} : {}).except(:type)) | ||
151 | - @article_object.parent = article_parent | ||
152 | - @article_object.author = requestor if requestor.present? | 183 | + if self.article_obj.nil? |
184 | + self.article_obj = article_type.new(article.merge(target.present? ? {:profile => target} : {}).except(:type)) | ||
185 | + self.article_obj.parent = article_parent | ||
186 | + self.article_obj.author = requestor if requestor.present? | ||
187 | + else | ||
188 | + self.article_obj.published = true | ||
189 | + self.article_obj.published_at = Time.now | ||
153 | end | 190 | end |
154 | - @article_object | 191 | + self.article_obj |
155 | end | 192 | end |
156 | 193 | ||
157 | def article_type | 194 | def article_type |
@@ -163,8 +200,9 @@ class ProposalsDiscussionPlugin::ProposalTask < Task | @@ -163,8 +200,9 @@ class ProposalsDiscussionPlugin::ProposalTask < Task | ||
163 | end | 200 | end |
164 | 201 | ||
165 | def perform | 202 | def perform |
166 | - article_object.save! | ||
167 | - self.data[:article][:id] = article_object[:id] | 203 | + self.article_obj = article_object |
204 | + self.article_obj.save! | ||
205 | + self.data[:article][:id] = self.article_obj.id | ||
168 | self.save! | 206 | self.save! |
169 | end | 207 | end |
170 | 208 |
@@ -77,6 +77,89 @@ class ProposalTaskTest < ActiveSupport::TestCase | @@ -77,6 +77,89 @@ class ProposalTaskTest < ActiveSupport::TestCase | ||
77 | assert_equal person2, task.responsible | 77 | assert_equal person2, task.responsible |
78 | end | 78 | end |
79 | 79 | ||
80 | + should 'undo flags from one or more proposal tasks' do | ||
81 | + role1 = Role.create!(:name => 'profile_role2', :permissions => ['perform_task'], :environment => Environment.default) | ||
82 | + role2 = Role.create!(:name => 'profile_role', :permissions => ['view_tasks'], :environment => Environment.default) | ||
83 | + | ||
84 | + person1 = fast_create(Person) | ||
85 | + person1.define_roles([role1], profile) | ||
86 | + person2 = fast_create(Person) | ||
87 | + person2.define_roles([role2], profile) | ||
88 | + | ||
89 | + task = ProposalsDiscussionPlugin::ProposalTask.create!(:requestor => person, :target => profile, :article => {:name => 'proposal 1', :abstract => 'proposal 1'}) | ||
90 | + task_two = ProposalsDiscussionPlugin::ProposalTask.create!(:requestor => person, :target => profile, :article => {:name => 'proposal 2', :abstract => 'proposal 2'}) | ||
91 | + task.categories = task_two.categories = [fast_create(ProposalsDiscussionPlugin::TaskCategory)] | ||
92 | + task.flag_accept_proposal(person2) | ||
93 | + task_two.flag_accept_proposal(person2) | ||
94 | + assert task.flagged? | ||
95 | + assert task_two.flagged? | ||
96 | + | ||
97 | + task.perform | ||
98 | + task_two.perform | ||
99 | + | ||
100 | + ProposalsDiscussionPlugin::ProposalTask.undo_flags([task.id,task_two.id], person) | ||
101 | + task.reload | ||
102 | + task_two.reload | ||
103 | + | ||
104 | + assert_equal [false,false], [task.flagged?,task_two.flagged?] | ||
105 | + assert_equal [Task::Status::ACTIVE,Task::Status::ACTIVE], [task.status, task_two.status] | ||
106 | + assert_equal [false,false], [task.article_obj.published,task_two.article_obj.published] | ||
107 | + end | ||
108 | + | ||
109 | + should 'undo flags from a specific proposal task' do | ||
110 | + role1 = Role.create!(:name => 'profile_role2', :permissions => ['perform_task'], :environment => Environment.default) | ||
111 | + role2 = Role.create!(:name => 'profile_role', :permissions => ['view_tasks'], :environment => Environment.default) | ||
112 | + | ||
113 | + person1 = fast_create(Person) | ||
114 | + person1.define_roles([role1], profile) | ||
115 | + person2 = fast_create(Person) | ||
116 | + person2.define_roles([role2], profile) | ||
117 | + | ||
118 | + task = ProposalsDiscussionPlugin::ProposalTask.create!(:requestor => person, :target => profile, :article => {:name => 'proposal', :abstract => 'proposal'}) | ||
119 | + task.categories = [fast_create(ProposalsDiscussionPlugin::TaskCategory)] | ||
120 | + task.flag_accept_proposal(person1) | ||
121 | + assert task.flagged? | ||
122 | + | ||
123 | + task.perform | ||
124 | + | ||
125 | + task.undo_flags(person) | ||
126 | + task.reload | ||
127 | + | ||
128 | + assert_equal false, task.flagged? | ||
129 | + assert_equal Task::Status::ACTIVE, task.status | ||
130 | + assert_equal false, task.article_obj.published | ||
131 | + | ||
132 | + end | ||
133 | + | ||
134 | + should 'redo flags from a specific proposal task' do | ||
135 | + role1 = Role.create!(:name => 'profile_role2', :permissions => ['perform_task'], :environment => Environment.default) | ||
136 | + role2 = Role.create!(:name => 'profile_role', :permissions => ['view_tasks'], :environment => Environment.default) | ||
137 | + | ||
138 | + person1 = fast_create(Person) | ||
139 | + person1.define_roles([role1], profile) | ||
140 | + person2 = fast_create(Person) | ||
141 | + person2.define_roles([role2], profile) | ||
142 | + | ||
143 | + task = ProposalsDiscussionPlugin::ProposalTask.create!(:requestor => person, :target => profile, :article => {:name => 'proposal', :abstract => 'proposal'}) | ||
144 | + task.categories = [fast_create(ProposalsDiscussionPlugin::TaskCategory)] | ||
145 | + task.flag_accept_proposal(person1) | ||
146 | + assert task.flagged? | ||
147 | + | ||
148 | + task.perform | ||
149 | + | ||
150 | + task.undo_flags(person) | ||
151 | + task.reload | ||
152 | + | ||
153 | + assert_equal false, task.flagged? | ||
154 | + assert_equal Task::Status::ACTIVE, task.status | ||
155 | + assert_equal false, task.article_obj.published | ||
156 | + | ||
157 | + task.flag_accept_proposal(person1) | ||
158 | + task.perform | ||
159 | + | ||
160 | + assert_equal true, task.article_obj.published | ||
161 | + end | ||
162 | + | ||
80 | should 'do not fail on task information with integer as abstract' do | 163 | should 'do not fail on task information with integer as abstract' do |
81 | task = ProposalsDiscussionPlugin::ProposalTask.new | 164 | task = ProposalsDiscussionPlugin::ProposalTask.new |
82 | task.expects(:abstract).returns(49) | 165 | task.expects(:abstract).returns(49) |
@@ -86,9 +169,10 @@ class ProposalTaskTest < ActiveSupport::TestCase | @@ -86,9 +169,10 @@ class ProposalTaskTest < ActiveSupport::TestCase | ||
86 | should 'create a proposal with category when accept a task' do | 169 | should 'create a proposal with category when accept a task' do |
87 | c1 = fast_create(Category) | 170 | c1 = fast_create(Category) |
88 | discussion.categories << c1 | 171 | discussion.categories << c1 |
89 | - task = ProposalsDiscussionPlugin::ProposalTask.create!(:requestor => person, :target => profile, :article_parent_id => @discussion.id, :article => {:name => 'proposal 1', :abstract => 'proposal 1', :type => "ProposalsDiscussionPlugin::Proposal"}) | 172 | + |
173 | + task = ProposalsDiscussionPlugin::ProposalTask.create!(:requestor => person, :target => profile, :article_parent_id => @discussion.id, :article => {:name => 'proposal 1', :abstract => 'proposal 1', :type => 'ProposalsDiscussionPlugin::Proposal'}) | ||
90 | task.perform | 174 | task.perform |
91 | - assert_equal [c1], ProposalsDiscussionPlugin::Proposal.last.categories | 175 | + assert_equal c1, ProposalsDiscussionPlugin::Proposal.last.categories.first |
92 | end | 176 | end |
93 | 177 | ||
94 | end | 178 | end |
@@ -16,8 +16,7 @@ class TaskCategoryTest < ActiveSupport::TestCase | @@ -16,8 +16,7 @@ class TaskCategoryTest < ActiveSupport::TestCase | ||
16 | task_data = { | 16 | task_data = { |
17 | article: {name: "test proposal", abstract: "teste adadd"}, | 17 | article: {name: "test proposal", abstract: "teste adadd"}, |
18 | requestor: person, | 18 | requestor: person, |
19 | - target: profile, | ||
20 | - spam: false | 19 | + target: profile |
21 | } | 20 | } |
22 | 21 | ||
23 | task = ProposalsDiscussionPlugin::ProposalTask.new task_data | 22 | task = ProposalsDiscussionPlugin::ProposalTask.new task_data |
@@ -35,8 +34,7 @@ class TaskCategoryTest < ActiveSupport::TestCase | @@ -35,8 +34,7 @@ class TaskCategoryTest < ActiveSupport::TestCase | ||
35 | task_data = { | 34 | task_data = { |
36 | article: {name: "test proposal", abstract: "teste adadd"}, | 35 | article: {name: "test proposal", abstract: "teste adadd"}, |
37 | requestor: person, | 36 | requestor: person, |
38 | - target: profile, | ||
39 | - spam: false | 37 | + target: profile |
40 | } | 38 | } |
41 | 39 | ||
42 | task = ProposalsDiscussionPlugin::ProposalTask.create! task_data | 40 | task = ProposalsDiscussionPlugin::ProposalTask.create! task_data |
@@ -51,8 +49,7 @@ class TaskCategoryTest < ActiveSupport::TestCase | @@ -51,8 +49,7 @@ class TaskCategoryTest < ActiveSupport::TestCase | ||
51 | task_data = { | 49 | task_data = { |
52 | article: {name: "test proposal", abstract: "teste adadd"}, | 50 | article: {name: "test proposal", abstract: "teste adadd"}, |
53 | requestor: person, | 51 | requestor: person, |
54 | - target: profile, | ||
55 | - spam: false | 52 | + target: profile |
56 | } | 53 | } |
57 | task = ProposalsDiscussionPlugin::ProposalTask.create! task_data | 54 | task = ProposalsDiscussionPlugin::ProposalTask.create! task_data |
58 | evaluated_by = false | 55 | evaluated_by = false |
@@ -0,0 +1,58 @@ | @@ -0,0 +1,58 @@ | ||
1 | +<%= stylesheet_link_tag 'tasks' %> | ||
2 | + | ||
3 | +<div class="task-processed"> | ||
4 | +<h1><%= _("%s's processed tasks") % profile.name %></h1> | ||
5 | + | ||
6 | +<div class="task-processed-filter"> | ||
7 | +<% | ||
8 | + type_collection = [[nil, _('All')]] + @task_types | ||
9 | +%> | ||
10 | + <%= form_tag '#', :method => 'get' do %> | ||
11 | + <%= field_set_tag _('Filter'), :class => 'filter_fields' do %> | ||
12 | + <div> | ||
13 | + <%= labelled_select(_('Type of task')+': ', 'filter[type]', :first, :last, @filter[:type], type_collection, {:id => 'filter-type'}) %> | ||
14 | + <%= labelled_select(_('Status:'), 'filter[status]', :last, :first, @filter[:status], [[_('Any'), nil], [_(Task::Status.names[Task::Status::CANCELLED]), 2], [_(Task::Status.names[Task::Status::FINISHED]), 3] ]) %> | ||
15 | + </div> | ||
16 | + | ||
17 | + <div> | ||
18 | + <%= labelled_text_field(_('Text Filter:'), 'filter[text]', @filter[:text]) %> | ||
19 | + </div> | ||
20 | + | ||
21 | + <div> | ||
22 | + <%= labelled_text_field(_('Requestor:'), 'filter[requestor]', @filter[:requestor]) %> | ||
23 | + <%= labelled_text_field(_('Closed by:'), 'filter[closed_by]', @filter[:closed_by]) %> | ||
24 | + </div> | ||
25 | + | ||
26 | + <%= labelled_form_field(_('Creation date'), date_range_field('filter[created_from]', 'filter[created_until]', @filter[:created_from], @filter[:created_until], '%Y-%m-%d', { :change_month => true, :change_year => true, :date_format => 'yy-mm-dd' }, { :size => 14, :from_id => 'filter_created_from', :to_id => 'filter_created_until' })) %> | ||
27 | + <%= labelled_form_field(_('Processed date'), date_range_field('filter[closed_from]', 'filter[closed_until]', @filter[:closed_from], @filter[:closed_until], '%Y-%m-%d', { :change_month => true, :change_year => true, :date_format => 'yy-mm-dd' }, { :size => 14, :from_id => 'filter_closed_from', :to_id => 'filter_closed_until' })) %> | ||
28 | + | ||
29 | + <div class="actions"> | ||
30 | + <%= submit_button(:search, _('Search')) %> | ||
31 | + </div> | ||
32 | + <% end %> | ||
33 | + <% end %> | ||
34 | +</div> | ||
35 | + | ||
36 | +<p> | ||
37 | +<% if @tasks.empty? %> | ||
38 | + <em><%= _('No processed tasks.') %></em> | ||
39 | +<% else %> | ||
40 | + <%= form_tag :controller => :proposals_discussion_plugin_tasks, :action => 'undo_flags', :method => 'post' do %> | ||
41 | + <ul class="task-list"> | ||
42 | + <% @tasks.each do |item| %> | ||
43 | + <li class="task status-<%= item.status%>"> | ||
44 | + <%= render :partial => partial_for_class(item.class, nil, 'processed'), :locals => {:task => item} %> | ||
45 | + </li> | ||
46 | + <% end %> | ||
47 | + </ul> | ||
48 | + <%= pagination_links(@tasks)%> | ||
49 | + <input type="submit" name="commit" value="Restore" class="button with-text icon-child-article submit"> | ||
50 | + <% end %> | ||
51 | +<% end %> | ||
52 | +</p> | ||
53 | + | ||
54 | +<% button_bar do %> | ||
55 | + <%= button(:back, _('Back'), :action => 'index') %> | ||
56 | +<% end %> | ||
57 | + | ||
58 | +</div> |
@@ -2,6 +2,7 @@ | @@ -2,6 +2,7 @@ | ||
2 | <b><%= _('Source') %>:</b> <%= task.proposal_source %> | 2 | <b><%= _('Source') %>:</b> <%= task.proposal_source %> |
3 | </div> | 3 | </div> |
4 | <div class="title"> | 4 | <div class="title"> |
5 | + <input type="checkbox" name="tasks[]" value="<%=task.id%>" | ||
5 | <%= task_information(task) %> | 6 | <%= task_information(task) %> |
6 | </div> | 7 | </div> |
7 | <div class="status"> | 8 | <div class="status"> |
-
I just think some error handling is missing.
We would just check for error in the controller call to the method ProposalTask #undo_flags
The method #undo_flag on ProposalTask isn't being used
-
Hello @abner. I made this changes that you suggested. The method
#undo_flag
was renamed toundo_flags
to remove flag of a specific proposal task or a array of tasks.So, you can use like this:
ProposalDiscussionPlugin::ProposalTask.undo_flags()
ortask.undo_flags()
.The unit tests for both situations was changed/created.
Please, verify if this feature it's ok now! :)