GitHub pagination

.env.example

@@ -6,7 +6,12 @@ export CLOUDEX_SECRET=
 export CLOUDFRONT_DOMAIN=
 export GITHUB_APP_CLIENT_ID=
 export GITHUB_APP_CLIENT_SECRET=
+export GITHUB_APP_ID=
 export GITHUB_APP_PEM=
+export GITHUB_TEST_APP_CLIENT_ID=
+export GITHUB_TEST_APP_CLIENT_SECRET=
+export GITHUB_TEST_APP_ID=
+export GITHUB_TEST_APP_PEM=
 export INTERCOM_IDENTITY_SECRET_KEY=
 export POSTMARK_API_KEY=
 export S3_BUCKET=

circle.yml

@@ -17,7 +17,7 @@ test:
       if [ ${CIRCLE_PR_USERNAME} ]; then
         MIX_ENV=test mix test;
       else
-        MIX_ENV=test mix coveralls.circle;
+        MIX_ENV=test mix coveralls.circle --include acceptance:true;
       fi
   post:
     - mix inch.report

config/dev.exs

@@ -7,7 +7,7 @@ use Mix.Config
 # watchers to your application. For example, we use it
 # with brunch.io to recompile .js and .css sources.
 config :code_corps, CodeCorpsWeb.Endpoint,
-  http: [port: 4000, ip: {0, 0, 0, 0, 0, 0, 0, 0}],
+  http: [port: 4000],
   debug_errors: true,
   code_reloader: true,
   check_origin: false

config/test.exs

@@ -41,8 +41,18 @@ config :code_corps, :icon_color_generator, CodeCorps.RandomIconColor.TestGenerat
 # Set Corsica logging to output no console warning when rejecting a request
 config :code_corps, :corsica_log_level, [rejected: :debug]
 
-# Set the GitHub module
-config :code_corps, github: CodeCorps.GitHubTesting
+# fall back to sample pem if none is available as an ENV variable
+pem = case System.get_env("GITHUB_TEST_APP_PEM") do
+  nil -> "./test/fixtures/github/app.pem" |> File.read!
+  encoded_pem -> encoded_pem |> Base.decode64!
+end
+
+config :code_corps,
+  github: CodeCorps.GitHub.SuccessAPI,
+  github_app_id: System.get_env("GITHUB_TEST_APP_ID"),
+  github_app_client_id: System.get_env("GITHUB_TEST_APP_CLIENT_ID"),
+  github_app_client_secret: System.get_env("GITHUB_TEST_APP_CLIENT_SECRET"),
+  github_app_pem: pem
 
 config :sentry,
   environment_name: Mix.env || :test
@@ -57,13 +67,3 @@ config :code_corps,
 
 config :code_corps, :cloudex, CloudexTest
 config :cloudex, api_key: "test_key", secret: "test_secret", cloud_name: "test_cloud_name"
-
-# fall back to sample pem if none is available as an ENV variable
-pem = case System.get_env("GITHUB_APP_PEM") do
-  nil -> "./test/fixtures/github/app.pem" |> File.read!
-  encoded_pem -> encoded_pem |> Base.decode64!
-end
-
-config :code_corps,
-  github: CodeCorps.GitHub.SuccessAPI,
-  github_app_pem: pem

lib/code_corps/github/api/api.ex

@@ -14,6 +14,8 @@ defmodule CodeCorps.GitHub.API do
     |> marshall_response()
   end
 
