Mon Jul 30 09:41:37 EST 2007 Matt Palmer * Add the ability to handle multiple trac refs in one conversational message diff -rN -u old-trac_urls/data/rbot/plugins/trac_urls.rb new-trac_urls/data/rbot/plugins/trac_urls.rb --- old-trac_urls/data/rbot/plugins/trac_urls.rb 2008-10-20 23:31:05.439857336 +1100 +++ new-trac_urls/data/rbot/plugins/trac_urls.rb 2008-10-20 23:31:05.467857621 +1100 @@ -49,28 +49,33 @@ # chat messages return unless m.kind_of?(PrivMessage) && m.public? - return unless m.message =~ /(\[\d+\]|r\d+|\#\d+|wiki:\S+)/ + refs = m.message.scan(/\[\d+\]|r\d+|\#\d+|wiki:\S+/) - debug "We're out to handle '#{$1}'" + # Do we have at least one possible reference? + return unless refs.length > 0 + + refs.each do |ref| + debug "We're out to handle '#{ref}'" - # So what's our ref? Strip out the wiki: if it's there, because - # that's just a special cheat to minimise the amount of crap we have - # to deal with in general conversation. - ref = $1.sub(/^wiki:/, '') - - url, title = expand_reference(ref, m.target) - - return unless url - - # Try to address the message to the right person - if m.message =~ /^(\S+)[:,]/ - addressee = "#{$1}: " - else - addressee = "#{m.sourcenick}: " + # So what's our ref? Strip out the wiki: if it's there, because + # that's just a special cheat to minimise the amount of crap we have + # to deal with in general conversation. + ref = ref.sub(/^wiki:/, '') + + url, title = expand_reference(ref, m.target) + + return unless url + + # Try to address the message to the right person + if m.message =~ /^(\S+)[:,]/ + addressee = "#{$1}: " + else + addressee = "#{m.sourcenick}: " + end + + # So we have a valid URL, and addressee, and now we just have to... speak! + m.reply "#{addressee}#{ref} is #{url}" + (title.nil? ? '' : ", \"#{title}\"") end - - # So we have a valid URL, and addressee, and now we just have to... speak! - m.reply "#{addressee}#{ref} is #{url}" + (title.nil? ? '' : ", \"#{title}\"") end def tracinfo(m, params) diff -rN -u old-trac_urls/test/test_trac_urls.rb new-trac_urls/test/test_trac_urls.rb --- old-trac_urls/test/test_trac_urls.rb 2008-10-20 23:31:05.439857336 +1100 +++ new-trac_urls/test/test_trac_urls.rb 2008-10-20 23:31:05.459857669 +1100 @@ -37,20 +37,21 @@ ), '/trac/changeset/69' => MOCK_HTTP_RESPONSE_CLASS.new( - "

Something Bad Happened

But I'm not going to tell you what

", + "

Something Unexpected Happened

This doesn't look like a changeset page to me...

", '200', 'OK' ) } + MOCK_HTTP_RESPONSES.values.each { |r| r.body = "#{r.body}" } + needs_mocha do - def init_net_http_mock(url_list) + def init_net_http_mock(*url_list) conn = mock - MOCK_HTTP_RESPONSES.each_pair do |url, resp| - resp.body = "#{resp.body}" - conn.expects(:get).with(url).returns(resp) if url_list.include?(url) + url_list.each do |url| + conn.expects(:get).with(url).returns(MOCK_HTTP_RESPONSES[url]) end - Net::HTTP.expects(:start).with('example.com').yields(conn) + Net::HTTP.expects(:start).times(url_list.length).with('example.com').yields(conn) end end @@ -99,9 +100,7 @@ :sourcenick => 'everyman', :content => 'config list' - assert_equal [], $rbotlog['error'] - assert_equal [], $rbotlog['warning'] - assert_equal [], $rbotlog['info'] + assert_no_rbot_errors assert_equal 1, @bot.replies.length # Make sure that the trac_urls plugin is in there somewhere @@ -123,7 +122,7 @@ @bot.config['trac_urls.channelmap'] << '#channel:http://example.com/trac' needs_mocha do - init_net_http_mock(['/trac/ticket/1']) + init_net_http_mock('/trac/ticket/1') @bot.message :target => '#channel', :sourcenick => 'everyman', @@ -141,14 +140,12 @@ @bot.config['trac_urls.channelmap'] << '#channel:http://example.com/trac' needs_mocha do - init_net_http_mock(['/trac/ticket/1']) + init_net_http_mock('/trac/ticket/1') @bot.message :target => '#channel', :sourcenick => "auser", :content => "#{@bot.nick}: tracinfo #1" - assert_equal [], $rbotlog['error'] - assert_equal [], $rbotlog['warning'] - assert_equal [], $rbotlog['info'] + assert_no_rbot_errors assert_equal 1, @bot.replies.length, "We didn't get a reply" assert_equal "auser: #1 is http://example.com/trac/ticket/1, \"My First Ticket\"", @@ -313,4 +310,39 @@ assert_equal 'everyman', @bot.replies[0][:target].nick assert_equal "I can't do tracinfo in private yet", @bot.replies[0][:content] end + + def test_multiple_refs_in_conversation + @bot.config['trac_urls.channelmap'] << '#channel:http://example.com/trac' + + needs_mocha do + init_net_http_mock('/trac/ticket/1', '/trac/wiki/RbotIsTehKing') + + @bot.message :target => '#channel', + :sourcenick => "everyman", + :content => "Does anyone else think that #1 is solved by wiki:RbotIsTehKing ?" + + assert_no_rbot_errors + assert_equal 2, @bot.replies.length + assert_equal "everyman: #1 is http://example.com/trac/ticket/1, \"My First Ticket\"", @bot.replies[0][:content] + assert_equal "everyman: RbotIsTehKing is http://example.com/trac/wiki/RbotIsTehKing", @bot.replies[1][:content] + end + end + + def test_multiple_refs_in_conversation_but_only_one_good_one + @bot.config['trac_urls.channelmap'] << '#channel:http://example.com/trac' + + needs_mocha do + init_net_http_mock('/trac/ticket/1', '/trac/changeset/666') + + @bot.message :target => '#channel', + :sourcenick => "everyman", + :content => "Does anyone else think that #1 is solved by [666]?" + + assert_equal 1, @bot.replies.length, "Didn't get exactly one reply: #{@bot.replies.inspect}" + assert_equal "everyman: #1 is http://example.com/trac/ticket/1, \"My First Ticket\"", @bot.replies[0][:content] + assert_equal 1, $rbotlog['error'].length, "Didn't get exactly one error" + assert_match /InvalidTracUrl returned/, $rbotlog['error'][0] + end + end + end