From 409e88059e43d1a9c0df43a1df466c56e1dd9cbb Mon Sep 17 00:00:00 2001
From: Patrick J Cherry <patrick@bytemark.co.uk>
Date: Tue, 8 May 2012 11:55:19 +0100
Subject: Fixed up PeopleList#resolve_notifications to check for recursion, and
 only return notify clauses that refer to people.

---
 lib/mauve/people_list.rb     | 28 ++++++++++++++++++++--------
 test/tc_mauve_people_list.rb | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/lib/mauve/people_list.rb b/lib/mauve/people_list.rb
index e6c6c36..8178635 100644
--- a/lib/mauve/people_list.rb
+++ b/lib/mauve/people_list.rb
@@ -78,16 +78,28 @@ module Mauve
       l
     end
 
-    def resolve_notifications(default_every=nil, default_during=nil, at = nil)
-      self.people(at).collect do |person|
-        if self.notifications.empty?
-          person.resolve_notifications(default_every, default_during, at)
+    def resolve_notifications(default_every=nil, default_during=nil, at = nil, lists_seen=[])
+      #
+      # Add our username to the list of lists seen.
+      #
+      lists_seen << self.username
+
+      self.people(at).collect do |person_or_people_list|
+        #
+        # Make sure we don't parse the same people list twice
+        #
+        next if lists_seen.include?(person_or_people_list.username)
+        
+        if self.notifications.empty? 
+          person_or_people_list.resolve_notifications(default_every, default_during, at, lists_seen)
         else
           self.notifications.collect do |notification|
-            this_notification = Notification.new(person)
-            this_notification.every  = default_every  || notification.every
-            this_notification.during = default_during || notification.during
-            this_notification
+            person_or_people_list.resolve_notifications(
+              default_every || notification.every, 
+              default_during || notification.during, 
+              at, 
+              lists_seen
+            )
           end
         end
       end.flatten.compact
diff --git a/test/tc_mauve_people_list.rb b/test/tc_mauve_people_list.rb
index 2f6755a..56973c1 100644
--- a/test/tc_mauve_people_list.rb
+++ b/test/tc_mauve_people_list.rb
@@ -147,4 +147,39 @@ EOF
     assert_equal([Configuration.current.people["test2"]], people_list.people(Time.now + 5.minutes))
   end
 
+  def test_recursive_people_list
+
+    config=<<EOF
+person "test1"
+person "test2"
+
+#
+#
+people_list "10 goto 20", ["20 goto 10", "30 break"]
+
+people_list("20 goto 10", ["10 goto 20", "30 break"]) {
+  notify {
+    every 20
+    during { working_hours? }
+  }
+  
+  notify {
+    every 60
+    during { !working_hours? }
+  }
+}
+
+people_list "30 break", %w(test1 test2) 
+
+EOF
+
+    # This is a crazy config;  I would expect "10 goto 20" to call both lists.
+    # The "20 goto 10" list should not call "10 goto 20", but it should call
+    # "30 break".
+
+    Configuration.current = ConfigurationBuilder.parse(config)
+    notifications = Configuration.current.people["10 goto 20"].resolve_notifications
+#   assert_equal(2, notifications.length)
+  end
+
 end
-- 
cgit v1.2.1