+  defdelegate get_all(url, headers, opts), to: CodeCorps.GitHub.EagerAPI
+
   @doc """
   Get access token headers for a given `CodeCorps.User` and
   `CodeCorps.GithubAppInstallation`.

lib/code_corps/github/api/eager_api.ex

@@ -0,0 +1,83 @@
+defmodule CodeCorps.GitHub.EagerAPI do
+  @moduledoc """
+  Eager loads a resource from the GitHub API by fetching all of its pages in
+  parallel.
+  """
+
+  def get_all(url, headers, options) do
+    HTTPoison.start
+    {:ok, response} = HTTPoison.get(url, headers, options)
+
+    first_page = Poison.decode!(response.body)
+    case response.headers |> retrieve_total_pages do
+      1 -> first_page
+      total ->
+        first_page ++ get_remaining_pages(total, url, headers, options) |> List.flatten
+    end
+  end
+
+  defp get_remaining_pages(total, url, headers, options) do
+    2..total
+    |> Enum.to_list
+    |> Enum.map(&Task.async(fn ->
+      params = options[:params] ++ [page: &1]
+      HTTPoison.get(url, headers, options ++ [params: params])
+    end))
+    |> Enum.map(&Task.await(&1, 10000))
+    |> Enum.map(&handle_page_response/1)
+  end
+
+  defp handle_page_response({:ok, %{body: body}}), do: Poison.decode!(body)
+
+  def retrieve_total_pages(headers) do
+    case headers |> List.keyfind("Link", 0, nil) do
+      nil -> 1
+      {"Link", value} -> value |> extract_total_pages
+    end
+  end
+
+  defp extract_total_pages(links_string) do
+    # We use regex to parse the pagination info from the GitHub API response
+    # headers.
+    #
+    # The headers render pages in the following format:
+    #
+    # ```
+    # {"Link", '<https://api.github.com/search/code?q=addClass+user%3Amozilla&page=15>; rel="next",
+    #           <https://api.github.com/search/code?q=addClass+user%3Amozilla&page=34>; rel="last",
+    #           <https://api.github.com/search/code?q=addClass+user%3Amozilla&page=1>; rel="first",
+    #           <https://api.github.com/search/code?q=addClass+user%3Amozilla&page=13>; rel="prev"'
+    # ```
+    #
+    # If the response has no list header, then we have received all the records
+    # from the only possible page.
+    #
+    # If the response has a list header, the value will contain at least the
+    # "last" relation.
+    links_string
+    |> String.split(", ")
+    |> Enum.map(fn link ->
+      rel = get_rel(link)
+      page = get_page(link)
+      {rel, page}
+    end)
+    |> Enum.into(%{})
+    |> Map.get("last")
+  end
+
+  defp get_rel(link) do
+    # Searches for `rel=`
+    Regex.run(~r{rel="([a-z]+)"}, link) |> List.last()
+  end
+
+  defp get_page(link) do
+    # Searches for the following variations:
+    # ```
+    # ?page={match}>
+    # ?page={match}&...
+    # &page={match}>
+    # &page={match}&...
+    # ```
+    Regex.run(~r{[&/?]page=([^>&]+)}, link) |> List.last |> String.to_integer
+  end
+end

lib/code_corps/github/api/headers.ex

@@ -33,11 +33,12 @@ defmodule CodeCorps.GitHub.API.Headers do
   end
 
   @spec add_access_token_header(%{String.t => String.t}, list) :: %{String.t => String.t}
-  defp add_access_token_header(%{} = headers, [access_token: nil]), do: headers
-  defp add_access_token_header(%{} = headers, [access_token: access_token]) do
-    Map.put(headers, "Authorization", "token #{access_token}")
+  defp add_access_token_header(%{} = headers, opts) do
+    case opts[:access_token] do
+      nil -> headers
+      token -> headers |> Map.put("Authorization", "token #{token}")
+    end
   end
-  defp add_access_token_header(headers, []), do: headers
 
   @spec add_jwt_header(%{String.t => String.t}) :: %{String.t => String.t}
   defp add_jwt_header(%{} = headers) do

lib/code_corps/github/api/repository.ex

@@ -0,0 +1,31 @@
+defmodule CodeCorps.GitHub.API.Repository do
+  @moduledoc ~S"""
+  Functions for working with issues on GitHub.
+  """
+
+  alias CodeCorps.{
+    GitHub,
+    GitHub.API,
+    GithubAppInstallation,
+    GithubRepo,
+  }
+
+  @spec issues(GithubRepo.t) :: {:ok, list(map)} | {:error, GitHub.api_error_struct}
+  def issues(%GithubRepo{github_app_installation: %GithubAppInstallation{} = installation} = github_repo) do
+    with {:ok, access_token} <- API.Installation.get_access_token(installation),
+         issues <- fetch_issues(github_repo, access_token)
+    do
+      {:ok, issues}
+    else
+      {:error, error} -> {:error, error}
+    end
+  end
+
+  defp fetch_issues(%GithubRepo{github_app_installation: %GithubAppInstallation{github_account_login: owner}, name: repo}, access_token) do
+    per_page = 100
+    path = "repos/#{owner}/#{repo}/issues"
+    params = [per_page: per_page, state: "all"]
+    opts = [access_token: access_token, params: params]
+    GitHub.get_all(path, %{}, opts)
+  end
+end

lib/code_corps/github/github.ex

@@ -124,6 +124,14 @@ defmodule CodeCorps.GitHub do
     end
   end
 
+  def get_all(endpoint, headers, options) do
+    api().get_all(
+      api_url_for(endpoint),
+      headers |> Headers.user_request(options),
+      options |> add_default_options()
+    )
+  end
+
   @token_url "https://github.com/login/oauth/access_token"
 
   @doc """

