Find stewards files in target branch

Gemfile

@@ -62,6 +62,7 @@ group :test do
   gem 'webmock'
   gem 'coveralls', require: false
   gem 'rugged', require: false
+  gem 'hashdiff', '0.2.2'
 end
 
 

Gemfile.lock

@@ -81,6 +81,7 @@ GEM
       multipart-post (>= 1.2, < 3)
     globalid (0.3.6)
       activesupport (>= 4.1.0)
+    hashdiff (0.2.2)
     hashie (3.4.2)
     http-cookie (1.0.2)
       domain_name (~> 0.5)
@@ -237,6 +238,7 @@ DEPENDENCIES
   devise
   devise-bootstrap-views
   factory_girl_rails (~> 4.0)
+  hashdiff (= 0.2.2)
   jbuilder (~> 2.0)
   jquery-rails
   mocha

app/mailers/user_mailer.rb

@@ -9,10 +9,10 @@ def title
     end
   end
 
-  def notify(user:, repository:, pull_request:, steward_changes_views:)
+  def notify(user:, repository:, pull_request:, changes_view:)
     @pull_request = PullRequestPresenter.new(pull_request)
-    @steward_changes_views = steward_changes_views
-    @user = user
+    @changes_view = changes_view
+    @user         = user
 
     mail(to:      user.email,
          subject: "[#{repository}] Ladle Alert: #{@pull_request.title}")

app/views/user_mailer/notify.html.erb

@@ -9,11 +9,11 @@
 <p>You were notified of this pull request because of the following changes:</p>
 
 <ul>
-<% @steward_changes_views.each do |steward_changes| %>
+  <% @changes_view.each do |rules, changes| %>
   <li>
-    <p>Changes monitored by <%= steward_changes.stewards_file %>:</p>
+    <p>Changes monitored by <%= rules.ref %>/<%= rules.stewards_file %>:</p>
     <ul>
-      <% steward_changes.changes.each do |change| %>
+      <% changes.each do |change| %>
         <li><strong><%= change.status_initial %></strong> <%= change.file %> (<%= change.changes_count %>)</li>
       <% end %>
     </ul>

app/views/user_mailer/notify.text.erb

@@ -10,10 +10,10 @@ You can view, comment on, or merge this pull request online at:
 
 You were notified of this pull request because of the following changes:
 
-<% @steward_changes_views.each do |steward_changes| %>
-  Changes monitored by <%= steward_changes.stewards_file %>:
+<% @changes_view.each do |rules, changes| %>
+  Changes monitored by <%= rules.ref %>/<%= rules.stewards_file %>:
 
-  <% steward_changes.changes.each do |change| %>
+  <% changes.each do |change| %>
     * <%= change.status_initial %> <%= change.file %> (<%= change.changes_count %>)
   <% end %>
 

lib/ladle.rb

@@ -2,7 +2,8 @@ module Ladle
   extend ActiveSupport::Autoload
 
   autoload :FileChange
-  autoload :StewardChangesView
+  autoload :ChangesView
+  autoload :StewardRules
 end
 
 require 'ladle/exceptions'

lib/ladle/changed_files.rb

@@ -44,7 +44,7 @@ def add_file_change(file_change)
       @directories[directory_name] ||= Directory.new(directory_name)
       @directories[directory_name].add_change(file_change)
 
-      if file_change.file.to_s =~ /stewards\.yml$/ && file_change.status != :added
+      if file_change.file.to_s =~ /stewards\.yml$/ && file_change.status != :removed
         @modified_stewards_files << file_change.file
       end
 

lib/ladle/changes_view.rb

