Add/Fix tests

plausible · May 9, 2024 · dc1ad1e · dc1ad1e
1 parent 5e73aea
commit dc1ad1e
Show file tree

Hide file tree

Showing 8 changed files with 345 additions and 136 deletions.
diff --git a/assets/js/dashboard/stats/sources/search-terms.js b/assets/js/dashboard/stats/sources/search-terms.js
@@ -40,7 +40,7 @@ export default class SearchTerms extends React.Component {
         searchTerms: res.search_terms || [],
         notConfigured: res.not_configured,
         isAdmin: res.is_admin,
-        invalidFilters: res.invalid_filters
+        unsupportedFilters: res.unsupported_filters
       })).catch((error) =>
         {
             this.setState({ loading: false, searchTerms: [], notConfigured: true, error: true, isAdmin: error.payload.is_admin })
@@ -69,7 +69,7 @@ export default class SearchTerms extends React.Component {
   }
 
   renderList() {
-   if (this.state.invalidFilters)  {
+   if (this.state.unsupportedFilters)  {
       return (
         <div className="text-center text-gray-700 dark:text-gray-300 text-sm mt-20">
           <RocketIcon />
@@ -81,7 +81,7 @@ export default class SearchTerms extends React.Component {
         <div className="text-center text-gray-700 dark:text-gray-300 text-sm mt-20">
           <RocketIcon />
           <div>
-          This site is not connected to Search Console so we cannot show the search phrases.
+          This site is not connected to Search Console so we cannot show the search terms
           {this.state.isAdmin && this.state.error && <><br/><br/><p>Please click below to connect your Search Console account.</p></>}
           </div>
           {this.state.isAdmin && <a href={`/${encodeURIComponent(this.props.site.domain)}/settings/integrations`} className="button mt-4">Connect with Google</a> }

diff --git a/lib/plausible/google/api.ex b/lib/plausible/google/api.ex
@@ -6,9 +6,9 @@ defmodule Plausible.Google.API do
   use Timex
 
   alias Plausible.Google.HTTP
+  alias Plausible.Google.SearchConsole
 
   require Logger
-  import Plausible.Stats.Base, only: [page_regex: 1]
 
   @search_console_scope URI.encode_www_form(
                           "email https://www.googleapis.com/auth/webmasters.readonly"
@@ -78,7 +78,7 @@ defmodule Plausible.Google.API do
     with {:ok, site} <- ensure_search_console_property(site),
          {:ok, access_token} <- maybe_refresh_token(site.google_auth),
          {:ok, search_console_filters} <-
-           get_search_console_filters(site.google_auth.property, filters),
+           SearchConsole.Filters.transform(site.google_auth.property, filters),
          {:ok, stats} <-
            HTTP.list_stats(
              access_token,
@@ -94,8 +94,7 @@ defmodule Plausible.Google.API do
       |> then(&{:ok, &1})
     else
       :google_property_not_configured -> {:err, :google_property_not_configured}
-      # Show empty report to user with message about not being able to get keyword data for this set of filters
-      :invalid_filters -> {:err, :invalid_filters}
+      :unsupported_filters -> {:err, :unsupported_filters}
       {:error, error} -> {:error, error}
     end
   end
@@ -150,89 +149,6 @@ defmodule Plausible.Google.API do
     Timex.before?(expires_at, thirty_seconds_ago)
   end
 
-  defp get_search_console_filters(property, plausible_filters) do
-    plausible_filters = Map.drop(plausible_filters, ["visit:source"])
-
-    search_console_filters =
-      Enum.reduce_while(plausible_filters, [], fn plausible_filter, search_console_filters ->
-        case transform_filter(property, plausible_filter) do
-          :err -> {:halt, :invalid_filters}
-          search_console_filter -> {:cont, [search_console_filter | search_console_filters]}
-        end
-      end)
-
-    case search_console_filters do
-      :invalid_filters -> :invalid_filters
-      [] -> {:ok, []}
-      filters when is_list(filters) -> {:ok, [%{filters: filters}]}
-    end
-  end
-
-  defp transform_filter(property, {"event:page", filter}) do
-    transform_filter(property, {"visit:entry_page", filter})
-  end
-
-  defp transform_filter(property, {"visit:entry_page", {:is, page}}) when is_binary(page) do
-    %{dimension: "page", expression: property_url(property, page)}
-  end
-
-  defp transform_filter(property, {"visit:entry_page", {:member, pages}}) when is_list(pages) do
-    expression =
-      Enum.map(pages, fn page -> property_url(property, Regex.escape(page)) end) |> Enum.join("|")
-
-    %{dimension: "page", operator: "includingRegex", expression: expression}
-  end
-
-  defp transform_filter(property, {"visit:entry_page", {:matches, page}}) when is_binary(page) do
-    page = page_regex(property_url(property, page))
-    %{dimension: "page", operator: "includingRegex", expression: page}
-  end
-
-  defp transform_filter(property, {"visit:entry_page", {:matches_member, pages}})
-       when is_list(pages) do
-    expression =
-      Enum.map(pages, fn page -> page_regex(property_url(property, page)) end) |> Enum.join("|")
-
-    %{dimension: "page", operator: "includingRegex", expression: expression}
-  end
-
-  defp transform_filter(_property, {"visit:screen", {:is, device}}) when is_binary(device) do
-    %{dimension: "device", expression: search_console_device(device)}
-  end
-
-  defp transform_filter(_property, {"visit:screen", {:is, device}}) when is_binary(device) do
-    %{dimension: "device", expression: search_console_device(device)}
-  end
-
-  defp transform_filter(_property, {"visit:screen", {:member, devices}}) when is_list(devices) do
-    expression = devices |> Enum.join("|")
-    %{dimension: "device", operator: "includingRegex", expression: expression}
-  end
-
-  defp transform_filter(_property, {"visit:country", {:is, country}}) when is_binary(country) do
-    %{dimension: "country", expression: search_console_country(country)}
-  end
-
-  defp transform_filter(_property, {"visit:country", {:member, countries}})
-       when is_list(countries) do
-    expression = Enum.map(countries, &search_console_country/1) |> Enum.join("|")
-    %{dimension: "country", operator: "includingRegex", expression: expression}
-  end
-
-  defp transform_filter(_, _filter), do: :err
-
-  defp property_url("sc-domain:" <> domain, page), do: "https://" <> domain <> page
-  defp property_url(url, page), do: url <> page
-
-  defp search_console_device("Desktop"), do: "DESKTOP"
-  defp search_console_device("Mobile"), do: "MOBILE"
-  defp search_console_device("Tablet"), do: "TABLET"
-
-  defp search_console_country(alpha_2) do
-    country = Location.Country.get_country(alpha_2)
-    country.alpha_3
-  end
-
   defp ensure_search_console_property(site) do
     site = Plausible.Repo.preload(site, :google_auth)
 

diff --git a/lib/plausible/google/search_console/filters.ex b/lib/plausible/google/search_console/filters.ex
@@ -0,0 +1,82 @@
+defmodule Plausible.Google.SearchConsole.Filters do
+  import Plausible.Stats.Base, only: [page_regex: 1]
+
+  def transform(property, plausible_filters) do
+    plausible_filters = Map.drop(plausible_filters, ["visit:source"])
+
+    search_console_filters =
+      Enum.reduce_while(plausible_filters, [], fn plausible_filter, search_console_filters ->
+        case transform_filter(property, plausible_filter) do
+          :err -> {:halt, :unsupported_filters}
+          search_console_filter -> {:cont, [search_console_filter | search_console_filters]}
+        end
+      end)
+
+    case search_console_filters do
+      :unsupported_filters -> :unsupported_filters
+      [] -> {:ok, []}
+      filters when is_list(filters) -> {:ok, [%{filters: filters}]}
+    end
+  end
+
+  defp transform_filter(property, {"event:page", filter}) do
+    transform_filter(property, {"visit:entry_page", filter})
+  end
+
+  defp transform_filter(property, {"visit:entry_page", {:is, page}}) when is_binary(page) do
+    %{dimension: "page", expression: property_url(property, page)}
+  end
+
+  defp transform_filter(property, {"visit:entry_page", {:member, pages}}) when is_list(pages) do
+    expression =
+      Enum.map(pages, fn page -> property_url(property, Regex.escape(page)) end) |> Enum.join("|")
+
+    %{dimension: "page", operator: "includingRegex", expression: expression}
+  end
+
+  defp transform_filter(property, {"visit:entry_page", {:matches, page}}) when is_binary(page) do
+    page = page_regex(property_url(property, page))
+    %{dimension: "page", operator: "includingRegex", expression: page}
+  end
+
+  defp transform_filter(property, {"visit:entry_page", {:matches_member, pages}})
+       when is_list(pages) do
+    expression =
+      Enum.map(pages, fn page -> page_regex(property_url(property, page)) end) |> Enum.join("|")
+
+    %{dimension: "page", operator: "includingRegex", expression: expression}
+  end
+
+  defp transform_filter(_property, {"visit:screen", {:is, device}}) when is_binary(device) do
+    %{dimension: "device", expression: search_console_device(device)}
+  end
+
+  defp transform_filter(_property, {"visit:screen", {:member, devices}}) when is_list(devices) do
+    expression = devices |> Enum.join("|")
+    %{dimension: "device", operator: "includingRegex", expression: expression}
+  end
+
+  defp transform_filter(_property, {"visit:country", {:is, country}}) when is_binary(country) do
+    %{dimension: "country", expression: search_console_country(country)}
+  end
+
+  defp transform_filter(_property, {"visit:country", {:member, countries}})
+       when is_list(countries) do
+    expression = Enum.map(countries, &search_console_country/1) |> Enum.join("|")
+    %{dimension: "country", operator: "includingRegex", expression: expression}
+  end
+
+  defp transform_filter(_, _filter), do: :err
+
+  defp property_url("sc-domain:" <> domain, page), do: "https://" <> domain <> page
+  defp property_url(url, page), do: url <> page
+
+  defp search_console_device("Desktop"), do: "DESKTOP"
+  defp search_console_device("Mobile"), do: "MOBILE"
+  defp search_console_device("Tablet"), do: "TABLET"
+
+  defp search_console_country(alpha_2) do
+    country = Location.Country.get_country(alpha_2)
+    country.alpha_3
+  end
+end
diff --git a/lib/plausible_web/controllers/api/stats_controller.ex b/lib/plausible_web/controllers/api/stats_controller.ex
@@ -691,8 +691,8 @@ defmodule PlausibleWeb.Api.StatsController do
       {:err, :google_propery_not_configured} ->
         json(conn, %{not_configured: true, is_admin: is_admin})
 
-      {:err, :invalid_filters} ->
-        json(conn, %{invalid_filters: true})
+      {:err, :unsupported_filters} ->
+        json(conn, %{unsupported_filters: true})
 
       {:ok, terms} ->
         json(conn, %{search_terms: terms})

diff --git a/test/plausible/google/api_test.exs b/test/plausible/google/api_test.exs
@@ -301,17 +301,44 @@ defmodule Plausible.Google.APITest do
     end
   end
 
-  describe "fetch_stats/3 with Mox" do
-    test "returns name and visitor count", %{user: user, site: site} do
-      mock_http_with("google_analytics_stats.json")
+  test "returns error when token refresh fails", %{user: user, site: site} do
+    mock_http_with("google_analytics_auth#invalid_grant.json")
+
+    insert(:google_auth,
+      user: user,
+      site: site,
+      property: "sc-domain:dummy.test",
+      access_token: "*****",
+      refresh_token: "*****",
+      expires: NaiveDateTime.add(NaiveDateTime.utc_now(), -3600)
+    )
+
+    query = %Plausible.Stats.Query{date_range: Date.range(~D[2022-01-01], ~D[2022-01-05])}
+
+    assert {:error, "invalid_grant"} = Google.API.fetch_stats(site, query, 5)
+  end
 
+  test "returns error when google auth not configured", %{site: site} do
+    query = %Plausible.Stats.Query{date_range: Date.range(~D[2022-01-01], ~D[2022-01-05])}
+
+    assert {:err, :google_property_not_configured} = Google.API.fetch_stats(site, query, 5)
+  end
+
+  describe "fetch_stats/3 with valid auth" do
+    setup %{user: user, site: site} do
       insert(:google_auth,
         user: user,
         site: site,
         property: "sc-domain:dummy.test",
         expires: NaiveDateTime.add(NaiveDateTime.utc_now(), 3600)
       )
 
+      :ok
+    end
+
+    test "returns name and visitor count", %{site: site} do
+      mock_http_with("google_analytics_stats.json")
+
       query = %Plausible.Stats.Query{date_range: Date.range(~D[2022-01-01], ~D[2022-01-05])}
 
       assert {:ok,
@@ -321,43 +348,49 @@ defmodule Plausible.Google.APITest do
               ]} = Google.API.fetch_stats(site, query, 5)
     end
 
-    # test "returns next page when page argument is set", %{user: user, site: site} do
-    #   mock_http_with("google_analytics_stats#with_page.json")
-
-    #   insert(:google_auth,
-    #     user: user,
-    #     site: site,
-    #     property: "sc-domain:dummy.test",
-    #     expires: NaiveDateTime.add(NaiveDateTime.utc_now(), 3600)
-    #   )
-
-    #   query = %Plausible.Stats.Query{
-    #     filters: %{"event:page" => {:is, "/5"}},
-    #     date_range: Date.range(~D[2022-01-01], ~D[2022-01-05])
-    #   }
-
-    #   assert {:ok,
-    #           [
-    #             %{name: ["keyword1", "keyword2"], visitors: 25},
-    #             %{name: ["keyword3", "keyword4"], visitors: 15}
-    #           ]} = Google.API.fetch_stats(site, query, 5)
-    # end
-
-    test "returns error when token refresh fails", %{user: user, site: site} do
-      mock_http_with("google_analytics_auth#invalid_grant.json")
-
-      insert(:google_auth,
-        user: user,
-        site: site,
-        property: "sc-domain:dummy.test",
-        access_token: "*****",
-        refresh_token: "*****",
-        expires: NaiveDateTime.add(NaiveDateTime.utc_now(), -3600)
+    test "transforms page filters to search console format", %{site: site} do
+      expect(
+        Plausible.HTTPClient.Mock,
+        :post,
+        fn
+          "https://www.googleapis.com/webmasters/v3/sites/sc-domain%3Adummy.test/searchAnalytics/query",
+          [{"Authorization", "Bearer 123"}],
+          %{
+            dimensionFilterGroups: [
+              %{filters: [%{expression: "https://dummy.test/page", dimension: "page"}]}
+            ],
+            dimensions: ["query"],
+            endDate: "2022-01-05",
+            rowLimit: 5,
+            startDate: "2022-01-01"
+          } ->
+            {:ok, %Finch.Response{status: 200, body: %{"rows" => []}}}
+        end
       )
 
-      query = %Plausible.Stats.Query{date_range: Date.range(~D[2022-01-01], ~D[2022-01-05])}
+      query =
+        Plausible.Stats.Query.from(site, %{
+          "period" => "custom",
+          "from" => "2022-01-01",
+          "to" => "2022-01-05",
+          "filters" => "event:page==/page"
+        })
+
+      assert {:ok, []} = Google.API.fetch_stats(site, query, 5)
+    end
 
-      assert {:error, "invalid_grant"} = Google.API.fetch_stats(site, query, 5)
+    test "returns :invalid filters when using filters that cannot be used in Search Console", %{
+      site: site
+    } do
+      query =
+        Plausible.Stats.Query.from(site, %{
+          "period" => "custom",
+          "from" => "2022-01-01",
+          "to" => "2022-01-05",
+          "filters" => "event:goal==Signup"
+        })
+
+      assert {:err, :unsupported_filters} = Google.API.fetch_stats(site, query, 5)
     end
   end