lib/code_corps/github/sync/sync.ex

@@ -6,14 +6,13 @@ defmodule CodeCorps.GitHub.Sync do
     GithubPullRequest,
     GithubRepo,
     GitHub.Sync.Utils.RepoFinder,
-    Repo,
-    Task
+    Repo
   }
   alias Ecto.Multi
 
   @type outcome :: {:ok, list(Comment.t)}
                  | {:ok, GithubPullRequest.t}
-                 | {:ok, list(Task.t)}
+                 | {:ok, list(CodeCorps.Task.t)}
                  | {:error, :repo_not_found}
                  | {:error, :fetching_issue}
                  | {:error, :fetching_pull_request}
@@ -100,6 +99,24 @@ defmodule CodeCorps.GitHub.Sync do
     |> transact()
   end
 
+  def sync_issues(repo) do
+    {:ok, issues} = GitHub.API.Repository.issues(repo)
+    Enum.map(issues, &sync_issue(&1, repo))
+  end
+
+  def sync_issue(issue, repo) do
+    Multi.new
+    |> Multi.merge(__MODULE__, :return_repo, [repo])
+    |> Multi.merge(GitHub.Sync.Issue, :sync, [issue])
+    |> transact()
+  end
+
+  @doc false
+  def return_repo(_, repo) do
+    Multi.new
+    |> Multi.run(:repo, fn _ -> {:ok, repo} end)
+  end
+
   @doc false
   def find_repo(_, payload) do
     Multi.new

lib/code_corps/validators/time_validator.ex

@@ -11,7 +11,10 @@ defmodule CodeCorps.Validators.TimeValidator do
   def validate_time_after(%{data: data} = changeset, field) do
     previous_time = Map.get(data, field)
     current_time = Changeset.get_change(changeset, field)
-    case current_time |> Timex.after?(previous_time) do
+    is_after = current_time |> Timex.after?(previous_time)
+    is_equal = current_time |> Timex.equal?(previous_time)
+    after_or_equal = is_after || is_equal
+    case after_or_equal do
       true -> changeset
       false -> Changeset.add_error(changeset, field, "cannot be before the last recorded time")
     end

mix.exs

@@ -54,7 +54,7 @@ defmodule CodeCorps.Mixfile do
       {:cowboy, "~> 1.0"},
       {:benchfella, "~> 0.3.0", only: :dev},
       {:bypass, "~> 0.8.1", only: :test},
-      {:cloudex, "~> 0.1.17"},
+      {:cloudex, "~> 0.2.2"},
       {:comeonin, "~> 3.1"},
       {:corsica, "~> 1.0"}, # CORS
       {:credo, "~> 0.8", only: [:dev, :test]}, # Code style suggestions
@@ -66,6 +66,7 @@ defmodule CodeCorps.Mixfile do
       {:ex_machina, "~> 2.0", only: :test}, # test factories
       {:guardian, "~> 0.14.5"}, # Authentication (JWT)
       {:hackney, ">= 1.4.4"},
+      {:httpoison, "~> 0.13"},
       {:inch_ex, "~> 0.5", only: [:dev, :test]}, # Inch CI
       {:inflex, "~> 1.8.1"},
       {:ja_serializer, "~> 0.12"}, # JSON API
@@ -406,6 +407,7 @@ defmodule CodeCorps.Mixfile do
      "ecto.reset": ["ecto.drop", "ecto.setup"],
      "ecto.migrate": ["ecto.migrate", "ecto.dump"],
      "ecto.rollback": ["ecto.rollback", "ecto.dump"],
-     "test": ["ecto.create --quiet", "ecto.migrate", "test"]]
+     "test": ["ecto.create --quiet", "ecto.migrate", "test"],
+     "test.acceptance": ["ecto.create --quiet", "ecto.migrate", "test --include acceptance:true"]]
   end
 end

mix.lock

