Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,25 @@ When the TODO's event is met (i.e. a certain date is reached), the TODO's assign
end
```

You can also add context to your TODOs by linking them to GitHub issues. The `context` attribute
works with `date`, `gem_release`, `gem_bump`, and `ruby_version` events:

```ruby
# TODO(on: date('2025-01-01'), to: '[email protected]', context: issue('shopify', 'smart_todo', '108'))
# Implement the caching strategy discussed in the issue
def process_order
end

# TODO(on: gem_release('rails', '> 7.2'), to: '[email protected]', context: issue('rails', 'rails', '456'))
# Upgrade to new Rails version as discussed in the issue
def legacy_method
end
```

When the TODO is triggered, the linked issue's title, state, and assignee will be included in the notification.

Note: The `context` attribute is not supported with `issue_close` or `pull_request_close` events as they already reference specific issues/PRs.

Documentation
----------------
Please check out the GitHub [wiki](https://github.com/Shopify/smart_todo/wiki) for documentation and example on how to setup SmartTodo in your project.
Expand Down
32 changes: 31 additions & 1 deletion lib/smart_todo/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,42 @@ 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)
# Context should not be applied to events that already reference specific issues/PRs
# (like issue_close or pull_request_close) since they already have that context.
todo.context && Todo.event_can_use_context?(event.method_name)
end

def process_dispatches(dispatches)
queue = Queue.new
dispatches.each { |dispatch| queue << dispatch }
Expand Down
26 changes: 26 additions & 0 deletions lib/smart_todo/events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
nil
end

# Check if the installed ruby version meets requirements.
#
# @param requirements [Array<String>] a list of version specifiers
Expand Down
31 changes: 31 additions & 0 deletions lib/smart_todo/todo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,23 @@ module SmartTodo
class Todo
attr_reader :filepath, :comment, :indent
attr_reader :events, :assignees, :errors
attr_accessor :context

# Events that already contain issue/PR context and therefore
# should not have additional context applied
EVENTS_WITH_IMPLICIT_CONTEXT = [:issue_close, :pull_request_close].freeze

class << self
# Check if an event is eligible to have context information applied.
# Events like issue_close and pull_request_close already reference
# specific issues/PRs and shouldn't have additional context.
#
# @param event_name [Symbol] the name of the event method
# @return [Boolean] true if the event can use context, false otherwise
def event_can_use_context?(event_name)
!EVENTS_WITH_IMPLICIT_CONTEXT.include?(event_name.to_sym)
end
end

def initialize(source, filepath = "-e")
@filepath = filepath
Expand All @@ -12,6 +29,7 @@ def initialize(source, filepath = "-e")

@events = []
@assignees = []
@context = nil
@errors = []

parse(source[(indent + 1)..])
Expand Down Expand Up @@ -66,6 +84,19 @@ def visit_keyword_hash_node(node)
end
when :to
metadata.assignees << visit(element.value)
when :context
value = visit(element.value)

if value.is_a?(CallNode) && value.method_name == :issue
if value.arguments.length == 3 && value.arguments.all? { |arg| arg.is_a?(String) }
metadata.context = value
else
metadata.errors << "Incorrect `:context` format: issue() requires exactly 3 string arguments " \
"(org, repo, issue_number)"
end
else
metadata.errors << "Incorrect `:context` format: only issue() function is supported"
end
end
end
end
Expand Down
27 changes: 27 additions & 0 deletions lib/smart_todo_cop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ def on_new_investigation
add_offense(comment, message: "Invalid event assignee. This method only accepts strings. #{HELP}")
elsif (invalid_events = validate_events(metadata.events)).any?
add_offense(comment, message: "#{invalid_events.join(". ")}. #{HELP}")
elsif (context_errors = validate_context(metadata)).any?
add_offense(comment, message: "#{context_errors.join(". ")}. #{HELP}")
end
end
end
Expand Down Expand Up @@ -103,6 +105,31 @@ def validate_pull_request_close_args(args)
validate_fixed_arity_args(args, 3, "pull_request_close", ["organization", "repo", "pr_number"])
end

# @param metadata [SmartTodo::Parser::Visitor] The metadata containing context and events
# @return [Array<String>] Returns array of error messages, empty if valid
def validate_context(metadata)
return [] unless metadata.context

context = metadata.context
events = metadata.events

restricted_events = events.reject { |e| ::SmartTodo::Todo.event_can_use_context?(e.method_name) }
if restricted_events.any?
event_name = restricted_events.first.method_name
return ["Invalid context: context attribute cannot be used with #{event_name} event"]
end

if context.method_name != :issue
["Invalid context: only issue() function is supported"]
elsif (error = validate_fixed_arity_args(
context.arguments, 3, "context issue", ["organization", "repo", "issue_number"]
))
[error]
else
[]
end
end

# @param args [Array]
# @return [String, nil] Returns error message if arguments are invalid, nil if valid
def validate_gem_release_args(args)
Expand Down
47 changes: 47 additions & 0 deletions test/smart_todo/cli_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,5 +150,52 @@ 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: '[email protected]')")
event = Todo::CallNode.new(:date, ["2015-03-01"], nil)

refute(cli.send(:should_apply_context?, todo, event))
end

def test_should_apply_context_returns_false_for_issue_close_event
cli = CLI.new
todo = Todo.new("# TODO(on: issue_close('org', 'repo', '123'), to: '[email protected]')")
todo.context = Todo::CallNode.new(:issue, ["org", "repo", "456"], nil)
event = Todo::CallNode.new(:issue_close, ["org", "repo", "123"], nil)

refute(cli.send(:should_apply_context?, todo, event))
end

def test_should_apply_context_returns_false_for_pull_request_close_event
cli = CLI.new
todo = Todo.new("# TODO(on: pull_request_close('org', 'repo', '123'), to: '[email protected]')")
todo.context = Todo::CallNode.new(:issue, ["org", "repo", "456"], nil)
event = Todo::CallNode.new(:pull_request_close, ["org", "repo", "123"], nil)

refute(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: '[email protected]')")
todo.context = Todo::CallNode.new(:issue, ["org", "repo", "456"], nil)
event = Todo::CallNode.new(:date, ["2015-03-01"], nil)

assert(cli.send(:should_apply_context?, todo, event))
end

def test_append_context_if_applicable_returns_original_message_when_context_not_applicable
cli = CLI.new
todo = Todo.new("# TODO(on: issue_close('org', 'repo', '123'), to: '[email protected]')")
todo.context = Todo::CallNode.new(:issue, ["org", "repo", "456"], nil)
event = Todo::CallNode.new(:issue_close, ["org", "repo", "123"], nil)
events = Events.new
original_message = "Original event message"

result = cli.send(:append_context_if_applicable, original_message, todo, event, events)
assert_equal(original_message, result)
end
end
end
147 changes: 147 additions & 0 deletions test/smart_todo/events/issue_context_test.rb
Original file line number Diff line number Diff line change
@@ -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(body: JSON.dump(
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(body: JSON.dump(
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(body: JSON.dump(
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(body: JSON.dump(
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(body: JSON.dump(
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(body: JSON.dump(
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(body: JSON.dump(
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
Loading
Loading