@@ -0,0 +1,54 @@
+module Ladle
+  class ChangesView
+    include Enumerable
+
+    RulesChanges = Struct.new(:rules, :changes)
+
+    def initialize
+      @paths = {}
+    end
+
+    def empty?
+      @paths.values.all?(&:empty?)
+    end
+
+    def ==(other)
+      @paths == other.paths
+    end
+
+    alias eql? ==
+
+    def each
+      sorted_paths = @paths.keys.sort
+      sorted_paths.each do |path_key|
+        rules_and_changes_list = @paths[path_key]
+
+        rules_and_changes_list.each do |rules_and_changes|
+          yield rules_and_changes.rules, rules_and_changes.changes
+        end
+      end
+    end
+
+    def add_changes(rules, changes)
+      @paths[rules.stewards_file.to_s] ||= []
+
+      add_changes_to(@paths[rules.stewards_file.to_s], rules, changes)
+    end
+
+    protected
+
+    attr_reader :paths
+
+    private
+
+    def add_changes_to(rules_and_changes_collection, rules, changes)
+      changes_already_exist = rules_and_changes_collection.any? do |rules_changes|
+        rules_changes.changes == changes
+      end
+
+      unless changes_already_exist
+        rules_and_changes_collection << RulesChanges.new(rules, changes)
+      end
+    end
+  end
+end

lib/ladle/file_filter.rb

@@ -32,5 +32,4 @@ def include?(file_pathname)
 
     attr_reader :include_patterns, :exclude_patterns
   end
-
 end

lib/ladle/pull_handler.rb

@@ -1,5 +1,6 @@
 require 'ladle/stewards_file_parser'
-require 'ladle/steward_changes_view'
+require 'ladle/steward_rules'
+require 'ladle/steward_tree'
 
 module Ladle
   class PullHandler
@@ -11,14 +12,11 @@ def initialize(client, notifier)
     def handle(pull_request)
       pr_info = @client.pull_request(pull_request.number)
 
-      pull_request_files = @client.pull_request_files(pull_request.number)
+      pr_files = @client.pull_request_files(pull_request.number)
 
-      stewards_registry = {}
+      stewards_trees = collect_stewards_rules(pr_info, pr_files)
 
-      read_current_stewards(stewards_registry, pull_request_files, pr_info.head_sha)
-      read_old_stewards(stewards_registry, pull_request_files, pr_info.base_sha)
-
-      collect_files(stewards_registry, pull_request_files)
+      stewards_registry = collect_changes(stewards_trees, pr_files)
 
       if stewards_registry.empty?
         Rails.logger.info('No stewards found. Doing nothing.')
@@ -30,51 +28,50 @@ def handle(pull_request)
 
     private
 
-    def read_current_stewards(registry, pull_request_files, pr_head)
-      pull_request_files.directories.each do |directory|
-        register_stewards(registry, directory.join('stewards.yml'), pr_head)
+    def collect_stewards_rules(pr_info, pr_files)
+      rules = {}
+
+      pr_files.directories.each do |directory|
+        register_stewards(rules, directory.join('stewards.yml'), pr_info.base_sha)
       end
-    end
 
-    def read_old_stewards(registry, pull_request_files, parent_head)
-      pull_request_files.modified_stewards_files.each do |stewards_file_path|
-        register_stewards(registry, stewards_file_path, parent_head)
+      pr_files.modified_stewards_files.each do |stewards_file_path|
+        register_stewards(rules, stewards_file_path, pr_info.head_sha)
       end
+
+      rules
     end
 
-    def register_stewards(registry, stewards_file_path, sha)
+    def register_stewards(stewards_rules_map, stewards_file_path, sha)
       contents = @client.contents(path: stewards_file_path.to_s, ref: sha)
       stewards_file = StewardsFileParser.parse(contents)
 
       stewards_file.stewards.each do |steward_config|
-        registry[steward_config.github_username] ||= []
+        rules = Ladle::StewardRules.new(ref:           sha,
+                                        stewards_file: stewards_file_path,
+                                        file_filter:   steward_config.file_filter)
 
