Skip to content

Commit

Permalink
Revert "[#2230] Removed answered questions and the 'opened' field fro…
Browse files Browse the repository at this point in the history
…m the database"

This reverts commit da135d4.

Conflicts:

	lib/question_journal_hooks.rb
	spec/lib/question_journal_hooks_spec.rb
  • Loading branch information
edavis10 committed Sep 9, 2009
1 parent aa85229 commit f5b4dcc
Show file tree
Hide file tree
Showing 13 changed files with 78 additions and 47 deletions.
2 changes: 1 addition & 1 deletion README.rdoc
Expand Up @@ -9,7 +9,7 @@ The Questions plugin will improve the workflow of Redmine by allowing users to a
* Filters for the issue list:
* Question is assigned to
* Question was asked by
* Question column for the Issue list showing a preview of all the questions asked on an issue
* Question column for the Issue list showing a preview of all the open questions asked on an issue
* Email notification when questions are asked and answered

== Getting the plugin
Expand Down
2 changes: 1 addition & 1 deletion app/models/journal_questions_observer.rb
Expand Up @@ -9,7 +9,7 @@ def after_create(journal)
QuestionMailer.deliver_asked_question(journal)
end

# Close any questions
# Close any open questions
if journal.issue && journal.issue.pending_question?(journal.user)
journal.issue.close_pending_questions(journal.user, journal)
end
Expand Down
7 changes: 5 additions & 2 deletions app/models/question.rb
Expand Up @@ -15,7 +15,10 @@ def for_anyone?
end

def close!(closing_journal=nil)
QuestionMailer.deliver_answered_question(self, closing_journal) if closing_journal
self.destroy
if self.opened
self.opened = false
self.save!
QuestionMailer.deliver_answered_question(self, closing_journal) if closing_journal
end
end
end
9 changes: 5 additions & 4 deletions lib/question_issue_hooks.rb
Expand Up @@ -2,7 +2,7 @@ class QuestionIssueHooks < QuestionHooksBase
# Applies the question class to each journal div if they are questions
def view_issues_history_journal_bottom(context = { })
o = ''
if context[:journal] && context[:journal].question
if context[:journal] && context[:journal].question && context[:journal].question.opened?
question = context[:journal].question

