[Updater] Wire up and flesh out branch naming for Grouped Update PRs

common/lib/dependabot/dependency_group.rb

@@ -2,10 +2,11 @@
 
 module Dependabot
   class DependencyGroup
-    attr_reader :name
+    attr_reader :name, :rules
 
-    def initialize(name)
+    def initialize(name:, rules:)
       @name = name
+      @rules = rules
     end
   end
 end

common/lib/dependabot/pull_request_creator.rb

@@ -49,7 +49,7 @@ def initialize(cause, pull_request)
                 :commit_message_options, :vulnerabilities_fixed,
                 :reviewers, :assignees, :milestone, :branch_name_separator,
                 :branch_name_prefix, :branch_name_max_length, :github_redirection_service,
-                :custom_headers, :provider_metadata
+                :custom_headers, :provider_metadata, :dependency_group
 
     def initialize(source:, base_commit:, dependencies:, files:, credentials:,
                    pr_message_header: nil, pr_message_footer: nil,
@@ -61,7 +61,7 @@ def initialize(source:, base_commit:, dependencies:, files:, credentials:,
                    automerge_candidate: false,
                    github_redirection_service: DEFAULT_GITHUB_REDIRECTION_SERVICE,
                    custom_headers: nil, require_up_to_date_base: false,
-                   provider_metadata: {}, message: nil)
+                   provider_metadata: {}, message: nil, dependency_group: nil)
       @dependencies               = dependencies
       @source                     = source
       @base_commit                = base_commit
