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

Noassignees #72

Merged
merged 2 commits into from
Jun 15, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions web/controllers/event_controller.ex
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
defmodule Dwylbot.EventController do
use Dwylbot.Web, :controller
alias Dwylbot.WaitProcess, as: DW
alias Dwylbot.WaitProcess, as: WAIT

def new(conn, params) do
spawn(DW, :delay, [params])
spawn(WAIT, :delay, [params])
conn
|> put_status(200)
|> json(%{ok: "event received"})
Expand Down
3 changes: 1 addition & 2 deletions web/controllers/github_api/http_client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ defmodule Dwylbot.GithubAPI.HTTPClient do
wrapper functions for the API Github App API
"""
alias Poison.Parser, as: PP
alias Dwylbot.Rules, as: R
alias JOSE.JWK, as: JJ
import Joken

Expand Down Expand Up @@ -64,7 +63,7 @@ defmodule Dwylbot.GithubAPI.HTTPClient do
def report_error(token, errors, comments_url) do
if !Enum.empty?(errors) do
message = errors
|> Enum.map(&(R.generate_message_error &1))
|> Enum.map(fn(e) -> e.error_message end)
|> Enum.join("/n")

comment = Poison.encode!(%{body: message})
Expand Down
27 changes: 13 additions & 14 deletions web/controllers/processes/wait.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,22 @@ defmodule Dwylbot.WaitProcess do
@moduledoc """
The delay function is used to create new checking rules processes
"""
alias Dwylbot.Rules, as: DR
alias Dwylbot.Rules
@github_api Application.get_env(:dwylbot, :github_api)
@duration Application.get_env(:dwylbot, :duration)

def delay(params) do
if params["sender"]["type"] != "Bot" do
errors = DR.check(params["issue"], params["sender"]["login"])
if !Enum.empty?(errors) do
Process.sleep(@duration)
token = @github_api.get_installation_token(params["installation"]["id"])
issue_url = params["issue"]["url"]
comments_url = params["issue"]["comments_url"]
issue = @github_api.get_issue(token, issue_url)
check_errors = DR.check(issue, "")
errors_to_report = DR.compare(check_errors, errors)
@github_api.report_error(token, errors_to_report, comments_url)
end
def delay(payload) do
errors = Rules.apply_and_check_errors(payload)

unless Enum.empty?(errors) do
Process.sleep(@duration)
token = @github_api.get_installation_token(payload["installation"]["id"])
issue_url = payload["issue"]["url"]
comments_url = payload["issue"]["comments_url"]
issue = @github_api.get_issue(token, issue_url)
check_errors = Rules.check_errors(%{"issue" => issue})
errors_to_report = Rules.compare(check_errors, errors)
@github_api.report_error(token, errors_to_report, comments_url)
end
end
end
29 changes: 29 additions & 0 deletions web/controllers/rules/issue/inprogress.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
defmodule Dwylbot.Rules.Inprogress do
@moduledoc """
Check errors for "in-progress and no assignees" errors
"""
def apply?(payload) do
payload["action"] == "labeled" && payload["label"]["name"] == "in-progress"
end

def check(payload) do
assignees = payload["issue"]["assignees"]
labels = payload["issue"]["labels"]
in_progress = Enum.any?(labels, fn(l) -> l["name"] == "in-progress" end)
if in_progress && Enum.empty?(assignees) do
%{
error_type: "inprogress_noassignees",
error_message: payload["sender"] && error_message(payload["sender"]["login"])
}
else
nil
end
end

defp error_message(login) do
"""
@#{login} the `in-progress` label has been added to this issue **without an Assignee**.
Please assign a user to this issue before applying the `in-progress label`.
"""
end
end
30 changes: 30 additions & 0 deletions web/controllers/rules/issue/noassignees.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
defmodule Dwylbot.Rules.Noassignees do
@moduledoc """
Check errors when an assignee is removed but the in-progress label
is still on the issue (list of assignees should be empty too)
"""
def apply?(payload) do
payload["action"] == "unassigned"
end

def check(payload) do
assignees = payload["issue"]["assignees"]
labels = payload["issue"]["labels"]
in_progress = Enum.any?(labels, fn(l) -> l["name"] == "in-progress" end)
if in_progress && Enum.empty?(assignees) do
%{
error_type: "inprogress_noassignees",
error_message: payload["sender"] && error_message(payload["sender"]["login"])
}
else
nil
end
end

defp error_message(login) do
"""
@#{login} the issue doesn't have anymore assignees but is still `in-progress`.
Please assign a user to this issue or remove the `in-progress` label.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message error might not be the best one, any suggestions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonLab we should put all error messages in a central place so they can easily be edited. (doesn't have to be done now, but in future...)

For now we should update it to:

"""
:warning: @#{login} issue has **no assignee** but is still `in-progress`. Who is working on it?
 Please assign a user to this issue or remove the `in-progress` label. Thanks! :heart:
"""

"""
end
end
12 changes: 12 additions & 0 deletions web/controllers/rules/list_rules.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
defmodule Dwylbot.Rules.List do
@moduledoc """
List of modules errors to check on Github event
Each module must implement apply and check functions
"""
def get_rules do
[
Dwylbot.Rules.Inprogress,
Dwylbot.Rules.Noassignees
]
end
end
37 changes: 17 additions & 20 deletions web/controllers/rules/rules.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,29 @@ defmodule Dwylbot.Rules do
@moduledoc """
Functions to check the wrokflow contribution rules
"""
def check(payload, author) do
assignees = payload["assignees"]
labels = payload["labels"]
in_progress = Enum.any?(labels, fn(l) -> l["name"] == "in-progress" end)
if in_progress && Enum.empty?(assignees) do
[%{error_type: "inprogress_noassignees", author: author}]
else
[]
end
alias Dwylbot.Rules.List, as: RulesList

def apply_and_check_errors(payload) do
rules = RulesList.get_rules()
rules
|> Enum.filter(fn(m) -> apply(m, :apply?, [payload]) end)
|> Enum.map(fn(m) -> apply(m, :check, [payload]) end)
|> Enum.filter(fn(e) -> e != nil end)
end

def check_errors(payload) do
rules = RulesList.get_rules()
rules
|> Enum.map(fn(m) -> apply(m, :check, [payload]) end)
|> Enum.filter(fn(e) -> e != nil end)
end

def compare(errors, check) do
Enum.filter check, fn(e) -> has_error?(errors, e) end
def compare(list_errors, event_errors) do
Enum.filter event_errors, fn(e) -> has_error?(list_errors, e) end
end

defp has_error?(list_errors, err) do
Enum.any?(list_errors, fn(e) -> e.error_type == err.error_type end)
end

def generate_message_error(error) do
case error.error_type do
"inprogress_noassignees" ->
"""
@#{error.author} the `in-progress` label has been added to this issue **without an Assignee**.
Please assign a user to this issue before applying the `in-progress label`.
"""
end
end
end