From 45c1132d5d291f80e731d3ffd011fce995310929 Mon Sep 17 00:00:00 2001 From: Ankit Honey Date: Mon, 31 Jul 2023 11:02:42 -0700 Subject: [PATCH] Show ignore conditions in a request body (#7654) * Added ignore condition to message builder class to show the ignore conditions of the dependencies in the PR body. --- .../pull_request_creator/message_builder.rb | 65 +++- .../message_builder_spec.rb | 283 +++++++++++++++++- updater/lib/dependabot/dependency_change.rb | 3 +- updater/spec/dependabot/api_client_spec.rb | 3 +- .../spec/dependabot/dependency_change_spec.rb | 8 +- 5 files changed, 353 insertions(+), 9 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 4c81ad31f440..bd8a977afe1e 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -23,7 +23,7 @@ class MessageBuilder :pr_message_header, :pr_message_footer, :commit_message_options, :vulnerabilities_fixed, :github_redirection_service, :dependency_group, :pr_message_max_length, - :pr_message_encoding + :pr_message_encoding, :ignore_conditions TRUNCATED_MSG = "...\n\n_Description has been truncated_" @@ -31,7 +31,7 @@ def initialize(source:, dependencies:, files:, credentials:, pr_message_header: nil, pr_message_footer: nil, commit_message_options: {}, vulnerabilities_fixed: {}, github_redirection_service: DEFAULT_GITHUB_REDIRECTION_SERVICE, - dependency_group: nil, pr_message_max_length: nil, pr_message_encoding: nil) + dependency_group: nil, pr_message_max_length: nil, pr_message_encoding: nil, ignore_conditions: []) @dependencies = dependencies @files = files @source = source @@ -44,6 +44,7 @@ def initialize(source:, dependencies:, files:, credentials:, @dependency_group = dependency_group @pr_message_max_length = pr_message_max_length @pr_message_encoding = pr_message_encoding + @ignore_conditions = ignore_conditions end attr_writer :pr_message_max_length @@ -57,13 +58,31 @@ def pr_name end def pr_message - msg = "#{suffixed_pr_message_header}#{commit_message_intro}#{metadata_cascades}#{prefixed_pr_message_footer}" + # TODO: Remove unignore_commands? feature flag once we are confident + # that it is working as expected + msg = if unignore_commands? + "#{suffixed_pr_message_header}" \ + "#{commit_message_intro}" \ + "#{metadata_cascades}" \ + "#{ignore_conditions_table}" \ + "#{prefixed_pr_message_footer}" + else + "#{suffixed_pr_message_header}" \ + "#{commit_message_intro}" \ + "#{metadata_cascades}" \ + "#{prefixed_pr_message_footer}" + end + truncate_pr_message(msg) rescue StandardError => e Dependabot.logger.error("Error while generating PR message: #{e.message}") suffixed_pr_message_header + prefixed_pr_message_footer end + def unignore_commands? + Experiments.enabled?(:unignore_commands) + end + # Truncate PR message as determined by the pr_message_max_length and pr_message_encoding instance variables # The encoding is used when calculating length, all messages are returned as ruby UTF_8 encoded string def truncate_pr_message(msg) @@ -504,6 +523,46 @@ def metadata_cascades_for_dep(dependency) ).to_s end + def ignore_conditions_table + # Return an empty string if ignore_conditions is empty + return "" if @ignore_conditions.empty? + + # Filter out the conditions where from_config_file is false and dependency is in @dependencies + valid_ignore_conditions = @ignore_conditions.select do |ic| + !ic[:from_config_file] && dependencies.any? { |dep| dep.name == ic[:dependency_name] } + end + + # Return an empty string if no valid ignore conditions after filtering + return "" if valid_ignore_conditions.empty? + + # Sort them by updated_at (or created_at if updated_at is nil), taking the latest 20 + sorted_ignore_conditions = valid_ignore_conditions.sort_by { |ic| ic[:updated_at] || ic[:created_at] }.last(20) + + # Map each condition to a row string + table_rows = sorted_ignore_conditions.map do |ic| + "| #{ic[:dependency_name]} | [#{ic[:version_requirement]}] |" + end + + summary = "Most Recent Ignore Conditions Applied to This Pull Request" + build_table(summary, table_rows) + end + + def build_table(summary, rows) + table_header = "| Dependency Name | Ignore Conditions |" + table_divider = "| --- | --- |" + table_body = rows.join("\n") + body = "\n#{[table_header, table_divider, table_body].join("\n")}\n" + + if %w(azure bitbucket codecommit).include?(source.provider) + "\n##{summary}\n\n#{body}" + else + # Build the collapsible section + msg = "
\n#{summary}\n\n" \ + "#{[table_header, table_divider, table_body].join("\n")}\n
" + "\n#{msg}\n" + end + end + def changelog_url(dependency) metadata_finder(dependency).changelog_url end diff --git a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb index 02c568f9fb2f..2f9948781c81 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -18,7 +18,8 @@ commit_message_options: commit_message_options, vulnerabilities_fixed: vulnerabilities_fixed, github_redirection_service: github_redirection_service, - dependency_group: dependency_group + dependency_group: dependency_group, + ignore_conditions: ignore_conditions ) end @@ -48,6 +49,7 @@ let(:vulnerabilities_fixed) { { "business" => [] } } let(:github_redirection_service) { "redirect.github.com" } let(:dependency_group) { nil } + let(:ignore_conditions) { [] } let(:gemfile) do Dependabot::DependencyFile.new( @@ -2214,6 +2216,285 @@ def commits_details(base:, head:) end end + context "with ignore conditions when feature flag is enabled", :vcr do + let(:dependency2) do + Dependabot::Dependency.new( + name: "business2", + version: "1.8.0", + previous_version: "1.7.0", + package_manager: "dummy", + requirements: [], + previous_requirements: [] + ) + end + let(:dependency3) do + Dependabot::Dependency.new( + name: "business3", + version: "1.5.0", + previous_version: "1.4.0", + package_manager: "dummy", + requirements: [], + previous_requirements: [] + ) + end + let(:dependency4) do + Dependabot::Dependency.new( + name: "business4", + version: "2.1.1", + previous_version: "2.1.0", + package_manager: "dummy", + requirements: [], + previous_requirements: [] + ) + end + let(:dependency5) do + Dependabot::Dependency.new( + name: "business5", + version: "0.17.0", + previous_version: "0.16.2", + package_manager: "dummy", + requirements: [], + previous_requirements: [] + ) + end + let(:dependencies) { [dependency, dependency2, dependency3, dependency4, dependency5] } + + before do + (2..5).each do |i| + repo_url = "https://api.github.com/repos/gocardless/business#{i}" + + stub_request(:get, repo_url). + to_return(status: 200, + body: fixture("github", "business_repo.json"), + headers: json_header) + stub_request(:get, "#{repo_url}/contents/"). + to_return(status: 200, + body: fixture("github", "business_files.json"), + headers: json_header) + stub_request(:get, "#{repo_url}/releases?per_page=100"). + to_return(status: 200, + body: fixture("github", "business_releases.json"), + headers: json_header) + stub_request(:get, "https://api.github.com/repos/gocardless/" \ + "business#{i}/contents/CHANGELOG.md?ref=master"). + to_return(status: 200, + body: fixture("github", "changelog_contents.json"), + headers: json_header) + stub_request(:get, "https://rubygems.org/api/v1/gems/business#{i}.json"). + to_return( + status: 200, + body: fixture("ruby", "rubygems_response_statesman.json") + ) + + service_pack_url = + "https://github.com/gocardless/business#{i}.git/info/refs" \ + "?service=git-upload-pack" + + stub_request(:get, service_pack_url). + to_return( + status: 200, + body: fixture("git", "upload_packs", "no_tags"), + headers: { + "content-type" => "application/x-git-upload-pack-advertisement" + } + + ) + ignore_conditions.push( + { + dependency_name: "business#{i}", + version_requirement: "<= 1.#{i}.0", + from_config_file: false, + updated_at: Time.now, + created_at: Time.now - (i * 86_400) + } + ) + end + Dependabot::Experiments.register(:unignore_commands, true) + end + + it "has the correct message" do + expect(pr_message).to include( + "| Dependency Name | Ignore Conditions |\n" \ + "| --- | --- |\n" \ + "| business2 | [<= 1.2.0] |\n" \ + "| business3 | [<= 1.3.0] |\n" \ + "| business4 | [<= 1.4.0] |\n" \ + "| business5 | [<= 1.5.0] |\n" + ) + end + end + + context "with ignore conditions when feature flag is disabled", :vcr do + let(:dependency2) do + Dependabot::Dependency.new( + name: "business2", + version: "1.8.0", + previous_version: "1.7.0", + package_manager: "dummy", + requirements: [], + previous_requirements: [] + ) + end + let(:dependency3) do + Dependabot::Dependency.new( + name: "business3", + version: "1.5.0", + previous_version: "1.4.0", + package_manager: "dummy", + requirements: [], + previous_requirements: [] + ) + end + let(:dependency4) do + Dependabot::Dependency.new( + name: "business4", + version: "2.1.1", + previous_version: "2.1.0", + package_manager: "dummy", + requirements: [], + previous_requirements: [] + ) + end + let(:dependency5) do + Dependabot::Dependency.new( + name: "business5", + version: "0.17.0", + previous_version: "0.16.2", + package_manager: "dummy", + requirements: [], + previous_requirements: [] + ) + end + let(:dependencies) { [dependency, dependency2, dependency3, dependency4, dependency5] } + + before do + (2..5).each do |i| + repo_url = "https://api.github.com/repos/gocardless/business#{i}" + + stub_request(:get, repo_url). + to_return(status: 200, + body: fixture("github", "business_repo.json"), + headers: json_header) + stub_request(:get, "#{repo_url}/contents/"). + to_return(status: 200, + body: fixture("github", "business_files.json"), + headers: json_header) + stub_request(:get, "#{repo_url}/releases?per_page=100"). + to_return(status: 200, + body: fixture("github", "business_releases.json"), + headers: json_header) + stub_request(:get, "https://api.github.com/repos/gocardless/" \ + "business#{i}/contents/CHANGELOG.md?ref=master"). + to_return(status: 200, + body: fixture("github", "changelog_contents.json"), + headers: json_header) + stub_request(:get, "https://rubygems.org/api/v1/gems/business#{i}.json"). + to_return( + status: 200, + body: fixture("ruby", "rubygems_response_statesman.json") + ) + + service_pack_url = + "https://github.com/gocardless/business#{i}.git/info/refs" \ + "?service=git-upload-pack" + + stub_request(:get, service_pack_url). + to_return( + status: 200, + body: fixture("git", "upload_packs", "no_tags"), + headers: { + "content-type" => "application/x-git-upload-pack-advertisement" + } + + ) + ignore_conditions.push( + { + dependency_name: "business#{i}", + version_requirement: "<= 1.#{i}.0", + from_config_file: false, + updated_at: Time.now, + created_at: Time.now - (i * 86_400) + } + ) + end + Dependabot::Experiments.register(:unignore_commands, false) + end + + it "does not include the ignore conditions section in the message" do + expect(pr_message).not_to include("Most Recent Ignore Conditions Applied to This Pull Request") + end + end + + context "without ignore conditions", :vcr do + let(:dependency1) do + Dependabot::Dependency.new( + name: "business2", + version: "1.8.0", + previous_version: "1.7.0", + package_manager: "dummy", + requirements: [], + previous_requirements: [] + ) + end + let(:dependency2) do + Dependabot::Dependency.new( + name: "business3", + version: "1.5.0", + previous_version: "1.4.0", + package_manager: "dummy", + requirements: [], + previous_requirements: [] + ) + end + let(:dependencies) { [dependency1, dependency2] } + + before do + (2..5).each do |i| + repo_url = "https://api.github.com/repos/gocardless/business#{i}" + + stub_request(:get, repo_url). + to_return(status: 200, + body: fixture("github", "business_repo.json"), + headers: json_header) + stub_request(:get, "#{repo_url}/contents/"). + to_return(status: 200, + body: fixture("github", "business_files.json"), + headers: json_header) + stub_request(:get, "#{repo_url}/releases?per_page=100"). + to_return(status: 200, + body: fixture("github", "business_releases.json"), + headers: json_header) + stub_request(:get, "https://api.github.com/repos/gocardless/" \ + "business#{i}/contents/CHANGELOG.md?ref=master"). + to_return(status: 200, + body: fixture("github", "changelog_contents.json"), + headers: json_header) + stub_request(:get, "https://rubygems.org/api/v1/gems/business#{i}.json"). + to_return( + status: 200, + body: fixture("ruby", "rubygems_response_statesman.json") + ) + + service_pack_url = + "https://github.com/gocardless/business#{i}.git/info/refs" \ + "?service=git-upload-pack" + + stub_request(:get, service_pack_url). + to_return( + status: 200, + body: fixture("git", "upload_packs", "no_tags"), + headers: { + "content-type" => "application/x-git-upload-pack-advertisement" + } + ) + end + end + + it "does not include the ignore conditions section in the message" do + expect(pr_message).not_to include("Most Recent Ignore Conditions Applied to This Pull Request") + end + end + context "with a directory specified" do let(:gemfile) do Dependabot::DependencyFile.new( diff --git a/updater/lib/dependabot/dependency_change.rb b/updater/lib/dependabot/dependency_change.rb index 5e72084e91ab..04ecf5f6030e 100644 --- a/updater/lib/dependabot/dependency_change.rb +++ b/updater/lib/dependabot/dependency_change.rb @@ -29,7 +29,8 @@ def pr_message files: updated_dependency_files, credentials: job.credentials, commit_message_options: job.commit_message_options, - dependency_group: dependency_group + dependency_group: dependency_group, + ignore_conditions: job.ignore_conditions ).message end diff --git a/updater/spec/dependabot/api_client_spec.rb b/updater/spec/dependabot/api_client_spec.rb index 024996ffdb20..ca0797aa9ecf 100644 --- a/updater/spec/dependabot/api_client_spec.rb +++ b/updater/spec/dependabot/api_client_spec.rb @@ -22,7 +22,8 @@ source: nil, credentials: [], commit_message_options: [], - updating_a_pull_request?: false) + updating_a_pull_request?: false, + ignore_conditions: []) end let(:dependencies) do [dependency] diff --git a/updater/spec/dependabot/dependency_change_spec.rb b/updater/spec/dependabot/dependency_change_spec.rb index aba2db46a242..1d2175ed3400 100644 --- a/updater/spec/dependabot/dependency_change_spec.rb +++ b/updater/spec/dependabot/dependency_change_spec.rb @@ -14,7 +14,7 @@ end let(:job) do - instance_double(Dependabot::Job) + instance_double(Dependabot::Job, ignore_conditions: []) end let(:updated_dependencies) do @@ -100,7 +100,8 @@ dependencies: updated_dependencies, credentials: job_credentials, commit_message_options: commit_message_options, - dependency_group: nil + dependency_group: nil, + ignore_conditions: [] ) expect(dependency_change.pr_message).to eql("Hello World!") @@ -124,7 +125,8 @@ dependencies: updated_dependencies, credentials: job_credentials, commit_message_options: commit_message_options, - dependency_group: group + dependency_group: group, + ignore_conditions: [] ) expect(dependency_change.pr_message).to eql("Hello World!")