Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass the serialized group name when creating a PR for grouped updates #7166

Merged
merged 9 commits into from
Apr 26, 2023
13 changes: 12 additions & 1 deletion common/lib/dependabot/dependency_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,28 @@

module Dependabot
class DependencyGroup
attr_reader :name, :rules, :dependencies
attr_reader :name, :rules, :dependencies, :id

def initialize(name:, rules:)
@name = name
@rules = rules
@dependencies = []
@id = id_hash
end

def contains?(dependency)
@dependencies.include?(dependency) if @dependencies.any?
rules.any? { |rule| WildcardMatcher.match?(rule, dependency.name) }
end

private

def id_hash
{ "name" => serialized_group_name }
end
Nishnha marked this conversation as resolved.
Show resolved Hide resolved

def serialized_group_name
name.downcase.gsub("-", "_").to_sym
end
Nishnha marked this conversation as resolved.
Show resolved Hide resolved
end
end
22 changes: 11 additions & 11 deletions updater/lib/dependabot/api_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,15 @@ def http_client
client
end

def dependency_group_hash(dependency_change)
return {} unless dependency_change.grouped_update?

# FIXME: We currently assumpt that _an attempt_ to send a DependencyGroup#id should
# result in the `grouped-update` flag being set, regardless of whether the
# DependencyGroup actually exists.
{ "dependency-group": dependency_change.dependency_group.id }.compact
Nishnha marked this conversation as resolved.
Show resolved Hide resolved
end

def create_pull_request_data(dependency_change, base_commit_sha)
data = {
dependencies: dependency_change.updated_dependencies.map do |dep|
Expand All @@ -181,17 +190,8 @@ def create_pull_request_data(dependency_change, base_commit_sha)
end,
"updated-dependency-files": dependency_change.updated_dependency_files_hash,
"base-commit-sha": base_commit_sha
}.merge({
# TODO: Replace this flag with a dependency-group object
#
# In future this should be something like:
# "dependency-group": dependency_change.dependency_group_hash
#
# This will allow us to pass back the rule id and other parameters
# to allow Dependabot API to augment PR creation and associate it
# with the rule for rebasing, etc.
"grouped-update": dependency_change.grouped_update? ? true : nil
}.compact)
}.merge(dependency_group_hash(dependency_change))

return data unless dependency_change.pr_message

data["commit-message"] = dependency_change.pr_message.commit_message
Expand Down
15 changes: 11 additions & 4 deletions updater/spec/dependabot/api_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,22 +175,25 @@
end

context "grouped updates" do
it "does not include the grouped_update key by default" do
it "does not include the grouped-update or dependency-group keys by default" do
Nishnha marked this conversation as resolved.
Show resolved Hide resolved
client.create_pull_request(dependency_change, base_commit)

expect(WebMock).
to(have_requested(:post, create_pull_request_url).
with do |req|
expect(req.body).not_to include("grouped-update")
Nishnha marked this conversation as resolved.
Show resolved Hide resolved
expect(req.body).not_to include("dependency-group")
end)
end

it "flags the PR as a grouped-update if the dependency change has a dependency group assigned" do
it "flags the PR as having dependency-groups if the dependency change has a dependency group assigned" do
group = Dependabot::DependencyGroup.new(name: "dummy-group-name", rules: ["*"])

grouped_dependency_change = Dependabot::DependencyChange.new(
job: job,
updated_dependencies: dependencies,
updated_dependency_files: dependency_files,
dependency_group: anything
dependency_group: group
)

client.create_pull_request(grouped_dependency_change, base_commit)
Expand All @@ -199,7 +202,11 @@
to(have_requested(:post, create_pull_request_url).
with do |req|
data = JSON.parse(req.body)["data"]
expect(data["grouped-update"]).to be true
# FIXME: Once we fully remove the `group-all` prototype behavior, we can also remove the
# `grouped-update` expectation since dependency-groups will require rules and only
# return the `dependency-group` key
expect(data["grouped-update"]).to be nil
Nishnha marked this conversation as resolved.
Show resolved Hide resolved
expect(data["dependency-group"]).to eq({ "name" => "dummy_group_name" })
end)
end
end
Expand Down