diff --git a/lib/smart_todo/cli.rb b/lib/smart_todo/cli.rb index 0d73923..b04bffb 100644 --- a/lib/smart_todo/cli.rb +++ b/lib/smart_todo/cli.rb @@ -106,12 +106,40 @@ def process_todos(todos) end @errors.concat(todo.errors) - dispatches << [event_message, todo] if event_met + + next unless event_met + + event_message = append_context_if_applicable(event_message, todo, event_met, events) + + dispatches << [event_message, todo] end dispatches end + private + + # @param event_message [String] the original event message + # @param todo [Todo] the todo object that may contain context + # @param event [Event] the event that was met + # @param events [Events] the events instance for fetching issue context + # @return [String] the event message, potentially with context appended + def append_context_if_applicable(event_message, todo, event, events) + return event_message unless should_apply_context?(todo, event) + + org, repo, issue_number = todo.context.arguments + context_message = events.issue_context(org, repo, issue_number) + + context_message ? "#{event_message}\n\n#{context_message}" : event_message + end + + # @param todo [Todo] the todo object to check for context + # @param event [Event] the event to check + # @return [Boolean] true if context should be applied, false otherwise + def should_apply_context?(todo, event) + !!todo.context + end + def process_dispatches(dispatches) queue = Queue.new dispatches.each { |dispatch| queue << dispatch } diff --git a/lib/smart_todo/events.rb b/lib/smart_todo/events.rb index fb21fb5..1ee1d4e 100644 --- a/lib/smart_todo/events.rb +++ b/lib/smart_todo/events.rb @@ -157,6 +157,32 @@ def pull_request_close(organization, repo, pr_number) end end + # Retrieve context information for an issue + # This is used when a TODO has a context: issue() attribute + # + # @param organization [String] the GitHub organization name + # @param repo [String] the GitHub repo name + # @param issue_number [String, Integer] + # @return [String, nil] + def issue_context(organization, repo, issue_number) + headers = github_headers(organization, repo) + response = github_client.get("/repos/#{organization}/#{repo}/issues/#{issue_number}", headers) + + if response.code_type < Net::HTTPClientError + nil + else + issue = JSON.parse(response.body) + state = issue["state"] + title = issue["title"] + assignee = issue["assignee"] ? "@#{issue["assignee"]["login"]}" : "unassigned" + + "📌 Context: Issue ##{issue_number} - \"#{title}\" [#{state}] (#{assignee}) - " \ + "https://github.com/#{organization}/#{repo}/issues/#{issue_number}" + end + rescue Net::HTTPError, JSON::ParserError + nil + end + # Check if the installed ruby version meets requirements. # # @param requirements [Array] a list of version specifiers diff --git a/lib/smart_todo/todo.rb b/lib/smart_todo/todo.rb index 51bdcb2..1bc0c26 100644 --- a/lib/smart_todo/todo.rb +++ b/lib/smart_todo/todo.rb @@ -4,6 +4,7 @@ module SmartTodo class Todo attr_reader :filepath, :comment, :indent attr_reader :events, :assignees, :errors + attr_accessor :context def initialize(source, filepath = "-e") @filepath = filepath @@ -12,6 +13,7 @@ def initialize(source, filepath = "-e") @events = [] @assignees = [] + @context = nil @errors = [] parse(source[(indent + 1)..]) @@ -66,6 +68,28 @@ def visit_keyword_hash_node(node) end when :to metadata.assignees << visit(element.value) + when :context + value = visit(element.value) + + unless value.is_a?(String) + metadata.errors << "Incorrect `:context` format: expected string value" + next + end + + unless value =~ %r{^([^/]+)/([^#]+)#(\d+)$} + metadata.errors << "Incorrect `:context` format: expected \"org/repo#issue_number\"" + next + end + + org = ::Regexp.last_match(1) + repo = ::Regexp.last_match(2) + issue_number = ::Regexp.last_match(3) + + if org.empty? || repo.empty? + metadata.errors << "Incorrect `:context` format: org and repo cannot be empty" + else + metadata.context = CallNode.new(:issue, [org, repo, issue_number], element.value.location) + end end end end diff --git a/test/smart_todo/cli_test.rb b/test/smart_todo/cli_test.rb index 92eddee..c7a6216 100644 --- a/test/smart_todo/cli_test.rb +++ b/test/smart_todo/cli_test.rb @@ -150,5 +150,41 @@ def hello end end end + + def test_should_apply_context_returns_false_without_context + cli = CLI.new + todo = Todo.new("# TODO(on: date('2015-03-01'), to: 'john@example.com')") + event = Todo::CallNode.new(:date, ["2015-03-01"], nil) + + refute(cli.send(:should_apply_context?, todo, event)) + end + + def test_should_apply_context_returns_true_for_issue_close_event_with_context + cli = CLI.new + todo = Todo.new( + "# TODO(on: issue_close('org', 'repo', '123'), to: 'john@example.com', context: \"org/repo#456\")", + ) + event = Todo::CallNode.new(:issue_close, ["org", "repo", "123"], nil) + + assert(cli.send(:should_apply_context?, todo, event)) + end + + def test_should_apply_context_returns_true_for_pull_request_close_event_with_context + cli = CLI.new + todo = Todo.new( + "# TODO(on: pull_request_close('org', 'repo', '123'), to: 'john@example.com', context: \"org/repo#456\")", + ) + event = Todo::CallNode.new(:pull_request_close, ["org", "repo", "123"], nil) + + assert(cli.send(:should_apply_context?, todo, event)) + end + + def test_should_apply_context_returns_true_for_regular_event_with_context + cli = CLI.new + todo = Todo.new("# TODO(on: date('2015-03-01'), to: 'john@example.com', context: \"org/repo#456\")") + event = Todo::CallNode.new(:date, ["2015-03-01"], nil) + + assert(cli.send(:should_apply_context?, todo, event)) + end end end diff --git a/test/smart_todo/events/issue_context_test.rb b/test/smart_todo/events/issue_context_test.rb new file mode 100644 index 0000000..70f3c29 --- /dev/null +++ b/test/smart_todo/events/issue_context_test.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true + +require "test_helper" + +module SmartTodo + class Events + class IssueContextTest < Minitest::Test + def test_when_issue_exists_and_is_open + stub_request(:get, /api.github.com/) + .to_return_json(body: { + state: "open", + title: "Add support for caching", + number: 123, + assignee: { login: "johndoe" }, + }) + + expected = "📌 Context: Issue #123 - \"Add support for caching\" [open] (@johndoe) - " \ + "https://github.com/rails/rails/issues/123" + + assert_equal(expected, issue_context("rails", "rails", "123")) + end + + def test_when_issue_exists_and_is_closed + stub_request(:get, /api.github.com/) + .to_return_json(body: { + state: "closed", + title: "Fix memory leak", + number: 456, + assignee: { login: "janedoe" }, + }) + + expected = "📌 Context: Issue #456 - \"Fix memory leak\" [closed] (@janedoe) - " \ + "https://github.com/shopify/smart_todo/issues/456" + + assert_equal(expected, issue_context("shopify", "smart_todo", "456")) + end + + def test_when_issue_has_no_assignee + stub_request(:get, /api.github.com/) + .to_return_json(body: { + state: "open", + title: "Improve documentation", + number: 789, + assignee: nil, + }) + + expected = "📌 Context: Issue #789 - \"Improve documentation\" [open] (unassigned) - " \ + "https://github.com/org/repo/issues/789" + + assert_equal(expected, issue_context("org", "repo", "789")) + end + + def test_when_issue_does_not_exist + stub_request(:get, /api.github.com/) + .to_return(status: 404) + + assert_nil(issue_context("rails", "rails", "999")) + end + + def test_when_token_env_is_not_present + stub_request(:get, /api.github.com/) + .to_return_json(body: { + state: "open", + title: "Test issue", + number: 1, + assignee: nil, + }) + + result = issue_context("rails", "rails", "1") + assert(result.include?("📌 Context: Issue #1")) + + assert_requested(:get, /api.github.com/) do |request| + assert(!request.headers.key?("Authorization")) + end + end + + def test_when_token_env_is_present + ENV[GITHUB_TOKEN] = "abc123" + + stub_request(:get, /api.github.com/) + .to_return_json(body: { + state: "open", + title: "Test issue", + number: 2, + assignee: nil, + }) + + result = issue_context("rails", "rails", "2") + assert(result.include?("📌 Context: Issue #2")) + + assert_requested(:get, /api.github.com/) do |request| + assert_equal("token abc123", request.headers["Authorization"]) + end + ensure + ENV.delete(GITHUB_TOKEN) + end + + def test_when_organization_specific_token_is_present + ENV["#{GITHUB_TOKEN}__RAILS"] = "rails_token" + + stub_request(:get, /api.github.com/) + .to_return_json(body: { + state: "open", + title: "Test issue", + number: 3, + assignee: nil, + }) + + result = issue_context("rails", "rails", "3") + assert(result.include?("📌 Context: Issue #3")) + + assert_requested(:get, /api.github.com/) do |request| + assert_equal("token rails_token", request.headers["Authorization"]) + end + ensure + ENV.delete("#{GITHUB_TOKEN}__RAILS") + end + + def test_when_repo_specific_token_is_present + ENV["#{GITHUB_TOKEN}__RAILS__RAILS"] = "rails_rails_token" + + stub_request(:get, /api.github.com/) + .to_return_json(body: { + state: "open", + title: "Test issue", + number: 4, + assignee: nil, + }) + + result = issue_context("rails", "rails", "4") + assert(result.include?("📌 Context: Issue #4")) + + assert_requested(:get, /api.github.com/) do |request| + assert_equal("token rails_rails_token", request.headers["Authorization"]) + end + ensure + ENV.delete("#{GITHUB_TOKEN}__RAILS__RAILS") + end + + private + + def issue_context(*args) + Events.new.issue_context(*args) + end + end + end +end diff --git a/test/smart_todo/integration_test.rb b/test/smart_todo/integration_test.rb index 0667a52..c780f08 100644 --- a/test/smart_todo/integration_test.rb +++ b/test/smart_todo/integration_test.rb @@ -104,6 +104,115 @@ def hello ) end + def test_includes_context_when_date_is_met_with_issue_context + ruby_code = <<~EOM + # TODO(on: date('2015-03-01'), to: 'john@example.com', context: "shopify/smart_todo#123") + # Implement the caching strategy discussed in the issue + def hello + end + EOM + + stub_request(:get, /api.github.com/) + .to_return(body: JSON.dump( + state: "open", + title: "Add caching support", + number: 123, + assignee: { login: "developer" }, + )) + + generate_ruby_file(ruby_code) do |file| + run_cli(file) + end + + assert_slack_message_sent( + "Hello :wave:,", + "We are past the *2015-03-01* due date", + "📌 Context: Issue #123", + "Add caching support", + "Implement the caching strategy discussed in the issue", + ) + end + + def test_includes_context_when_gem_release_is_met_with_issue_context + ruby_code = <<~EOM + # TODO(on: gem_release('rails', '> 5.1'), to: 'john@example.com', context: "rails/rails#456") + # Upgrade to new Rails version as per issue + def hello + end + EOM + + stub_request(:get, /rubygems.org/) + .to_return(body: JSON.dump([{ number: "5.1.1" }])) + + stub_request(:get, /api.github.com/) + .to_return(body: JSON.dump( + state: "closed", + title: "Rails upgrade needed", + number: 456, + assignee: nil, + )) + + generate_ruby_file(ruby_code) do |file| + run_cli(file) + end + + assert_slack_message_sent( + "Hello :wave:,", + "The gem *rails* was released to version *5.1.1*", + "📌 Context: Issue #456", + "Rails upgrade needed", + "Upgrade to new Rails version as per issue", + ) + end + + def test_context_does_not_affect_todos_without_events_triggered + ruby_code = <<~EOM + # TODO(on: date('2099-12-31'), to: 'john@example.com', context: "shopify/smart_todo#789") + # This TODO is not ready yet + def hello + end + EOM + + generate_ruby_file(ruby_code) do |file| + stub_slack_request + CLI.new.run([file.path, "--slack_token", "123", "--fallback_channel", '#general"', "--dispatcher", "slack"]) + end + + assert_not_requested(:post, /chat.postMessage/) + assert_not_requested(:get, /api.github.com/) + end + + def test_context_included_with_issue_close_event + ruby_code = <<~EOM + # TODO(on: issue_close('shopify', 'smart_todo', '100'), to: 'john@example.com', context: "shopify/smart_todo#999") + # This context should be included + def hello + end + EOM + + stub_request(:get, %r{api.github.com.*issues/100}) + .to_return(body: JSON.dump(state: "closed")) + + stub_request(:get, %r{api.github.com.*issues/999}) + .to_return(body: JSON.dump( + title: "Context Issue", + state: "open", + assignee: { login: "developer" }, + )) + + generate_ruby_file(ruby_code) do |file| + run_cli(file) + end + + assert_slack_message_sent( + "Hello :wave:,", + "The issue https://github.com/shopify/smart_todo/issues/100 is now closed", + "This context should be included", + "Context:", + "Context Issue", + ) + end + private def assert_slack_message_sent(*messages) diff --git a/test/smart_todo/smart_todo_cop_test.rb b/test/smart_todo/smart_todo_cop_test.rb index c87b963..3a334e0 100644 --- a/test/smart_todo/smart_todo_cop_test.rb +++ b/test/smart_todo/smart_todo_cop_test.rb @@ -487,6 +487,67 @@ def hello RUBY end + def test_does_not_add_offense_when_todo_has_valid_context_with_issue + expect_no_offense(<<~RUBY) + # TODO(on: date('2019-08-04'), to: 'john@example.com', context: "shopify/smart_todo#123") + def hello + end + RUBY + end + + def test_does_not_add_offense_when_todo_has_context_with_different_events + expect_no_offense(<<~RUBY) + # TODO(on: gem_release('rails', '> 6.0'), to: 'john@example.com', context: "rails/rails#456") + def hello + end + + # TODO(on: gem_bump('rails', '> 7.0'), to: 'john@example.com', context: "rails/rails#789") + def hello + end + + # TODO(on: ruby_version('> 3.0'), to: 'john@example.com', context: "ruby/ruby#123") + def hello + end + RUBY + end + + def test_add_offense_when_todo_has_context_with_wrong_number_of_arguments + expect_offense(<<~RUBY) + # TODO(on: date('2019-08-04'), to: 'john@example.com', context: "shopify/smart_todo") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SmartTodo/SmartTodoCop: Invalid TODO format: Incorrect `:context` format: expected "org/repo#issue_number". For more info please look at https://github.com/Shopify/smart_todo/wiki/Syntax + def hello + end + RUBY + end + + def test_add_offense_when_todo_has_context_with_non_string_arguments + expect_offense(<<~RUBY) + # TODO(on: date('2019-08-04'), to: 'john@example.com', context: 123) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SmartTodo/SmartTodoCop: Invalid TODO format: Incorrect `:context` format: expected string value. For more info please look at https://github.com/Shopify/smart_todo/wiki/Syntax + def hello + end + RUBY + end + + def test_add_offense_when_todo_has_context_with_invalid_function + expect_offense(<<~RUBY) + # TODO(on: date('2019-08-04'), to: 'john@example.com', context: "shopify-smart_todo-123") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SmartTodo/SmartTodoCop: Invalid TODO format: Incorrect `:context` format: expected "org/repo#issue_number". For more info please look at https://github.com/Shopify/smart_todo/wiki/Syntax + def hello + end + RUBY + end + + def test_context_does_not_require_assignee_when_event_does + # Even though context is present, the TODO still requires an assignee for the event + expect_offense(<<~RUBY) + # TODO(on: date('2019-08-04'), context: "shopify/smart_todo#123") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{expected_message} + def hello + end + RUBY + end + private def expected_message