From d31db86b494e34b4582860c3bda3c49ef5e1e786 Mon Sep 17 00:00:00 2001 From: Vinicius Brasil Date: Thu, 8 Sep 2022 11:02:17 -0700 Subject: [PATCH] List all Google Analytics views during import (#2184) * List all Google Analytics views during import This commit fixes a bug where different Google Analytics views with the same name and URI were not shown. This was caused because GA views were stored as a map, that naturally doesn't support duplicate keys. This change updates the GA views list to display view IDs, making it clearer to know what is being imported. The dropdown is now grouped by website URL. * Put Google Analytics API URLs in app env * Add controller test to GA view list --- config/runtime.exs | 2 + fixture/ga_list_views.json | 67 +++++++++++++++++++ lib/plausible/google/api.ex | 49 +++++++++++--- lib/plausible/google/http.ex | 17 ++--- .../controllers/site_controller.ex | 9 ++- .../controllers/site_controller_test.exs | 33 ++++++++- 6 files changed, 152 insertions(+), 25 deletions(-) create mode 100644 fixture/ga_list_views.json diff --git a/config/runtime.exs b/config/runtime.exs index c2eecae3b..5b01a1c26 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -243,6 +243,8 @@ config :plausible, :paddle, config :plausible, :google, client_id: google_cid, client_secret: google_secret, + api_url: "https://www.googleapis.com", + reporting_api_url: "https://analyticsreporting.googleapis.com", max_buffer_size: get_int_from_path_or_env(config_dir, "GOOGLE_MAX_BUFFER_SIZE", 10_000) config :plausible, Plausible.ClickhouseRepo, diff --git a/fixture/ga_list_views.json b/fixture/ga_list_views.json new file mode 100644 index 000000000..1dce94274 --- /dev/null +++ b/fixture/ga_list_views.json @@ -0,0 +1,67 @@ +{ + "kind": "analytics#profiles", + "username": "email@provider.test", + "totalResults": 2, + "startIndex": 1, + "itemsPerPage": 1000, + "items": [ + { + "id": "57238190", + "kind": "analytics#profile", + "selfLink": "https://www.googleapis.com/analytics/v3/management/accounts/61782930/webproperties/UA-1625528-17/profiles/57238190", + "accountId": "61782930", + "webPropertyId": "UA-61782930-17", + "internalWebPropertyId": "53451925", + "name": "one.test", + "currency": "USD", + "timezone": "Europe/London", + "websiteUrl": "http://one.test/", + "type": "WEB", + "permissions": { + "effective": [ + "READ_AND_ANALYZE" + ] + }, + "created": "2011-12-25T15:16:53.069Z", + "updated": "2012-07-26T09:45:03.811Z", + "eCommerceTracking": false, + "parentLink": { + "type": "analytics#webproperty", + "href": "https://www.googleapis.com/analytics/v3/management/accounts/61782930/webproperties/UA-1625528-17" + }, + "childLink": { + "type": "analytics#goals", + "href": "https://www.googleapis.com/analytics/v3/management/accounts/61782930/webproperties/UA-1625528-17/profiles/57238190/goals" + } + }, + { + "id": "54460083", + "kind": "analytics#profile", + "selfLink": "https://www.googleapis.com/analytics/v3/management/accounts/61782930/webproperties/UA-1625528-18/profiles/54460083", + "accountId": "61782930", + "webPropertyId": "UA-61782930-18", + "internalWebPropertyId": "53597744", + "name": "two.test", + "currency": "USD", + "timezone": "Europe/London", + "websiteUrl": "http://two.test/", + "type": "WEB", + "permissions": { + "effective": [ + "READ_AND_ANALYZE" + ] + }, + "created": "2011-12-31T19:14:11.434Z", + "updated": "2012-04-03T18:34:11.475Z", + "eCommerceTracking": false, + "parentLink": { + "type": "analytics#webproperty", + "href": "https://www.googleapis.com/analytics/v3/management/accounts/61782930/webproperties/UA-1625528-18" + }, + "childLink": { + "type": "analytics#goals", + "href": "https://www.googleapis.com/analytics/v3/management/accounts/61782930/webproperties/UA-1625528-18/profiles/54460083/goals" + } + } + ] +} diff --git a/lib/plausible/google/api.ex b/lib/plausible/google/api.ex index 8c3ac11d3..caa5a6e05 100644 --- a/lib/plausible/google/api.ex +++ b/lib/plausible/google/api.ex @@ -3,6 +3,8 @@ defmodule Plausible.Google.Api do use Timex require Logger + @type google_analytics_view() :: {view_name :: String.t(), view_id :: String.t()} + @scope URI.encode_www_form( "https://www.googleapis.com/auth/webmasters.readonly email https://www.googleapis.com/auth/analytics.readonly" ) @@ -53,27 +55,52 @@ defmodule Plausible.Google.Api do end end - def get_analytics_view_ids(access_token) do + @spec list_views(access_token :: String.t()) :: + {:ok, %{(hostname :: String.t()) => [google_analytics_view()]}} | {:error, term()} + @doc """ + Lists Google Analytics views grouped by hostname. + """ + def list_views(access_token) do case HTTP.list_views_for_user(access_token) do {:ok, %{"items" => views}} -> - view_ids = for view <- views, do: build_view_ids(view), into: %{} - {:ok, view_ids} + views = Enum.group_by(views, &view_hostname/1, &view_names/1) + {:ok, views} error -> error end end - defp build_view_ids(view) do - uri = URI.parse(Map.get(view, "websiteUrl", "")) - - if !uri.host do - Sentry.capture_message("No URI for view ID", extra: view) + defp view_hostname(view) do + case view do + %{"websiteUrl" => url} when is_binary(url) -> url |> URI.parse() |> Map.get(:host) + _any -> "Others" end + end - host = uri.host || Map.get(view, "id", "") - name = Map.get(view, "name") - {"#{host} - #{name}", Map.get(view, "id")} + defp view_names(%{"name" => name, "id" => id}) do + {"#{id} - #{name}", id} + end + + @spec get_view(access_token :: String.t(), lookup_id :: String.t()) :: + {:ok, google_analytics_view()} | {:ok, nil} | {:error, term()} + @doc """ + Returns a single Google Analytics view if the user has access to it. + """ + def get_view(access_token, lookup_id) do + case list_views(access_token) do + {:ok, views} -> + view = + views + |> Map.values() + |> List.flatten() + |> Enum.find(fn {_name, id} -> id == lookup_id end) + + {:ok, view} + + {:error, cause} -> + {:error, cause} + end end @per_page 10_000 diff --git a/lib/plausible/google/http.ex b/lib/plausible/google/http.ex index 78cb1afd8..41603dde9 100644 --- a/lib/plausible/google/http.ex +++ b/lib/plausible/google/http.ex @@ -30,7 +30,7 @@ defmodule Plausible.Google.HTTP do response = :post |> Finch.build( - "https://analyticsreporting.googleapis.com/v4/reports:batchGet", + "#{reporting_api_url()}/v4/reports:batchGet", [{"Authorization", "Bearer #{report_request.access_token}"}], params ) @@ -94,7 +94,7 @@ defmodule Plausible.Google.HTTP do end def list_sites(access_token) do - url = "https://www.googleapis.com/webmasters/v3/sites" + url = "#{api_url()}/webmasters/v3/sites" headers = [{"Content-Type", "application/json"}, {"Authorization", "Bearer #{access_token}"}] case HTTPClient.get(url, headers) do @@ -111,7 +111,7 @@ defmodule Plausible.Google.HTTP do end def fetch_access_token(code) do - url = "https://www.googleapis.com/oauth2/v4/token" + url = "#{api_url()}/oauth2/v4/token" headers = [{"Content-Type", "application/x-www-form-urlencoded"}] params = %{ @@ -130,8 +130,7 @@ defmodule Plausible.Google.HTTP do end def list_views_for_user(access_token) do - url = - "https://www.googleapis.com/analytics/v3/management/accounts/~all/webproperties/~all/profiles" + url = "#{api_url()}/analytics/v3/management/accounts/~all/webproperties/~all/profiles" headers = [{"Authorization", "Bearer #{access_token}"}] @@ -168,7 +167,7 @@ defmodule Plausible.Google.HTTP do dimensionFilterGroups: filter_groups } - url = "https://www.googleapis.com/webmasters/v3/sites/#{property}/searchAnalytics/query" + url = "#{api_url()}/webmasters/v3/sites/#{property}/searchAnalytics/query" headers = [{"Authorization", "Bearer #{access_token}"}] case HTTPClient.post(url, headers, params) do @@ -198,7 +197,7 @@ defmodule Plausible.Google.HTTP do defp property_base_url(url), do: url def refresh_auth_token(refresh_token) do - url = "https://www.googleapis.com/oauth2/v4/token" + url = "#{api_url()}/oauth2/v4/token" headers = [{"content-type", "application/x-www-form-urlencoded"}] params = %{ @@ -244,7 +243,7 @@ defmodule Plausible.Google.HTTP do ] } - url = "https://analyticsreporting.googleapis.com/v4/reports:batchGet" + url = "#{reporting_api_url()}/v4/reports:batchGet" headers = [{"Authorization", "Bearer #{access_token}"}] case HTTPClient.post(url, headers, params) do @@ -275,5 +274,7 @@ defmodule Plausible.Google.HTTP do defp config, do: Application.get_env(:plausible, :google) defp client_id, do: Keyword.fetch!(config(), :client_id) defp client_secret, do: Keyword.fetch!(config(), :client_secret) + defp reporting_api_url, do: Keyword.fetch!(config(), :reporting_api_url) + defp api_url, do: Keyword.fetch!(config(), :api_url) defp redirect_uri, do: PlausibleWeb.Endpoint.url() <> "/auth/google/callback" end diff --git a/lib/plausible_web/controllers/site_controller.ex b/lib/plausible_web/controllers/site_controller.ex index d4457f411..1dc855af5 100644 --- a/lib/plausible_web/controllers/site_controller.ex +++ b/lib/plausible_web/controllers/site_controller.ex @@ -652,7 +652,7 @@ defmodule PlausibleWeb.SiteController do def import_from_google_view_id_form(conn, %{"access_token" => access_token}) do site = conn.assigns[:site] - view_ids = Plausible.Google.Api.get_analytics_view_ids(access_token) + view_ids = Plausible.Google.Api.list_views(access_token) conn |> assign(:skip_plausible_tracking, true) @@ -673,7 +673,7 @@ defmodule PlausibleWeb.SiteController do case start_date do {:ok, nil} -> site = conn.assigns[:site] - view_ids = Plausible.Google.Api.get_analytics_view_ids(access_token) + view_ids = Plausible.Google.Api.list_views(access_token) conn |> assign(:skip_plausible_tracking, true) @@ -714,8 +714,7 @@ defmodule PlausibleWeb.SiteController do end_date = Plausible.Stats.Clickhouse.pageview_start_date_local(site) || Timex.today(site.timezone) - {:ok, view_ids} = Plausible.Google.Api.get_analytics_view_ids(access_token) - {view_id_name, _} = Enum.find(view_ids, fn {_, v} -> v == view_id end) + {:ok, {view_name, view_id}} = Plausible.Google.Api.get_view(access_token, view_id) conn |> assign(:skip_plausible_tracking, true) @@ -723,7 +722,7 @@ defmodule PlausibleWeb.SiteController do access_token: access_token, site: site, selected_view_id: view_id, - selected_view_id_name: view_id_name, + selected_view_id_name: view_name, start_date: start_date, end_date: end_date, layout: {PlausibleWeb.LayoutView, "focus.html"} diff --git a/test/plausible_web/controllers/site_controller_test.exs b/test/plausible_web/controllers/site_controller_test.exs index a018ea337..b4dc35ed8 100644 --- a/test/plausible_web/controllers/site_controller_test.exs +++ b/test/plausible_web/controllers/site_controller_test.exs @@ -1,5 +1,5 @@ defmodule PlausibleWeb.SiteControllerTest do - use PlausibleWeb.ConnCase + use PlausibleWeb.ConnCase, async: false use Plausible.Repo use Bamboo.Test use Oban.Testing, repo: Plausible.Repo @@ -722,6 +722,37 @@ defmodule PlausibleWeb.SiteControllerTest do end end + describe "GET /:website/import/google-analytics/view-id" do + setup [:create_user, :log_in, :create_new_site] + + test "lists Google Analytics views", %{conn: conn, site: site} do + bypass = Bypass.open() + + Bypass.expect_once( + bypass, + "GET", + "/analytics/v3/management/accounts/~all/webproperties/~all/profiles", + fn conn -> + response_body = File.read!("fixture/ga_list_views.json") + Plug.Conn.resp(conn, 200, response_body) + end + ) + + :plausible + |> Application.get_env(:google) + |> Keyword.put(:api_url, "http://0.0.0.0:#{bypass.port}") + |> then(&Application.put_env(:plausible, :google, &1)) + + response = + conn + |> get("/#{site.domain}/import/google-analytics/view-id", %{"access_token" => "token"}) + |> html_response(200) + + assert response =~ "57238190 - one.test" + assert response =~ "54460083 - two.test" + end + end + describe "POST /:website/settings/google-import" do setup [:create_user, :log_in, :create_new_site]