if question.assigned_to
Expand Down Expand Up @@ -60,12 +60,13 @@ def controller_issues_edit_before_save(context = { })
def view_issues_sidebar_issues_bottom(context = { })
project = context[:project]
if project
question_count = Question.count(:conditions => ["#{Question.table_name}.assigned_to_id = ? AND #{Project.table_name}.id = ?",
question_count = Question.count(:conditions => ["#{Question.table_name}.assigned_to_id = ? AND #{Project.table_name}.id = ? AND #{Question.table_name}.opened = ?",
User.current,
project.id],
project.id,
true],
:include => [:issue => [:project]])
else
question_count = Question.count(:conditions => {:assigned_to_id => User.current})
question_count = Question.count(:conditions => {:assigned_to_id => User.current, :opened => true})
end

if question_count > 0
Expand Down
5 changes: 3 additions & 2 deletions lib/question_issue_patch.rb
Expand Up @@ -9,6 +9,7 @@ def self.included(base) # :nodoc:
base.class_eval do
unloadable # Send unloadable so it will not be unloaded in development
has_many :questions
has_many :open_questions, :class_name => 'Question', :conditions => { :opened => true }

class << self
# I dislike alias method chain, it's not the most readable backtraces
Expand Down Expand Up @@ -78,14 +79,14 @@ def add_questions_to_the_includes(arg)

module InstanceMethods
def pending_question?(user)
self.questions.find(:all).each do |question|
self.open_questions.find(:all).each do |question|
return true if question.assigned_to == user || question.for_anyone?
end
return false
end

def close_pending_questions(user, closing_journal)
self.questions.find(:all).each do |question|
self.open_questions.find(:all).each do |question|
question.close!(closing_journal) if question.assigned_to == user || question.for_anyone?
end
end
Expand Down
10 changes: 7 additions & 3 deletions lib/question_journal_hooks.rb
@@ -1,7 +1,7 @@
class QuestionJournalHooks < QuestionHooksBase
def view_journals_notes_form_after_notes(context = { })
@journal = context[:journal]
if @journal.question && @journal.question.assigned_to
if @journal.question && @journal.question.opened && @journal.question.assigned_to
assigned_to = @journal.question.assigned_to.login
else
assigned_to = ''
Expand All @@ -27,13 +27,17 @@ def controller_journals_edit_post(context = { })
if journal.question && params[:question][:assigned_to].blank?
# Wants to remove the question
journal.question.destroy
elsif journal.question
elsif journal.question && journal.question.opened
# Reassignment
if params[:question][:assigned_to].downcase == 'anyone'
journal.question.update_attributes(:assigned_to => nil)
else
journal.question.update_attributes(:assigned_to => User.find_by_login(params[:question][:assigned_to]))
end
elsif journal.question && !journal.question.opened
# Existing question, destry it first and then add a new question
journal.question.destroy
add_new_question(journal, User.find_by_login(params[:question][:assigned_to]))
else
if params[:question][:assigned_to].downcase == 'anyone'
add_new_question(journal)
Expand All @@ -54,7 +58,7 @@ def view_journals_update_rjs_bottom(context = { })
page = context[:page]
unless @journal.frozen?
@journal.reload
if @journal && @journal.question
if @journal && @journal.question && @journal.question.opened?
question = @journal.question

if question.assigned_to
Expand Down
2 changes: 1 addition & 1 deletion lib/question_queries_helper_patch.rb
Expand Up @@ -18,7 +18,7 @@ module ClassMethods
module InstanceMethods
def question_column_content(column, issue)
if column.name == :formatted_questions
return format_questions(issue.questions)
return format_questions(issue.open_questions)
else
default_column_content(column, issue)
end
Expand Down
4 changes: 2 additions & 2 deletions lib/question_query_patch.rb
Expand Up @@ -77,9 +77,9 @@ def question_sql_for_field(field, operator, v, db_table, db_field, is_custom_fil

case operator
when "="
sql = "#{db_table}.#{db_field} IN (" + v.collect{|val| "'#{connection.quote_string(val)}'"}.join(",") + ")"
sql = "#{db_table}.#{db_field} IN (" + v.collect{|val| "'#{connection.quote_string(val)}'"}.join(",") + ") AND #{db_table}.opened = true"
when "!"
sql = "(#{db_table}.#{db_field} IS NULL OR #{db_table}.#{db_field} NOT IN (" + v.collect{|val| "'#{connection.quote_string(val)}'"}.join(",") + "))"
sql = "(#{db_table}.#{db_field} IS NULL OR #{db_table}.#{db_field} NOT IN (" + v.collect{|val| "'#{connection.quote_string(val)}'"}.join(",") + ")) AND #{db_table}.opened = true"
end

return sql
Expand Down
4 changes: 2 additions & 2 deletions spec/lib/question_issue_hooks_spec.rb
Expand Up @@ -102,7 +102,7 @@ def call_hook(context)
describe QuestionIssueHooks, 'view_issues_history_journal_bottom with a journal and question' do
before(:each) do
@user = mock_model(User, :to_s => "A user", :mail => 'user@example.com')
@question = mock_model(Question, :assigned_to => @user)
@question = mock_model(Question, :assigned_to => @user, :opened? => true)
@context = {
:journal => mock_model(Journal, :question => @question)
}
Expand Down Expand Up @@ -187,7 +187,7 @@ def call_hook(context)
end

it 'should get the number of questions for the current user on all projects' do
Question.should_receive(:count).with(:conditions => { :assigned_to_id => @user}).and_return(10)
Question.should_receive(:count).with(:conditions => { :assigned_to_id => @user, :opened => true}).and_return(10)
call_hook(@context)
end

Expand Down
30 changes: 15 additions & 15 deletions spec/lib/question_issue_patch_spec.rb
Expand Up @@ -11,94 +11,94 @@
@user = mock_model(User)
end

it 'should return false if there are no questions' do
it 'should return false if there are no open questions' do
question_mock = mock('question_mock')
question_mock.stub!(:find).and_return([])
@issue = Issue.new
@issue.should_receive(:questions).and_return(question_mock)
@issue.should_receive(:open_questions).and_return(question_mock)
@issue.pending_question?(@user).should be_false
end

it 'should return false if there are no questions for the current user' do
it 'should return false if there are no open questions for the current user' do
@other_user = mock_model(User)
question_one = mock_model(Question, :assigned_to => @other_user, :for_anyone? => false)
question_two = mock_model(Question, :assigned_to => @other_user, :for_anyone? => false)
question_mock = mock('question_mock')
question_mock.should_receive(:find).and_return([question_one, question_two])

@issue = Issue.new
@issue.should_receive(:questions).and_return(question_mock)
@issue.should_receive(:open_questions).and_return(question_mock)
@issue.pending_question?(@user).should be_false
end

it 'should return true if there is an question for the current user' do
it 'should return true if there is an open question for the current user' do
@other_user = mock_model(User)
question_one = mock_model(Question, :assigned_to => @other_user, :for_anyone? => false)
question_two = mock_model(Question, :assigned_to => @user, :for_anyone? => false)
question_mock = mock('question_mock')
question_mock.should_receive(:find).and_return([question_one, question_two])

@issue = Issue.new
@issue.should_receive(:questions).and_return(question_mock)
@issue.should_receive(:open_questions).and_return(question_mock)
@issue.pending_question?(@user).should be_true

end

it 'should return true if there is a question for anyone' do
it 'should return true if there is an open question for anyone' do
@other_user = mock_model(User)
question_one = mock_model(Question, :assigned_to => @other_user, :for_anyone? => false)
question_two = mock_model(Question, :assigned_to => nil, :for_anyone? => true)
question_mock = mock('question_mock')
question_mock.should_receive(:find).and_return([question_one, question_two])

@issue = Issue.new
@issue.should_receive(:questions).and_return(question_mock)
@issue.should_receive(:open_questions).and_return(question_mock)
@issue.pending_question?(@user).should be_true

end
end

describe QuestionIssuePatch,"#close_pending_questions" do
it 'should close any questions for user' do
it 'should close any open questions for user' do
@user = mock_model(User)
@journal = mock_model(Journal)
question = mock_model(Question, :assigned_to => @user)
question = mock_model(Question, :opened => true, :assigned_to => @user)
question.should_receive(:close!).and_return(question)
question_mock = mock('question_mock')
question_mock.should_receive(:find).and_return([question])


@issue = Issue.new
@issue.should_receive(:questions).and_return(question_mock)
@issue.should_receive(:open_questions).and_return(question_mock)
@issue.close_pending_questions(@user, @journal)
end

it 'should close any questions for anyone' do
@user = mock_model(User)
@journal = mock_model(Journal)
question = mock_model(Question, :assigned_to => nil, :for_anyone? => true)
question = mock_model(Question, :opened => true, :assigned_to => nil, :for_anyone? => true)
question.should_receive(:close!).and_return(question)
question_mock = mock('question_mock')
question_mock.should_receive(:find).and_return([question])


@issue = Issue.new
@issue.should_receive(:questions).and_return(question_mock)
@issue.should_receive(:open_questions).and_return(question_mock)
@issue.close_pending_questions(@user, @journal)
end

it 'should not close any questions for other users' do
@user = mock_model(User)
@journal = mock_model(Journal)
@other_user = mock_model(User)
question = mock_model(Question, :assigned_to => @other_user, :for_anyone? => false)
question = mock_model(Question, :opened => true, :assigned_to => @other_user, :for_anyone? => false)
question.should_not_receive(:close!)
question_mock = mock('question_mock')
question_mock.should_receive(:find).and_return([question])


@issue = Issue.new
@issue.should_receive(:questions).and_return(question_mock)
@issue.should_receive(:open_questions).and_return(question_mock)
@issue.close_pending_questions(@user, @journal)
end

Expand Down
10 changes: 6 additions & 4 deletions spec/lib/question_journal_hooks_spec.rb
Expand Up @@ -7,8 +7,9 @@
@user2 = mock_model(User, :id => 2, :name => 'Test two', :login => 'test2')
@project = mock_model(Project)
@issue = mock_model(Issue)
@question = mock_model(Question, :assigned_to => @user2)
@question = mock_model(Question, :assigned_to => @user2, :opened => true)
@journal = mock_model(Journal, :issue => @issue, :question => @question, :project => @project)

@context = { :journal => @journal }
end

Expand Down Expand Up @@ -71,8 +72,9 @@
describe QuestionJournalHooks, '#controller_journals_edit_post with a reassigned question' do
it 'should change the assignment of the question' do
issue = mock_model(Issue)
question = mock_model(Question, :assigned_to_id => 2)
question = mock_model(Question, :assigned_to_id => 2, :opened => true)
question.should_receive(:update_attributes).with({ :assigned_to => nil}).and_return(true)

journal = mock_model(Journal, :question => nil, :issue => issue, :question => question)
context = {
:journal => journal,
Expand All @@ -85,7 +87,7 @@
describe QuestionJournalHooks, '#controller_journals_edit_post with a removed question' do
it 'should destroy the question' do
issue = mock_model(Issue)
question = mock_model(Question, :assigned_to_id => 2)
question = mock_model(Question, :assigned_to_id => 2, :opened => true)
journal = mock_model(Journal, :question => nil, :issue => issue, :question => question)
question.should_receive(:destroy).and_return(true)
context = {
Expand All @@ -99,7 +101,7 @@
describe QuestionJournalHooks, '#view_journals_update_rjs_bottom with a question for anyone' do
before(:each) do
@rjs_page = ''
@question = mock_model(Question, :assigned_to => nil)
@question = mock_model(Question, :opened? => true, :assigned_to => nil)
@journal = mock_model(Journal, :reload => true, :question => @question)
@context = { :page => @rjs_page, :journal => @journal}
QuestionJournalHooks.instance.view_journals_update_rjs_bottom( @context )
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/question_queries_helper_patch_spec.rb
Expand Up @@ -90,7 +90,7 @@ def link_to(name, options = {}, html_options = nil)
describe QueriesHelper, "#question_column_content" do
it 'should use a special format for the questions column' do
issue = mock_model(Issue)
issue.should_receive(:questions)
issue.should_receive(:open_questions)
column = mock_model(QueryColumn, :name => :formatted_questions)

helper = QueriesHelperWrapper.new
Expand Down
38 changes: 29 additions & 9 deletions spec/models/question_spec.rb
Expand Up @@ -64,22 +64,42 @@

end

describe Question, "#close!" do
describe Question, "#close! on a closed question" do
include QuestionSpecHelper

it 'should remove a question' do
it 'should do nothing' do
question = question_factory(1, { :opened => false })
question.should_not_receive(:save)
question.should_not_receive(:save!)
proc {
question.close!
}.should_not change(question, :opened)
end

it 'should not send a Question Mail' do
question = question_factory(1, { :opened => false })
QuestionMailer.should_not_receive(:deliver_answered_question)
question.close!
end
end

describe Question, "#close! on an open question" do
include QuestionSpecHelper

it 'should change an open question to a closed one' do
journal = mock_model(Journal)
question = question_factory(1)
question = question_factory(1, { :opened => true })
question.should_receive(:save!)
QuestionMailer.stub!(:deliver_answered_question)
question.should_receive(:destroy).and_return(true)

question.close!(journal)
proc {
question.close!(journal)
}.should change(question, :opened).to(false)
end

it 'should send a Question Mail when closing a question' do
it 'should send a Question Mail when closing an open question' do
journal = mock_model(Journal)
question = question_factory(1)
question.stub!(:destroy)
question = question_factory(1, { :opened => true })
question.stub!(:save!)
QuestionMailer.should_receive(:deliver_answered_question).with(question, journal)
question.close!(journal)
end
Expand Down

0 comments on commit f5b4dcc

Please sign in to comment.