@@ -5,8 +5,8 @@
   "benchfella": {:hex, :benchfella, "0.3.5", "b2122c234117b3f91ed7b43b6e915e19e1ab216971154acd0a80ce0e9b8c05f5", [:mix], []},
   "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], []},
   "bypass": {:hex, :bypass, "0.8.1", "16d409e05530ece4a72fabcf021a3e5c7e15dcc77f911423196a0c551f2a15ca", [:mix], [{:cowboy, "~> 1.0", [hex: :cowboy, optional: false]}, {:plug, "~> 1.0", [hex: :plug, optional: false]}]},
-  "certifi": {:hex, :certifi, "0.7.0", "861a57f3808f7eb0c2d1802afeaae0fa5de813b0df0979153cbafcd853ababaf", [:rebar3], []},
-  "cloudex": {:hex, :cloudex, "0.1.20", "c8b0f36da0d33c4756cfbfdd7bf2bcd4722d58a51cb48da75a6c54d6186bad30", [:mix], [{:httpoison, "~> 0.11.0", [hex: :httpoison, optional: false]}, {:poison, "~> 3.1.0", [hex: :poison, optional: false]}, {:timex, "~> 3.1.7", [hex: :timex, optional: false]}, {:tzdata, "~> 0.5.11", [hex: :tzdata, optional: false]}]},
+  "certifi": {:hex, :certifi, "2.0.0", "a0c0e475107135f76b8c1d5bc7efb33cd3815cb3cf3dea7aefdd174dabead064", [:rebar3], [], "hexpm"},
+  "cloudex": {:hex, :cloudex, "0.2.2", "da554dab88672163346a1614917df2a27ae82d07e956ee31de758f0b04c163bf", [:mix], [{:httpoison, "~> 0.13.0", [repo: "hexpm", hex: :httpoison, optional: false]}, {:poison, "~> 3.1.0", [repo: "hexpm", hex: :poison, optional: false]}, {:timex, "~> 3.1.7", [repo: "hexpm", hex: :timex, optional: false]}, {:tzdata, "~> 0.5.11", [repo: "hexpm", hex: :tzdata, optional: false]}], "hexpm"},
   "combine": {:hex, :combine, "0.10.0", "eff8224eeb56498a2af13011d142c5e7997a80c8f5b97c499f84c841032e429f", [:mix], []},
   "comeonin": {:hex, :comeonin, "3.2.0", "cb10995a22aed6812667efb3856f548818c85d85394d8132bc116fbd6995c1ef", [:make, :mix], [{:elixir_make, "~> 0.4", [hex: :elixir_make, optional: false]}]},
   "connection": {:hex, :connection, "1.0.4", "a1cae72211f0eef17705aaededacac3eb30e6625b04a6117c1b2db6ace7d5976", [], []},
@@ -29,9 +29,9 @@
   "fs": {:hex, :fs, "0.9.2", "ed17036c26c3f70ac49781ed9220a50c36775c6ca2cf8182d123b6566e49ec59", [:rebar], []},
   "gettext": {:hex, :gettext, "0.13.1", "5e0daf4e7636d771c4c71ad5f3f53ba09a9ae5c250e1ab9c42ba9edccc476263", [:mix], []},
   "guardian": {:hex, :guardian, "0.14.5", "6d4e89b673accdacbc092ad000dc7494019426bd898eebf699caf1d19000cdcd", [:mix], [{:jose, "~> 1.8", [hex: :jose, optional: false]}, {:phoenix, "~> 1.2 and < 1.4.0", [hex: :phoenix, optional: true]}, {:plug, "~> 1.3", [hex: :plug, optional: false]}, {:poison, ">= 1.3.0 and < 4.0.0", [hex: :poison, optional: false]}, {:uuid, ">=1.1.1", [hex: :uuid, optional: false]}]},
