From 5ade344555922cac95a60606efe855fd30772cd6 Mon Sep 17 00:00:00 2001 From: Kamil Bukum Date: Wed, 21 Aug 2024 20:39:16 -0700 Subject: [PATCH] replaced warning `message` as `description` to reduce confussion logged deprecation warning. --- common/lib/dependabot/notices.rb | 115 +++++++++++------- .../pull_request_creator/message_builder.rb | 4 +- common/spec/dependabot/notices_spec.rb | 62 ++++++---- .../message_builder_spec.rb | 45 ++++--- .../updater/group_update_creation.rb | 3 + .../create_security_update_pull_request.rb | 9 +- .../refresh_security_update_pull_request.rb | 9 +- .../refresh_version_update_pull_request.rb | 9 +- .../updater/operations/update_all_versions.rb | 9 +- .../updater/security_update_helpers.rb | 55 ++++++++- .../create_group_update_pull_request_spec.rb | 21 ++-- ...reate_security_update_pull_request_spec.rb | 13 +- ...fresh_security_update_pull_request_spec.rb | 17 +-- ...efresh_version_update_pull_request_spec.rb | 5 +- .../operations/update_all_versions_spec.rb | 9 +- .../updater/pull_request_helpers_spec.rb | 80 +++++++++--- 16 files changed, 316 insertions(+), 149 deletions(-) diff --git a/common/lib/dependabot/notices.rb b/common/lib/dependabot/notices.rb index 9daef0c302a1..7c926fdf5ac2 100644 --- a/common/lib/dependabot/notices.rb +++ b/common/lib/dependabot/notices.rb @@ -6,32 +6,54 @@ module Dependabot class Notice + module NoticeMode + INFO = "INFO" + WARN = "WARN" + ERROR = "ERROR" + end + extend T::Sig sig { returns(String) } - attr_reader :mode, :type, :package_manager_name, :message, :markdown + attr_reader :mode, :type, :package_manager_name, :title, :description, :markdown + + sig { returns(T::Boolean) } + attr_reader :show_in_pr, :show_in_log # Initializes a new Notice object. # @param mode [String] The mode of the notice (e.g., "WARN", "ERROR"). # @param type [String] The type of the notice (e.g., "bundler_deprecated_warn"). # @param package_manager_name [String] The name of the package manager (e.g., "bundler"). - # @param message [String] The main message of the notice. - # @param markdown [String] The markdown formatted message. + # @param title [String] The title of the notice. + # @param description [String] The main description of the notice. + # @param markdown [String] The markdown formatted description. + # @param show_in_pr [Boolean] Whether the notice should be shown in a pull request. + # @param show_in_log [Boolean] Whether the notice should be shown in the log. sig do params( mode: String, type: String, package_manager_name: String, - message: String, - markdown: String + title: String, + description: String, + markdown: String, + show_in_pr: T::Boolean, + show_in_log: T::Boolean ).void end - def initialize(mode:, type:, package_manager_name:, message: "", markdown: "") + def initialize( + mode:, type:, package_manager_name:, + title: "", description: "", markdown: "", + show_in_pr: false, show_in_log: true + ) @mode = mode @type = type @package_manager_name = package_manager_name - @message = message + @title = title + @description = description @markdown = markdown + @show_in_pr = show_in_pr + @show_in_log = show_in_log end # Converts the Notice object to a hash. @@ -42,22 +64,25 @@ def to_hash mode: @mode, type: @type, package_manager_name: @package_manager_name, - message: @message, - markdown: @markdown + title: @title, + description: @description, + markdown: @markdown, + show_in_pr: @show_in_pr, + show_in_log: @show_in_log } end - # Generates a message for supported versions. + # Generates a description for supported versions. # @param supported_versions [Array, nil] The supported versions of the package manager. # @param support_later_versions [Boolean] Whether later versions are supported. - # @return [String, nil] The generated message or nil if no supported versions are provided. + # @return [String, nil] The generated description or nil if no supported versions are provided. sig do params( supported_versions: T.nilable(T::Array[Dependabot::Version]), support_later_versions: T::Boolean ).returns(String) end - def self.generate_supported_versions_message(supported_versions, support_later_versions) + def self.generate_supported_versions_description(supported_versions, support_later_versions) return "" unless supported_versions&.any? versions_string = supported_versions.map { |version| "`v#{version}`" } @@ -66,11 +91,11 @@ def self.generate_supported_versions_message(supported_versions, support_later_v versions_string = versions_string.join(", ") - later_message = support_later_versions ? ", or later" : "" + later_description = support_later_versions ? ", or later" : "" - return "Please upgrade to version #{versions_string}#{later_message}." if supported_versions.count == 1 + return "Please upgrade to version #{versions_string}#{later_description}." if supported_versions.count == 1 - "Please upgrade to one of the following versions: #{versions_string}#{later_message}." + "Please upgrade to one of the following versions: #{versions_string}#{later_description}." end # Generates a support notice for the given package manager. @@ -100,30 +125,34 @@ def self.generate_support_notice(package_manager) def self.generate_pm_deprecation_notice(package_manager) return nil unless package_manager.deprecated? - mode = "WARN" - supported_versions_message = generate_supported_versions_message( + mode = NoticeMode::WARN + supported_versions_description = generate_supported_versions_description( package_manager.supported_versions, package_manager.support_later_versions? ) - notice_type = "#{package_manager.name}_deprecated_#{mode.downcase}" - message = "Dependabot will stop supporting `#{package_manager.name} v#{package_manager.version}`!" - ## Create a warning markdown message + notice_type = "#{package_manager.name}_deprecated_warn" + title = "Package manager deprecation notice" + description = "Dependabot will stop supporting `#{package_manager.name} v#{package_manager.version}`!" + ## Create a warning markdown description markdown = "> [!WARNING]\n" - ## Add the deprecation warning to the message - markdown += "> #{message}\n>\n" + ## Add the deprecation warning to the description + markdown += "> #{description}\n>\n" - ## Add the supported versions to the message - unless supported_versions_message.empty? - message += "\n#{supported_versions_message}\n" - markdown += "> #{supported_versions_message}\n>\n" + ## Add the supported versions to the description + unless supported_versions_description.empty? + description += "\n#{supported_versions_description}\n" + markdown += "> #{supported_versions_description}\n>\n" end Notice.new( mode: mode, type: notice_type, package_manager_name: package_manager.name, - message: message, - markdown: markdown + title: title, + description: description, + markdown: markdown, + show_in_pr: true, + show_in_log: true ) end @@ -138,30 +167,34 @@ def self.generate_pm_deprecation_notice(package_manager) def self.generate_pm_unsupported_notice(package_manager) return nil unless package_manager.unsupported? - mode = "ERROR" - supported_versions_message = generate_supported_versions_message( + mode = NoticeMode::ERROR + supported_versions_description = generate_supported_versions_description( package_manager.supported_versions, package_manager.support_later_versions? ) - notice_type = "#{package_manager.name}_unsupported_#{mode.downcase}" - message = "Dependabot no longer supports `#{package_manager.name} v#{package_manager.version}`!" - ## Create an error markdown message + notice_type = "#{package_manager.name}_unsupported_error" + title = "Package manager unsupported notice" + description = "Dependabot no longer supports `#{package_manager.name} v#{package_manager.version}`!" + ## Create an error markdown description markdown = "> [!IMPORTANT]\n" - ## Add the error message to the message - markdown += "> #{message}\n>\n" + ## Add the error description to the description + markdown += "> #{description}\n>\n" - ## Add the supported versions to the message - unless supported_versions_message.empty? - message += "\n#{supported_versions_message}\n" - markdown += "> #{supported_versions_message}\n>\n" + ## Add the supported versions to the description + unless supported_versions_description.empty? + description += "\n#{supported_versions_description}\n" + markdown += "> #{supported_versions_description}\n>\n" end Notice.new( mode: mode, type: notice_type, package_manager_name: package_manager.name, - message: message, - markdown: markdown + title: title, + description: description, + markdown: markdown, + show_in_pr: true, + show_in_log: true ) end end diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 1968cfb3f6c0..a4d1f46322bd 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -143,8 +143,8 @@ def pr_message def pr_notices notices = @notices || [] unique_messages = notices.filter_map do |notice| - markdown = notice.markdown if notice - markdown unless markdown.empty? + markdown = notice.markdown if notice.show_in_pr + markdown if markdown && !markdown.empty? end.uniq message = unique_messages.join("\n\n") diff --git a/common/spec/dependabot/notices_spec.rb b/common/spec/dependabot/notices_spec.rb index c15a64a2bbc7..df4a5893db63 100644 --- a/common/spec/dependabot/notices_spec.rb +++ b/common/spec/dependabot/notices_spec.rb @@ -26,17 +26,17 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end RSpec.describe Dependabot::Notice do - describe ".generate_supported_versions_message" do - subject(:generate_supported_versions_message) do - described_class.generate_supported_versions_message(supported_versions, support_later_versions) + describe ".generate_supported_versions_description" do + subject(:generate_supported_versions_description) do + described_class.generate_supported_versions_description(supported_versions, support_later_versions) end context "when supported_versions has one version" do let(:supported_versions) { [Dependabot::Version.new("2")] } let(:support_later_versions) { false } - it "returns the correct message" do - expect(generate_supported_versions_message) + it "returns the correct description" do + expect(generate_supported_versions_description) .to eq("Please upgrade to version `v2`.") end end @@ -45,8 +45,8 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ let(:supported_versions) { [Dependabot::Version.new("2")] } let(:support_later_versions) { true } - it "returns the correct message" do - expect(generate_supported_versions_message) + it "returns the correct description" do + expect(generate_supported_versions_description) .to eq("Please upgrade to version `v2`, or later.") end end @@ -58,8 +58,8 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end let(:support_later_versions) { false } - it "returns the correct message" do - expect(generate_supported_versions_message) + it "returns the correct description" do + expect(generate_supported_versions_description) .to eq("Please upgrade to one of the following versions: `v2`, `v3`, or `v4`.") end end @@ -71,8 +71,8 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end let(:support_later_versions) { true } - it "returns the correct message" do - expect(generate_supported_versions_message) + it "returns the correct description" do + expect(generate_supported_versions_description) .to eq("Please upgrade to one of the following versions: `v2`, `v3`, `v4`, or later.") end end @@ -82,7 +82,7 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ let(:support_later_versions) { false } it "returns empty string" do - expect(generate_supported_versions_message).to eq("") + expect(generate_supported_versions_description).to eq("") end end @@ -91,7 +91,7 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ let(:support_later_versions) { false } it "returns nil" do - expect(generate_supported_versions_message).to eq("") + expect(generate_supported_versions_description).to eq("") end end end @@ -124,10 +124,13 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true }) end end @@ -142,10 +145,13 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ mode: "ERROR", type: "bundler_unsupported_error", package_manager_name: "bundler", - message: "Dependabot no longer supports `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + title: "Package manager unsupported notice", + description: "Dependabot no longer supports `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", markdown: "> [!IMPORTANT]\n> Dependabot no longer supports `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true }) end end @@ -194,10 +200,13 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true }) end end @@ -222,10 +231,13 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ mode: "ERROR", type: "bundler_unsupported_error", package_manager_name: "bundler", - message: "Dependabot no longer supports `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + title: "Package manager unsupported notice", + description: "Dependabot no longer supports `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", markdown: "> [!IMPORTANT]\n> Dependabot no longer supports `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true }) end 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 d9f6acb7481f..f2fb40847ba8 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -3324,10 +3324,13 @@ def commits_details(base:, head:) mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true )] end @@ -3342,18 +3345,24 @@ def commits_details(base:, head:) mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true ), Dependabot::Notice.new( mode: "ERROR", type: "bundler_unsupported_error", package_manager_name: "bundler", - message: "Dependabot no longer supports `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + title: "Package manager deprecation notice", + description: "Dependabot no longer supports `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", markdown: "> [!IMPORTANT]\n> Dependabot no longer supports `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true )] end @@ -3370,18 +3379,24 @@ def commits_details(base:, head:) mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true ), Dependabot::Notice.new( mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true )] end diff --git a/updater/lib/dependabot/updater/group_update_creation.rb b/updater/lib/dependabot/updater/group_update_creation.rb index 96458e2f3c4f..13b750692ff9 100644 --- a/updater/lib/dependabot/updater/group_update_creation.rb +++ b/updater/lib/dependabot/updater/group_update_creation.rb @@ -123,6 +123,9 @@ def compile_all_dependency_changes_for(group) notices: notices ) + # Record any warning notices that were generated during the update process if show_in_log is true + record_warning_notices(notices) + if Experiments.enabled?("dependency_change_validation") && !dependency_change.all_have_previous_version? log_missing_previous_version(dependency_change) return nil diff --git a/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb b/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb index ac69fe2d56ed..5d1d4067eef9 100644 --- a/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb @@ -46,7 +46,7 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) # TODO: Collect @created_pull_requests on the Job object? @created_pull_requests = T.let([], T::Array[PullRequest]) - @pr_notices = T.let([], T::Array[Dependabot::Notice]) + @notices = T.let([], T::Array[Dependabot::Notice]) end # TODO: We currently tolerate multiple dependencies for this operation @@ -61,7 +61,7 @@ def perform # Add a deprecation notice if the package manager is deprecated add_deprecation_notice( - notices: @pr_notices, + notices: @notices, package_manager: dependency_snapshot.package_manager ) @@ -180,9 +180,12 @@ def check_and_create_pull_request(dependency) dependency_files: dependency_snapshot.dependency_files, updated_dependencies: updated_deps, change_source: checker.dependency, - notices: @pr_notices + notices: @notices ) + # Record any warning notices that were generated during the update process if conditions are met + record_warning_notices(@notices) + create_pull_request(dependency_change) rescue Dependabot::AllVersionsIgnored Dependabot.logger.info("All updates for #{dependency.name} were ignored") diff --git a/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb index de8ac93120ec..564f19badac2 100644 --- a/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb @@ -46,7 +46,7 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) @dependency_snapshot = dependency_snapshot @error_handler = error_handler - @pr_notices = T.let([], T::Array[Dependabot::Notice]) + @notices = T.let([], T::Array[Dependabot::Notice]) end sig { void } @@ -56,7 +56,7 @@ def perform # Add a deprecation notice if the package manager is deprecated add_deprecation_notice( - notices: @pr_notices, + notices: @notices, package_manager: dependency_snapshot.package_manager ) @@ -158,9 +158,12 @@ def check_and_update_pull_request(dependencies) dependency_files: dependency_snapshot.dependency_files, updated_dependencies: updated_deps, change_source: checker.dependency, - notices: @pr_notices + notices: @notices ) + # Record any warning notices that were generated during the update process if conditions are met + record_warning_notices(@notices) + # NOTE: Gradle, Maven and Nuget dependency names can be case-insensitive # and the dependency name in the security advisory often doesn't match # what users have specified in their manifest. diff --git a/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb index 5da1fce0ac88..430610d72a26 100644 --- a/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb @@ -47,7 +47,7 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) @dependency_snapshot = dependency_snapshot @error_handler = error_handler - @pr_notices = T.let([], T::Array[Dependabot::Notice]) + @notices = T.let([], T::Array[Dependabot::Notice]) return unless job.source.directory.nil? && job.source.directories&.count == 1 @@ -62,7 +62,7 @@ def perform # Add a deprecation notice if the package manager is deprecated add_deprecation_notice( - notices: @pr_notices, + notices: @notices, package_manager: dependency_snapshot.package_manager ) @@ -146,9 +146,12 @@ def check_and_update_pull_request(dependencies) dependency_files: dependency_snapshot.dependency_files, updated_dependencies: updated_deps, change_source: checker.dependency, - notices: @pr_notices + notices: @notices ) + # Record any warning notices that were generated during the update process if conditions are met + record_warning_notices(@notices) + # NOTE: Gradle, Maven and Nuget dependency names can be case-insensitive # and the dependency name in the security advisory often doesn't match # what users have specified in their manifest. diff --git a/updater/lib/dependabot/updater/operations/update_all_versions.rb b/updater/lib/dependabot/updater/operations/update_all_versions.rb index 9730766f7512..35a2f95f0dd0 100644 --- a/updater/lib/dependabot/updater/operations/update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/update_all_versions.rb @@ -40,7 +40,7 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) # TODO: Collect @created_pull_requests on the Job object? @created_pull_requests = T.let([], T::Array[PullRequest]) - @pr_notices = T.let([], T::Array[Dependabot::Notice]) + @notices = T.let([], T::Array[Dependabot::Notice]) return unless job.source.directory.nil? && job.source.directories&.count == 1 @@ -54,7 +54,7 @@ def perform # Add a deprecation notice if the package manager is deprecated add_deprecation_notice( - notices: @pr_notices, + notices: @notices, package_manager: dependency_snapshot.package_manager ) @@ -167,13 +167,16 @@ def check_and_create_pull_request(dependency) dependency_files: dependency_snapshot.dependency_files, updated_dependencies: updated_deps, change_source: checker.dependency, - notices: @pr_notices + notices: @notices ) if dependency_change.updated_dependency_files.empty? raise "UpdateChecker found viable dependencies to be updated, but FileUpdater failed to update any files" end + # Record any warning notices that were generated during the update process if conditions are met + record_warning_notices(@notices) + create_pull_request(dependency_change) end # rubocop:enable Metrics/PerceivedComplexity diff --git a/updater/lib/dependabot/updater/security_update_helpers.rb b/updater/lib/dependabot/updater/security_update_helpers.rb index 253e8fbd39af..ec0b512b53d8 100644 --- a/updater/lib/dependabot/updater/security_update_helpers.rb +++ b/updater/lib/dependabot/updater/security_update_helpers.rb @@ -199,20 +199,63 @@ module PullRequestHelpers .void end def add_deprecation_notice(notices:, package_manager:) - return unless Dependabot::Experiments.enabled?( - :add_deprecation_warn_to_pr_message - ) + deprecation_notice = create_deprecation_notice(package_manager) + + return unless deprecation_notice + + notices << deprecation_notice + end + + sig { params(notices: T.nilable(T::Array[Dependabot::Notice])).void } + def record_warning_notices(notices) + return if !notices || notices.empty? + + # Find unique warning notices which are going to be shown on insight page. + warn_notices = unique_warn_notices(notices) + + warn_notices.each do |notice| + # Log notice eif show in log is enabled. + next unless notice.show_in_log + + log_notice_description(notice) + end + rescue StandardError => e + Dependabot.logger.error( + "Failed to send package manager deprecation notice warning: #{e.message}" + ) + end + + private + + sig { params(package_manager: T.nilable(PackageManagerBase)).returns(T.nilable(Dependabot::Notice)) } + def create_deprecation_notice(package_manager) + # Feature flag check if deprecation notice should be added to notices. + return unless Dependabot::Experiments.enabled?(:add_deprecation_warn_to_pr_message) + return unless package_manager return unless package_manager.is_a?(PackageManagerBase) - deprecation_notice = Notice.generate_pm_deprecation_notice( + Notice.generate_pm_deprecation_notice( package_manager ) + end - return unless deprecation_notice + # Resurns unique warning notices which are going to be shown on insight page. + sig { params(notices: T::Array[Dependabot::Notice]).returns(T::Array[Dependabot::Notice]) } + def unique_warn_notices(notices) + notices + .select { |notice| notice.mode == Dependabot::Notice::NoticeMode::WARN } + .uniq { |notice| [notice.type, notice.package_manager_name] } + end - notices << deprecation_notice + sig { params(notice: Dependabot::Notice).void } + def log_notice_description(notice) + # Log each non-empty line of the deprecation notice description + notice.description.each_line do |line| + line = line.strip + Dependabot.logger.warn(line) unless line.empty? + end end end end diff --git a/updater/spec/dependabot/updater/operations/create_group_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/create_group_update_pull_request_spec.rb index e3f7fb287cf9..212102de0939 100644 --- a/updater/spec/dependabot/updater/operations/create_group_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/create_group_update_pull_request_spec.rb @@ -122,21 +122,25 @@ mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true ).to_hash ] ) end before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) + allow(Dependabot::UpdateCheckers).to receive(:for_package_manager).and_return(stub_update_checker_class) allow(Dependabot::DependencyChangeBuilder) .to receive(:create_from) .and_return(stub_dependency_change) - allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) end after do @@ -162,10 +166,13 @@ mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true } ) create_group_update_pull_request.perform diff --git a/updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb index 4d9a08cb5a3b..22d6622468f6 100644 --- a/updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb @@ -203,6 +203,8 @@ def support_later_versions? let(:mock_package_manager_instance) { concrete_package_manager_class.new } before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) + # Allow for_package_manager to return the stub_update_checker_class allow(Dependabot::UpdateCheckers).to receive(:for_package_manager).and_return(stub_update_checker_class) @@ -219,8 +221,6 @@ def support_later_versions? allow(dependency_snapshot) .to receive(:package_manager) .and_return(mock_package_manager_instance) - - allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) end after do @@ -369,10 +369,13 @@ def support_later_versions? mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true ) ) diff --git a/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb index f2609a16e1df..2a76e704341b 100644 --- a/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb @@ -112,11 +112,12 @@ end before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) + allow(Dependabot::UpdateCheckers).to receive(:for_package_manager).and_return(stub_update_checker_class) allow(Dependabot::DependencyChangeBuilder) .to receive(:create_from) .and_return(stub_dependency_change) - allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) end after do @@ -206,13 +207,13 @@ mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - details: { - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - current_version: "v1", - markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" - } + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true }]) expect(refresh_security_update_pull_request).to receive(:create_pull_request) refresh_security_update_pull_request.send(:check_and_update_pull_request, [dependency]) diff --git a/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb index 146dcf1fef90..b14bb7988441 100644 --- a/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb @@ -143,14 +143,13 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) + allow(Dependabot::UpdateCheckers).to receive(:for_package_manager).and_return(stub_update_checker_class) allow(Dependabot::DependencyChangeBuilder) .to receive(:create_from) .and_return(stub_dependency_change) allow(dependency_snapshot).to receive(:package_manager).and_return(package_manager) - allow(Dependabot::Experiments).to receive(:enabled?).with( - :add_deprecation_warn_to_pr_message - ).and_return(true) end after do diff --git a/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb b/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb index e50fb27907bb..6cd7d6f48074 100644 --- a/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb +++ b/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb @@ -152,6 +152,8 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) + allow(Dependabot::UpdateCheckers).to receive( :for_package_manager ).and_return(stub_update_checker_class) @@ -161,9 +163,6 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ allow(dependency_snapshot).to receive( :package_manager ).and_return(package_manager) - allow(Dependabot::Experiments).to receive(:enabled?).with( - :add_deprecation_warn_to_pr_message - ).and_return(true) end after do @@ -210,7 +209,7 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end end - context "when package manager version is 1" do + context "when package manager version is deprecated" do let(:package_manager_version) { "1" } it "creates a pull request" do @@ -225,7 +224,7 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end end - context "when package manager version is 2" do + context "when package manager version is not deprecated" do let(:package_manager_version) { "2" } it "creates a pull request" do diff --git a/updater/spec/dependabot/updater/pull_request_helpers_spec.rb b/updater/spec/dependabot/updater/pull_request_helpers_spec.rb index 977490748c67..f5cd117341e2 100644 --- a/updater/spec/dependabot/updater/pull_request_helpers_spec.rb +++ b/updater/spec/dependabot/updater/pull_request_helpers_spec.rb @@ -21,6 +21,26 @@ def initialize let(:dummy_instance) { dummy_class.new } + let(:package_manager) do + Class.new(Dependabot::PackageManagerBase) do + def name + "bundler" + end + + def version + Dependabot::Version.new("1") + end + + def deprecated_versions + [Dependabot::Version.new("1")] + end + + def supported_versions + [Dependabot::Version.new("2"), Dependabot::Version.new("3")] + end + end.new + end + before do allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) end @@ -30,26 +50,6 @@ def initialize end describe "#add_deprecation_notice" do - let(:package_manager) do - Class.new(Dependabot::PackageManagerBase) do - def name - "bundler" - end - - def version - Dependabot::Version.new("1") - end - - def deprecated_versions - [Dependabot::Version.new("1")] - end - - def supported_versions - [Dependabot::Version.new("2"), Dependabot::Version.new("3")] - end - end.new - end - context "when package manager is provided and is deprecated" do it "adds a deprecation notice to the notices array" do expect do @@ -100,4 +100,44 @@ def supported_versions end end end + + describe "#record_warning_notices" do + context "when deprecation notice is generated" do + let(:deprecation_notice) do + Dependabot::Notice.new( + mode: "WARN", + type: "bundler_deprecated_warn", + package_manager_name: "bundler", + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "Please upgrade to one of the following versions: `v2`, or `v3`.\n", + markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ + "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n", + show_in_pr: true, + show_in_log: true + ) + end + + it "logs each line of the deprecation notice separately and records it as a warning" do + deprecation_notice.description.each_line do |line| + line = line.strip + expect(Dependabot.logger).to receive(:warn).with(line).once unless line.empty? + end + + dummy_instance.record_warning_notices([deprecation_notice]) + end + end + + context "when no deprecation notice is generated" do + before do + allow(dummy_instance).to receive(:create_deprecation_notice).and_return(nil) + end + + it "does not log or record any warnings" do + expect(Dependabot.logger).not_to receive(:warn) + + dummy_instance.record_warning_notices([]) + end + end + end end