From 6ff205db4b1daa6be63a508deb9d7362762d3383 Mon Sep 17 00:00:00 2001 From: Chao Lin Date: Mon, 23 Dec 2019 12:06:16 -0800 Subject: [PATCH 1/6] [#170146288] Add batch details page --- lib/web/controllers/batch_controller.ex | 23 +++++++++++++++ lib/web/router.ex | 8 +++++ lib/web/templates/batch/show.html.eex | 39 +++++++++++++++++++++++++ lib/web/views/batch_view.ex | 7 +++++ 4 files changed, 77 insertions(+) create mode 100644 lib/web/controllers/batch_controller.ex create mode 100644 lib/web/templates/batch/show.html.eex create mode 100644 lib/web/views/batch_view.ex diff --git a/lib/web/controllers/batch_controller.ex b/lib/web/controllers/batch_controller.ex new file mode 100644 index 000000000..64a4dd7f4 --- /dev/null +++ b/lib/web/controllers/batch_controller.ex @@ -0,0 +1,23 @@ +defmodule BorsNG.BatchController do + @moduledoc """ + The controller for the batches + + This will either show a batch detail page + """ + + use BorsNG.Web, :controller + + alias BorsNG.Database.Batch + alias BorsNG.Database.Repo + alias BorsNG.Database.Patch + alias BorsNG.Database.Project + alias BorsNG.Database.Status + + def show(conn, %{"id" => id}) do + batch = Repo.get(Batch, id) + project = Repo.get(Project, batch.project_id) + patches = Repo.all(Patch.all_for_batch(batch.id)) + statuses = Repo.all(Status.all_for_batch(batch.id)) + render conn, "show.html", batch: batch, patches: patches, project: project, statuses: statuses + end +end diff --git a/lib/web/router.ex b/lib/web/router.ex index c57ddb1f3..f463a6322 100644 --- a/lib/web/router.ex +++ b/lib/web/router.ex @@ -45,6 +45,14 @@ defmodule BorsNG.Router do get "/", PageController, :index end + scope "/batches", BorsNG do + pipe_through :browser_page + pipe_through :browser_session + pipe_through :browser_login + + get "/:id", BatchController, :show + end + scope "/repositories", BorsNG do pipe_through :browser_page pipe_through :browser_session diff --git a/lib/web/templates/batch/show.html.eex b/lib/web/templates/batch/show.html.eex new file mode 100644 index 000000000..b27af8b9e --- /dev/null +++ b/lib/web/templates/batch/show.html.eex @@ -0,0 +1,39 @@ +
+ +

Batch details

+ + + + <%= if @batch do %> + + + + + + + + + + + + + + + + + + <% else %> + There is no such batch + <% end %> + +
State<%= @batch.state %>
Priority<%= @batch.priority %>
Pull requests + <%= for patch <- @patches do %> + #<%= patch.pr_xref %> + <% end %> +
Statuses + <%= for status <- @statuses do %> + <%= status.identifier %> + <% end %> +
+ +
diff --git a/lib/web/views/batch_view.ex b/lib/web/views/batch_view.ex new file mode 100644 index 000000000..b9f9be9cb --- /dev/null +++ b/lib/web/views/batch_view.ex @@ -0,0 +1,7 @@ +defmodule BorsNG.BatchView do + @moduledoc """ + Batch details page + """ + + use BorsNG.Web, :view +end From 3b95af526a272c1acbb5b776613afaf48f7c3db4 Mon Sep 17 00:00:00 2001 From: Adam Neumann Date: Mon, 23 Dec 2019 13:57:50 -0700 Subject: [PATCH 2/6] Update view to fit bors look and feel --- lib/web/templates/batch/show.html.eex | 56 +++++++++------------ lib/web/templates/project/log_page.html.eex | 2 + lib/web/views/batch_view.ex | 11 ++++ 3 files changed, 36 insertions(+), 33 deletions(-) diff --git a/lib/web/templates/batch/show.html.eex b/lib/web/templates/batch/show.html.eex index b27af8b9e..b4d86d0f9 100644 --- a/lib/web/templates/batch/show.html.eex +++ b/lib/web/templates/batch/show.html.eex @@ -1,39 +1,29 @@
-

Batch details

+

Batch Details

- - - <%= if @batch do %> - - - - - - - - - - - - - - - - - - <% else %> - There is no such batch +<%= if @batch do %> +

Priority: <%= @batch.priority %>

+

State: <%= stringify_state(@batch.state) %>

+

+ Status: + <%= for status <- @statuses do %> + <%= if status.url do %> + <%= status.identifier %> + <% else %> + <%= status.identifier %> + <% end %> + <% end %> +

+

+ Pull Requests: + <%= for patch <- @patches do %> + #<%= patch.pr_xref %> <% end %> -

-
State<%= @batch.state %>
Priority<%= @batch.priority %>
Pull requests - <%= for patch <- @patches do %> - #<%= patch.pr_xref %> - <% end %> -
Statuses - <%= for status <- @statuses do %> - <%= status.identifier %> - <% end %> -
+

+ +<% else %> +There is no such batch +<% end %>
diff --git a/lib/web/templates/project/log_page.html.eex b/lib/web/templates/project/log_page.html.eex index 534c32f9e..015a7d621 100644 --- a/lib/web/templates/project/log_page.html.eex +++ b/lib/web/templates/project/log_page.html.eex @@ -3,6 +3,7 @@ <% %BorsNG.Database.Crash{id: id, component: component, crash: crash, updated_at: updated_at} -> %> <%= htmlify_naive_datetime(updated_at) %> + Crash <%= component %> 
<%= crash %>
@@ -10,6 +11,7 @@ <% %BorsNG.Database.Batch{id: id, state: state, patches: patches, updated_at: updated_at} -> %> <%= htmlify_naive_datetime(updated_at) %> + <%= id %> Batch <%= stringify_state(state) %> diff --git a/lib/web/views/batch_view.ex b/lib/web/views/batch_view.ex index b9f9be9cb..c1782783e 100644 --- a/lib/web/views/batch_view.ex +++ b/lib/web/views/batch_view.ex @@ -4,4 +4,15 @@ defmodule BorsNG.BatchView do """ use BorsNG.Web, :view + + def stringify_state(state) do + case state do + :waiting -> "Waiting to run" + :running -> "Running" + :ok -> "Succeeded" + :error -> "Failed" + :canceled -> "Canceled" + _ -> "Invalid" + end + end end From 470d8ffe8033837ff47ffa9f2d4943d9f5aabfa1 Mon Sep 17 00:00:00 2001 From: Adam Neumann Date: Mon, 23 Dec 2019 15:21:27 -0700 Subject: [PATCH 3/6] Backfill test --- test/controllers/batch_controller_test.exs | 80 ++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 test/controllers/batch_controller_test.exs diff --git a/test/controllers/batch_controller_test.exs b/test/controllers/batch_controller_test.exs new file mode 100644 index 000000000..322cedfb6 --- /dev/null +++ b/test/controllers/batch_controller_test.exs @@ -0,0 +1,80 @@ +defmodule BorsNG.BatchControllerTest do + use BorsNG.ConnCase + + alias BorsNG.Database.Batch + alias BorsNG.Database.Installation + alias BorsNG.Database.LinkPatchBatch + alias BorsNG.Database.Patch + alias BorsNG.Database.Project + alias BorsNG.Database.Repo + alias BorsNG.Database.Status + alias BorsNG.Database.User + + setup do + installation = Repo.insert!(%Installation{ + installation_xref: 31, + }) + project = Repo.insert!(%Project{ + installation_id: installation.id, + repo_xref: 13, + name: "example/project", + }) + user = Repo.insert!(%User{ + user_xref: 23, + login: "ghost", + }) + patch = Repo.insert!(%Patch{ + project_id: project.id, + pr_xref: 43 + }) + batch = Repo.insert!(%Batch{ + project_id: project.id, + priority: 33 + }) + {:ok, installation: installation, project: project, user: user, batch: batch, patch: patch} + end + + test "need to log in to see this", %{conn: conn, batch: batch} do + conn = get conn, "/batches/#{batch.id}" + assert html_response(conn, 302) =~ "auth" + end + + def login(conn) do + conn = get conn, auth_path(conn, :index, "github") + assert html_response(conn, 302) =~ "MOCK_GITHUB_AUTHORIZE_URL" + conn = get conn, auth_path( + conn, + :callback, + "github", + %{"code" => "MOCK_GITHUB_AUTHORIZE_CODE"}) + html_response(conn, 302) + conn + end + + test "shows the batch details", %{conn: conn, patch: patch, batch: batch} do + Repo.insert!(%LinkPatchBatch{ + patch_id: patch.id, + batch_id: batch.id, + }) + Repo.insert!(%Status{ + batch_id: batch.id, + identifier: "some-identifier" + }) + Repo.insert!(%Status{ + batch_id: batch.id, + identifier: "with-url-identifier", + url: "http://example.com" + }) + + conn = login conn + conn = get conn, "/batches/#{batch.id}" + + assert html_response(conn, 200) =~ "Batch Details" + assert html_response(conn, 200) =~ "Priority: 33" + assert html_response(conn, 200) =~ "State: Invalid" + assert html_response(conn, 200) =~ "#43" + assert html_response(conn, 200) =~ "some-identifier" + assert html_response(conn, 200) =~ ~S(with-url-identifier) + end + +end From f9b19da5b49e236ddea9856bc1d351844b583efb Mon Sep 17 00:00:00 2001 From: Adam Neumann Date: Tue, 24 Dec 2019 13:04:27 -0700 Subject: [PATCH 4/6] Expand tests to cover important authorization cases --- lib/web/controllers/batch_controller.ex | 19 +++++++ test/controllers/batch_controller_test.exs | 65 +++++++++++++++++----- 2 files changed, 69 insertions(+), 15 deletions(-) diff --git a/lib/web/controllers/batch_controller.ex b/lib/web/controllers/batch_controller.ex index 64a4dd7f4..54310655f 100644 --- a/lib/web/controllers/batch_controller.ex +++ b/lib/web/controllers/batch_controller.ex @@ -8,6 +8,7 @@ defmodule BorsNG.BatchController do use BorsNG.Web, :controller alias BorsNG.Database.Batch + alias BorsNG.Database.Context.Permission alias BorsNG.Database.Repo alias BorsNG.Database.Patch alias BorsNG.Database.Project @@ -16,6 +17,24 @@ defmodule BorsNG.BatchController do def show(conn, %{"id" => id}) do batch = Repo.get(Batch, id) project = Repo.get(Project, batch.project_id) + + allow_private_repos = Confex.fetch_env!(:bors, BorsNG)[:allow_private_repos] + admin? = conn.assigns.user.is_admin + mode = conn.assigns.user + |> Permission.get_permission(project) + |> case do + _ when admin? -> :rw + :reviewer -> :rw + :member -> :ro + _ when not allow_private_repos -> :ro + _ -> raise BorsNG.PermissionDeniedError + end + + case mode do + :ro -> raise BorsNG.PermissionDeniedError + _ -> + end + patches = Repo.all(Patch.all_for_batch(batch.id)) statuses = Repo.all(Status.all_for_batch(batch.id)) render conn, "show.html", batch: batch, patches: patches, project: project, statuses: statuses diff --git a/test/controllers/batch_controller_test.exs b/test/controllers/batch_controller_test.exs index 322cedfb6..bd217b865 100644 --- a/test/controllers/batch_controller_test.exs +++ b/test/controllers/batch_controller_test.exs @@ -4,6 +4,8 @@ defmodule BorsNG.BatchControllerTest do alias BorsNG.Database.Batch alias BorsNG.Database.Installation alias BorsNG.Database.LinkPatchBatch + alias BorsNG.Database.LinkMemberProject + alias BorsNG.Database.LinkUserProject alias BorsNG.Database.Patch alias BorsNG.Database.Project alias BorsNG.Database.Repo @@ -31,7 +33,20 @@ defmodule BorsNG.BatchControllerTest do project_id: project.id, priority: 33 }) - {:ok, installation: installation, project: project, user: user, batch: batch, patch: patch} + Repo.insert!(%LinkPatchBatch{ + patch_id: patch.id, + batch_id: batch.id, + }) + Repo.insert!(%Status{ + batch_id: batch.id, + identifier: "some-identifier" + }) + Repo.insert!(%Status{ + batch_id: batch.id, + identifier: "with-url-identifier", + url: "http://example.com" + }) + {:ok, installation: installation, project: project, user: user, batch: batch, user: user} end test "need to log in to see this", %{conn: conn, batch: batch} do @@ -51,19 +66,18 @@ defmodule BorsNG.BatchControllerTest do conn end - test "shows the batch details", %{conn: conn, patch: patch, batch: batch} do - Repo.insert!(%LinkPatchBatch{ - patch_id: patch.id, - batch_id: batch.id, - }) - Repo.insert!(%Status{ - batch_id: batch.id, - identifier: "some-identifier" - }) - Repo.insert!(%Status{ - batch_id: batch.id, - identifier: "with-url-identifier", - url: "http://example.com" + test "hides batch details from unlinked user", %{conn: conn, batch: batch} do + conn = login conn + + assert_raise BorsNG.PermissionDeniedError, fn -> + get conn, "/batches/#{batch.id}" + end + end + + test "shows the batch details as a reviewer", %{conn: conn, batch: batch, user: user, project: project} do + Repo.insert!(%LinkUserProject{ + user_id: user.id, + project_id: project.id }) conn = login conn @@ -77,4 +91,25 @@ defmodule BorsNG.BatchControllerTest do assert html_response(conn, 200) =~ ~S(with-url-identifier) end -end + test "hides batch details from a member", %{conn: conn, batch: batch, user: user, project: project} do + Repo.insert!(%LinkMemberProject{ + user_id: user.id, + project_id: project.id + }) + + conn = login conn + + assert_raise BorsNG.PermissionDeniedError, fn -> + get conn, "/batches/#{batch.id}" + end + end + + test "shows the batch details for an admin", %{conn: conn, batch: batch, user: user} do + Repo.update!(User.changeset(user, %{is_admin: true})) + + conn = login conn + conn = get conn, "/batches/#{batch.id}" + + assert html_response(conn, 200) =~ "Batch Details" + end +end \ No newline at end of file From 3a62199d80242c99613972f91a56b57b0ae4c4ce Mon Sep 17 00:00:00 2001 From: Adam Neumann Date: Tue, 24 Dec 2019 13:08:28 -0700 Subject: [PATCH 5/6] Combine batch details link with entry type cell --- lib/web/templates/project/log_page.html.eex | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/web/templates/project/log_page.html.eex b/lib/web/templates/project/log_page.html.eex index 015a7d621..9c58e15c4 100644 --- a/lib/web/templates/project/log_page.html.eex +++ b/lib/web/templates/project/log_page.html.eex @@ -3,7 +3,6 @@ <% %BorsNG.Database.Crash{id: id, component: component, crash: crash, updated_at: updated_at} -> %> <%= htmlify_naive_datetime(updated_at) %> - Crash <%= component %> 
<%= crash %>
@@ -11,8 +10,7 @@ <% %BorsNG.Database.Batch{id: id, state: state, patches: patches, updated_at: updated_at} -> %> <%= htmlify_naive_datetime(updated_at) %> - <%= id %> - Batch + Batch <%= id %> <%= stringify_state(state) %> <%= for patch <- patches do %> From f29f98cdf71bbda38ef3acf63173e53b50e75622 Mon Sep 17 00:00:00 2001 From: Chao Lin Date: Thu, 26 Dec 2019 14:07:39 -0800 Subject: [PATCH 6/6] add state for statuses --- lib/web/templates/batch/show.html.eex | 4 ++-- test/controllers/batch_controller_test.exs | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/web/templates/batch/show.html.eex b/lib/web/templates/batch/show.html.eex index b4d86d0f9..358137eba 100644 --- a/lib/web/templates/batch/show.html.eex +++ b/lib/web/templates/batch/show.html.eex @@ -9,9 +9,9 @@ Status: <%= for status <- @statuses do %> <%= if status.url do %> - <%= status.identifier %> + <%= status.identifier %> (<%= stringify_state(status.state) %>) <% else %> - <%= status.identifier %> + <%= status.identifier %> (<%= stringify_state(status.state) %>) <% end %> <% end %>

diff --git a/test/controllers/batch_controller_test.exs b/test/controllers/batch_controller_test.exs index bd217b865..6952ff651 100644 --- a/test/controllers/batch_controller_test.exs +++ b/test/controllers/batch_controller_test.exs @@ -39,12 +39,14 @@ defmodule BorsNG.BatchControllerTest do }) Repo.insert!(%Status{ batch_id: batch.id, - identifier: "some-identifier" + identifier: "some-identifier", + state: :running }) Repo.insert!(%Status{ batch_id: batch.id, identifier: "with-url-identifier", - url: "http://example.com" + url: "http://example.com", + state: :waiting }) {:ok, installation: installation, project: project, user: user, batch: batch, user: user} end @@ -87,8 +89,8 @@ defmodule BorsNG.BatchControllerTest do assert html_response(conn, 200) =~ "Priority: 33" assert html_response(conn, 200) =~ "State: Invalid" assert html_response(conn, 200) =~ "#43" - assert html_response(conn, 200) =~ "some-identifier" - assert html_response(conn, 200) =~ ~S(with-url-identifier) + assert html_response(conn, 200) =~ "some-identifier (Running)" + assert html_response(conn, 200) =~ ~s(with-url-identifier (Waiting to run\)) end test "hides batch details from a member", %{conn: conn, batch: batch, user: user, project: project} do