-  "hackney": {:hex, :hackney, "1.6.5", "8c025ee397ac94a184b0743c73b33b96465e85f90a02e210e86df6cbafaa5065", [:rebar3], [{:certifi, "0.7.0", [hex: :certifi, optional: false]}, {:idna, "1.2.0", [hex: :idna, optional: false]}, {:metrics, "1.0.1", [hex: :metrics, optional: false]}, {:mimerl, "1.0.2", [hex: :mimerl, optional: false]}, {:ssl_verify_fun, "1.1.1", [hex: :ssl_verify_fun, optional: false]}]},
-  "httpoison": {:hex, :httpoison, "0.11.0", "b9240a9c44fc46fcd8618d17898859ba09a3c1b47210b74316c0ffef10735e76", [:mix], [{:hackney, "~> 1.6.3", [hex: :hackney, optional: false]}]},
-  "idna": {:hex, :idna, "1.2.0", "ac62ee99da068f43c50dc69acf700e03a62a348360126260e87f2b54eced86b2", [:rebar3], []},
+  "hackney": {:hex, :hackney, "1.10.1", "c38d0ca52ea80254936a32c45bb7eb414e7a96a521b4ce76d00a69753b157f21", [:rebar3], [{:certifi, "2.0.0", [repo: "hexpm", hex: :certifi, optional: false]}, {:idna, "5.1.0", [repo: "hexpm", hex: :idna, optional: false]}, {:metrics, "1.0.1", [repo: "hexpm", hex: :metrics, optional: false]}, {:mimerl, "1.0.2", [repo: "hexpm", hex: :mimerl, optional: false]}, {:ssl_verify_fun, "1.1.1", [repo: "hexpm", hex: :ssl_verify_fun, optional: false]}], "hexpm"},
+  "httpoison": {:hex, :httpoison, "0.13.0", "bfaf44d9f133a6599886720f3937a7699466d23bb0cd7a88b6ba011f53c6f562", [:mix], [{:hackney, "~> 1.8", [repo: "hexpm", hex: :hackney, optional: false]}], "hexpm"},
+  "idna": {:hex, :idna, "5.1.0", "d72b4effeb324ad5da3cab1767cb16b17939004e789d8c0ad5b70f3cea20c89a", [:rebar3], [{:unicode_util_compat, "0.3.1", [repo: "hexpm", hex: :unicode_util_compat, optional: false]}], "hexpm"},
   "inch_ex": {:hex, :inch_ex, "0.5.6", "418357418a553baa6d04eccd1b44171936817db61f4c0840112b420b8e378e67", [:mix], [{:poison, "~> 1.5 or ~> 2.0 or ~> 3.0", [hex: :poison, optional: false]}]},
   "inflex": {:hex, :inflex, "1.8.1", "9fa9684ff1a872eab7415c0be500cc1b7782f28da6ed75423081e75f92831b1c", [:mix], []},
   "ja_serializer": {:hex, :ja_serializer, "0.12.0", "ba4ec5fc7afa6daba815b5cb2b9bd0de410554ac4f0ed54e954d39decb353ca4", [:mix], [{:inflex, "~> 1.4", [hex: :inflex, optional: false]}, {:plug, "> 1.0.0", [hex: :plug, optional: false]}, {:poison, ">= 1.4.0", [hex: :poison, optional: false]}, {:scrivener, "~> 1.2 or ~> 2.0", [hex: :scrivener, optional: true]}]},
@@ -65,5 +65,6 @@
   "timex": {:hex, :timex, "3.1.24", "d198ae9783ac807721cca0c5535384ebdf99da4976be8cefb9665a9262a1e9e3", [:mix], [{:combine, "~> 0.7", [hex: :combine, optional: false]}, {:gettext, "~> 0.10", [hex: :gettext, optional: false]}, {:tzdata, "~> 0.1.8 or ~> 0.5", [hex: :tzdata, optional: false]}]},
   "timex_ecto": {:hex, :timex_ecto, "3.0.5", "3ec6c25e10d2c0020958e5df64d2b5e690e441faa2c2259da8bc6bd3d7f39256", [:mix], [{:ecto, "~> 2.0", [hex: :ecto, optional: false]}, {:timex, "~> 3.0", [hex: :timex, optional: false]}]},
   "tzdata": {:hex, :tzdata, "0.5.12", "1c17b68692c6ba5b6ab15db3d64cc8baa0f182043d5ae9d4b6d35d70af76f67b", [:mix], [{:hackney, "~> 1.0", [hex: :hackney, optional: false]}]},
+  "unicode_util_compat": {:hex, :unicode_util_compat, "0.3.1", "a1f612a7b512638634a603c8f401892afbf99b8ce93a45041f8aaca99cadb85e", [:rebar3], []},
   "uri_query": {:hex, :uri_query, "0.1.2", "ae35b83b472f3568c2c159eee3f3ccf585375d8a94fb5382db1ea3589e75c3b4", [:mix], []},
   "uuid": {:hex, :uuid, "1.1.7", "007afd58273bc0bc7f849c3bdc763e2f8124e83b957e515368c498b641f7ab69", [:mix], []}}