@@ -87,6 +87,7 @@ def initialize(source:, base_commit:, dependencies:, files:, credentials:,
       @require_up_to_date_base    = require_up_to_date_base
       @provider_metadata          = provider_metadata
       @message                    = message
+      @dependency_group           = dependency_group
 
       check_dependencies_have_previous_version
     end
@@ -235,7 +236,7 @@ def branch_namer
           dependencies: dependencies,
           files: files,
           target_branch: source.branch,
-          dependency_group: nil,
+          dependency_group: dependency_group,
           separator: branch_name_separator,
           prefix: branch_name_prefix,
           max_length: branch_name_max_length

common/lib/dependabot/pull_request_creator/branch_namer.rb

@@ -5,6 +5,7 @@
 require "dependabot/metadata_finders"
 require "dependabot/pull_request_creator"
 require "dependabot/pull_request_creator/branch_namer/solo_strategy"
+require "dependabot/pull_request_creator/branch_namer/dependency_group_strategy"
 
 module Dependabot
   class PullRequestCreator

common/lib/dependabot/pull_request_creator/branch_namer/dependency_group_strategy.rb

@@ -15,13 +15,40 @@ def initialize(dependencies:, files:, target_branch:, dependency_group:,
           @max_length       = max_length
         end
 
+        # FIXME: Incorporate max_length truncation once we allow user config
+        #
+        # For now, we are using a placeholder DependencyGroup with a
+        # fixed-length name, so we can punt on handling truncation until
+        # we determine the strict validation rules for names
         def new_branch_name
-          dependency_group.name
+          File.join(prefixes, dependency_group.name, prototype_suffix).gsub("/", separator)
         end
 
         private
 
-        attr_reader :dependency_group
+        attr_reader :dependencies, :dependency_group, :files, :target_branch, :separator, :prefix, :max_length
+
+        def prefixes
+          [
+            prefix,
+            package_manager,
+            directory,
+            target_branch
+          ].compact
+        end
+
+        # FIXME: Remove once grouped PRs can supersede each other
+        def prototype_suffix
+          "prototype-#{Time.now.utc.to_i}"
+        end
+
+        def package_manager
+          dependencies.first.package_manager
+        end
+
+        def directory
+          files.first.directory.tr(" ", "-")
+        end
       end
     end
   end

common/spec/dependabot/dependency_group_spec.rb

@@ -6,7 +6,7 @@
   describe "#name" do
     it "returns the name" do
       my_dependency_group_name = "darren-from-work"
-      dependency_group = described_class.new(my_dependency_group_name)
+      dependency_group = described_class.new(name: my_dependency_group_name, rules: anything)
 
       expect(dependency_group.name).to eq(my_dependency_group_name)
     end

common/spec/dependabot/pull_request_creator/branch_namer/dependency_group_strategy_spec.rb

@@ -1,19 +1,93 @@
 # frozen_string_literal: true
 
+require "dependabot/dependency"
+require "dependabot/dependency_file"
+require "dependabot/dependency_group"
 require "dependabot/pull_request_creator/branch_namer/dependency_group_strategy"
 
 RSpec.describe Dependabot::PullRequestCreator::BranchNamer::DependencyGroupStrategy do
+  subject(:namer) do
+    described_class.new(
+      dependencies: dependencies,
+      files: [gemfile],
+      target_branch: target_branch,
+      separator: separator,
+      dependency_group: dependency_group
+    )
+  end
+
+  let(:dependencies) { [dependency] }
+  let(:dependency) do
+    Dependabot::Dependency.new(
+      name: "business",
+      version: "1.5.0",
+      previous_version: "1.4.0",
+      package_manager: "bundler",
+      requirements: {},
+      previous_requirements: {}
+    )
+  end
+  let(:gemfile) do
+    Dependabot::DependencyFile.new(
+      name: "Gemfile",
+      content: "anything",
+      directory: directory
+    )
+  end
+
+  let(:dependency_group) do
+    Dependabot::DependencyGroup.new(name: "my-dependency-group", rules: anything)
+  end
+
   describe "#new_branch_name" do
-    it "returns the name of the dependency group" do
-      dependency_group = double("DependencyGroup", name: "my_dependency_group")
-      strategy = described_class.new(
-        dependencies: [],
-        files: [],
-        target_branch: "main",
-        dependency_group: dependency_group
-      )
-
-      expect(strategy.new_branch_name).to eq(dependency_group.name)
+    context "with defaults for separator, target branch and files in the root directory" do
+      let(:directory) { "/" }
+      let(:target_branch) { nil }
+      let(:separator) { "/" }
+
+      it "returns the name of the dependency group prefixed correctly" do
+        expect(namer.new_branch_name).to start_with("dependabot/bundler/my-dependency-group")
+      end
+    end
+
+    context "with a custom separator" do
+      let(:directory) { "/" }
+      let(:target_branch) { nil }
+      let(:separator) { "_" }
+
+      it "returns the name of the dependency group prefixed correctly" do
+        expect(namer.new_branch_name).to start_with("dependabot_bundler_my-dependency-group")
+      end
+    end
+
+    context "for files in a non-root directory" do
+      let(:directory) { "rails app/" } # let's make sure we deal with spaces too
+      let(:target_branch) { nil }
+      let(:separator) { "/" }
+
+      it "returns the name of the dependency group prefixed correctly" do
+        expect(namer.new_branch_name).to start_with("dependabot/bundler/rails-app/my-dependency-group")
+      end
+    end
+
+    context "targeting a branch" do
+      let(:directory) { "/" }
+      let(:target_branch) { "develop" }
+      let(:separator) { "/" }
+
+      it "returns the name of the dependency group prefixed correctly" do
+        expect(namer.new_branch_name).to start_with("dependabot/bundler/develop/my-dependency-group")
+      end
+    end
+
+    context "for files in a non-root directory targetting a branch" do
+      let(:directory) { "rails-app/" }
+      let(:target_branch) { "develop" }
+      let(:separator) { "_" }
+
+      it "returns the name of the dependency group prefixed correctly" do
+        expect(namer.new_branch_name).to start_with("dependabot_bundler_rails-app_develop_my-dependency-group")
+      end
     end
   end
 end

common/spec/dependabot/pull_request_creator/branch_namer_spec.rb

@@ -5,8 +5,6 @@
 require "dependabot/dependency"
 require "dependabot/dependency_file"
 require "dependabot/pull_request_creator/branch_namer"
-require "dependabot/pull_request_creator/branch_namer/solo_strategy"
-require "dependabot/pull_request_creator/branch_namer/dependency_group_strategy"
 
 RSpec.describe Dependabot::PullRequestCreator::BranchNamer do
   subject(:namer) do

common/spec/dependabot/pull_request_creator_spec.rb

@@ -4,6 +4,7 @@
 require "spec_helper"
 require "dependabot/dependency"
 require "dependabot/dependency_file"
+require "dependabot/dependency_group"
 require "dependabot/pull_request_creator"
 require "dependabot/pull_request_creator/message"
 
@@ -354,5 +355,54 @@
         creator.create
       end
     end
+
+    context "with a dependency group" do
+      let(:dependency_group) { Dependabot::DependencyGroup.new(name: "all-the-things", rules: anything) }
+      let(:source) { Dependabot::Source.new(provider: "github", repo: "gc/bp") }
+      let(:dummy_creator) { instance_double(described_class::Github) }
+
+      subject(:creator_with_group) do
+        described_class.new(
+          source: source,
+          base_commit: base_commit,
+          dependencies: dependencies,
+          files: files,
+          credentials: credentials,
+          custom_labels: custom_labels,
+          reviewers: reviewers,
+          assignees: assignees,
+          milestone: milestone,
+          author_details: author_details,
+          signature_key: signature_key,
+          provider_metadata: provider_metadata,
+          dependency_group: dependency_group
+        )
+      end
+
+      it "delegates to PullRequestCreator::Github with correct params" do
+        expect(described_class::Github).
+          to receive(:new).
+          with(
+            source: source,
+            branch_name: start_with("dependabot/bundler/all-the-things/prototype-"),
+            base_commit: base_commit,
+            credentials: credentials,
+            files: files,
+            commit_message: "Commit msg",
+            pr_description: "PR msg",
+            pr_name: "PR name",
+            author_details: author_details,
+            signature_key: signature_key,
+            custom_headers: nil,
+            labeler: instance_of(described_class::Labeler),
+            reviewers: reviewers,
+            assignees: assignees,
+            milestone: milestone,
+            require_up_to_date_base: false
+          ).and_return(dummy_creator)
+        expect(dummy_creator).to receive(:create)
+        creator_with_group.create
+      end
+    end
   end
 end

updater/lib/dependabot/updater/operations/group_update_all_versions.rb

@@ -37,7 +37,7 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:)
           @dependency_snapshot = dependency_snapshot
           @error_handler = error_handler
           # This is a placeholder for a real rule object obtained from config in future
-          @dependency_group = Dependabot::DependencyGroup.new(name: GROUP_NAME_PLACEHOLDER)
+          @dependency_group = Dependabot::DependencyGroup.new(name: GROUP_NAME_PLACEHOLDER, rules: [])
         end
 
         def perform