There are several spots that include trailing white spaces. Please make sure you remove all of them. Inline are my comments for this patch.
Also, since this patch requires the tester to run some additional steps, those steps need to be included in the commit message. IOW, when you
do "git commit -a --amend", please include any steps needed to review the patch. In this case, I needed to run:
in order to test. So that should be included in the commit message. Also what do I need to do to get the system to send an email; how do I configure the system?
I tested this with a single backlog item that has no tasks against it. That backlog item should've received a nag email as well. Change the code to also check whether a backlog item is marked as assigned but doesn't have any tasks as well and include those backlog items in the email.
NAK - Please fix the above issue, plus the bits in the comments below and resend the patch for review.
On Oct 21, 2008, at 4:44 PM, LAN-SUN-LUK Benjamin wrote:
Signed-off-by: Benjamin <benjamin.lan-sun-luk@supinfo.com>
---
app/models/user_mailer.rb | 10 +++++
.../user_mailer/no_activity_recorded.html.erb | 6 +++
config/environment.rb | 5 ++-
config/initializers/schedules.rb | 37 ++++++++++++++++++++
4 files changed, 57 insertions(+), 1 deletions(-)
create mode 100644 app/views/user_mailer/no_activity_recorded.html.erb
create mode 100644 config/initializers/schedules.rb
diff --git a/app/models/user_mailer.rb b/app/models/user_mailer.rb
index 73308af..8d60671 100644
--- a/app/models/user_mailer.rb
+++ b/app/models/user_mailer.rb
@@ -7,4 +7,14 @@ class UserMailer < ActionMailer::Base
subject "Your password"
body :user => user, :new_password => new_password
end
+
Trailing whitespaces here. Please clean those up.
+ # Send an e-mail to an user an notify him that no-activity has been detected
+ # for some backlog items.
Change the comment to "Send an email to a user to notify him that no activitiy has been detected in his backlog.
+ #
+ def no_activity_recorded(user, backlog_items)
+ recipients user.email
+ from "no_reply@projxp.org" # TODO Email must be edit
I had mentioned on the previous patch about passwords that you should go ahead and add a configuration for this. Can you go ahead and add that to this patch?
+ subject "No activity has been detected"
+ body :user => user, :backlog_items => backlog_items
+ end
end
diff --git a/app/views/user_mailer/no_activity_recorded.html.erb b/app/views/user_mailer/no_activity_recorded.html.erb
new file mode 100644
index 0000000..e4acd3c
--- /dev/null
+++ b/app/views/user_mailer/no_activity_recorded.html.erb
@@ -0,0 +1,6 @@
+Dear <%= @user.display_name %>,
+
+No activities was detected on this backlogs:
This isn't a very clean message. A better text would be something like:
The following backlog items are active, but no tasks were entered against them. Please take a moment to update them.
+<% @backlog_items.each do |backlog_item|%>
+<%= backlog_item.user_story.title %>
+<% end %>
diff --git a/config/environment.rb b/config/environment.rb
index af8a593..1687102 100644
--- a/config/environment.rb
+++ b/config/environment.rb
@@ -72,6 +72,9 @@ Rails::Initializer.run do |config|
# Make Active Record use UTC-base instead of local time
# config.active_record.default_timezone = :utc
+
Trailing whitespace.
+ # Gems
+ config.gem 'openwferu-scheduler', :lib => 'openwfe/util/scheduler'
end
ActiveSupport::CoreExtensions::Time::Conversions::DATE_FORMATS.merge!(
@@ -82,7 +85,7 @@ ActiveSupport::CoreExtensions::Time::Conversions::DATE_FORMATS.merge!(
# Mail configuration.
# Please complete the config file are locate at /config/mailer.yml
-#
+#
This line introduces whitespace. Please clean that out.
require "smtp_tls"
mailer_config = File.open("#{RAILS_ROOT}/config/mailer.yml")
diff --git a/config/initializers/schedules.rb b/config/initializers/schedules.rb
new file mode 100644
index 0000000..287756c
--- /dev/null
+++ b/config/initializers/schedules.rb
There are several bits of trailing whitespace here as well. Please clean them out, then use "git diff" to see if there are any more.
@@ -0,0 +1,37 @@
+require 'fastthread'
+require 'openwfe/util/scheduler'
+include OpenWFE
+
+# Initialize a threads array
+threads = []
+
+threads << Thread.new do
+ scheduler = Scheduler.new
+ scheduler.start
+
+ # Do this everyday at 10:00 am
This comment doesn't match the text that follows. Please fix the comment or remove it.
+ scheduler.schedule('0 8 * * *') do
I'd really like this to be configurable, but for now it's okay to leave it as hard-coded. But, let's move it back to a little earlier in the day, maybe 5am?
+
+ user_no_activity_on_backlog_items = {}
+ BacklogItem.find_all_by_state(BacklogItem::STATE_ASSIGNED).each do |backlog_item|
Here we should check if the item has any tasks and, if it doesn't, send a name message about that.
+ backlog_item.tasks.each do |task|
+ if task.created_at.to_date < Date.yesterday
+ backlog_item_owner = backlog_item.owner
+ user_no_activity_on_backlog_items[backlog_item_owner] ||= []
+ user_no_activity_on_backlog_items[backlog_item_owner] << backlog_item
+ end
+ end
+ end
+
+ # Send the e-mail
+ user_no_activity_on_backlog_items.each do |owner, backlog_items|
+ UserMailer.deliver_no_activity_recorded(owner, backlog_items)
+ end
+
+ end
+
+ scheduler.join
+end
+
+# Run all threads
+threads.each { |thread| thread.run }
\ No newline at end of file
--
1.5.2.4
_______________________________________________
projxp-devel mailing list