-        changes_view = Ladle::StewardChangesView.new(stewards_file: stewards_file_path,
-                                                     file_filter: steward_config.file_filter)
-
-        registry[steward_config.github_username] << changes_view
+        stewards_rules_map[steward_config.github_username] ||= StewardTree.new
+        stewards_rules_map[steward_config.github_username].add_rules(rules)
       end
     rescue Ladle::RemoteFileNotFound
       # Ignore - stewards files don't have to exist
     rescue StewardsFileParser::ParsingError => e
       Rails.logger.error("Error parsing file #{stewards_file_path}: #{e.message}\n#{e.backtrace.join("\n")}")
     end
 
-    def collect_files(stewards_registry, pull_request_files)
-      stewards_registry.each_value do |steward_change_views|
-        steward_change_views.each do |change_view|
-          file_changes = pull_request_files.file_changes_in(change_view.stewards_file.dirname)
-          change_view.add_file_changes(file_changes)
-        end
+    def collect_changes(stewards_trees, pull_request_files)
+      output = {}
 
-        steward_change_views.reject! do |change_view|
-          change_view.empty?
+      stewards_trees.each do |github_username, steward_tree|
+        changes_view = steward_tree.changes(pull_request_files)
+
+        unless changes_view.empty?
+          output[github_username] = changes_view
         end
       end
 
-      stewards_registry.reject! do |_, steward_change_views|
-        steward_change_views.empty?
-      end
+      output
     end
   end
 end

lib/ladle/steward_notifier.rb

@@ -7,11 +7,11 @@ def initialize(repository_name, pull_request)
 
     def notify(stewards_changes)
       notified_users = []
-      stewards_changes.each do |github_username, steward_changes_views|
+      stewards_changes.each do |github_username, changes_view|
         user = User.find_by_github_username(github_username)
 
         if user && ! user_has_been_notified?(user)
-          send_email(user, steward_changes_views)
+          send_email(user, changes_view)
           notified_users << user
         end
       end
@@ -25,8 +25,8 @@ def user_has_been_notified?(user)
       user.notifications.where(pull_request: @pull_request).exists?
     end
 
-    def send_email(user, steward_changes_views)
-      UserMailer.notify(user: user, repository: @repository_name, pull_request: @pull_request, steward_changes_views: steward_changes_views).deliver_now
+    def send_email(user, changes_view)
+      UserMailer.notify(user: user, repository: @repository_name, pull_request: @pull_request, changes_view: changes_view).deliver_now
     end
 
     def create_notification(users)

lib/ladle/steward_rules.rb

@@ -0,0 +1,31 @@
+require 'ladle/file_filter'
+
+module Ladle
+  class StewardRules
+    attr_reader :ref, :stewards_file, :file_filter
+
+    def initialize(ref:, stewards_file:, file_filter: nil)
+      @ref           = ref
+      @file_filter   = file_filter || FileFilter.new
+      @stewards_file = Pathname.new(stewards_file)
+    end
+
+    def select_matching_file_changes(file_changes)
+      file_changes.select do |file_change|
+        @file_filter.include?(file_change.file.relative_path_from(@stewards_file.dirname))
+      end
+    end
+
+    def ==(other)
+      @ref == other.ref &&
+        @stewards_file == other.stewards_file &&
+        @file_filter == other.file_filter
+    end
+
+    def hash
+      [@ref, @stewards_file, @file_filter].hash
+    end
+
+    alias eql? ==
+  end
+end

lib/ladle/steward_tree.rb

@@ -0,0 +1,29 @@
+require 'ladle/changes_view'
+
+module Ladle
+  class StewardTree
+
+    def initialize
+      @rules = []
+    end
+
+    def add_rules(rules)
+      @rules << rules
+    end
+
+    def changes(pull_request_files)
+      changes_view = ChangesView.new
+
+      @rules.each do |rules|
+        file_changes = pull_request_files.file_changes_in(rules.stewards_file.dirname)
+        file_changes = rules.select_matching_file_changes(file_changes)
+
+        unless file_changes.empty?
+          changes_view.add_changes(rules, file_changes)
+        end
+      end
+
+      changes_view
+    end
+  end
+end