diff --git a/assets/js/dashboard/stats/sources/search-terms.js b/assets/js/dashboard/stats/sources/search-terms.js index 53ab72b19ed81..ea36b4987ccce 100644 --- 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 (
@@ -81,7 +81,7 @@ export default class SearchTerms extends React.Component {
- 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 && <>

Please click below to connect your Search Console account.

}
{this.state.isAdmin && Connect with Google } diff --git a/lib/plausible/google/api.ex b/lib/plausible/google/api.ex index ad139bd934463..4a16d3d2e678e 100644 --- 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 new file mode 100644 index 0000000000000..88ba6c9bdbd56 --- /dev/null +++ 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 index be7ff1c19b33b..c50a4aa298d07 100644 --- 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 index 4f6ee8cbec2ed..698ecd28d62cb 100644 --- a/test/plausible/google/api_test.exs +++ b/test/plausible/google/api_test.exs @@ -301,10 +301,31 @@ 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, @@ -312,6 +333,12 @@ defmodule Plausible.Google.APITest do 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 diff --git a/test/plausible/google/search_console/filters_test.exs b/test/plausible/google/search_console/filters_test.exs new file mode 100644 index 0000000000000..7d73dacda8b9b --- /dev/null +++ b/test/plausible/google/search_console/filters_test.exs @@ -0,0 +1,183 @@ +defmodule Plausible.Google.SearchConsole.FiltersTest do + alias Plausible.Google.SearchConsole.Filters + use Plausible.DataCase, async: true + + test "transforms simple page filter" do + filters = %{ + "visit:entry_page" => {:is, "/page"} + } + + {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) + + assert transformed == [ + %{filters: [%{dimension: "page", expression: "https://plausible.io/page"}]} + ] + end + + test "transforms matches page filter" do + filters = %{ + "visit:entry_page" => {:matches, "*page*"} + } + + {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) + + assert transformed == [ + %{ + filters: [ + %{ + dimension: "page", + operator: "includingRegex", + expression: "^https://plausible\\.io.*page.*$" + } + ] + } + ] + end + + test "transforms member page filter" do + filters = %{ + "visit:entry_page" => {:member, ["/pageA", "/pageB"]} + } + + {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) + + assert transformed == [ + %{ + filters: [ + %{ + dimension: "page", + operator: "includingRegex", + expression: "https://plausible.io/pageA|https://plausible.io/pageB" + } + ] + } + ] + end + + test "transforms matches_member page filter" do + filters = %{ + "visit:entry_page" => {:matches_member, ["/pageA*", "/pageB*"]} + } + + {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) + + assert transformed == [ + %{ + filters: [ + %{ + dimension: "page", + operator: "includingRegex", + expression: "^https://plausible\\.io/pageA.*$|^https://plausible\\.io/pageB.*$" + } + ] + } + ] + end + + test "transforms event:page exactly like visit:entry_page" do + filters = %{ + "event:page" => {:matches_member, ["/pageA*", "/pageB*"]} + } + + {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) + + assert transformed == [ + %{ + filters: [ + %{ + dimension: "page", + operator: "includingRegex", + expression: "^https://plausible\\.io/pageA.*$|^https://plausible\\.io/pageB.*$" + } + ] + } + ] + end + + test "transforms simple visit:screen filter" do + filters = %{ + "visit:screen" => {:is, "Desktop"} + } + + {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) + + assert transformed == [%{filters: [%{dimension: "device", expression: "DESKTOP"}]}] + end + + test "transforms member visit:screen filter" do + filters = %{ + "visit:screen" => {:member, ["Mobile", "Tablet"]} + } + + {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) + + assert transformed == [ + %{ + filters: [ + %{dimension: "device", operator: "includingRegex", expression: "Mobile|Tablet"} + ] + } + ] + end + + test "transforms simple visit:country filter to alpha3" do + filters = %{ + "visit:country" => {:is, "EE"} + } + + {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) + + assert transformed == [%{filters: [%{dimension: "country", expression: "EST"}]}] + end + + test "transforms member visit:country filter" do + filters = %{ + "visit:country" => {:member, ["EE", "PL"]} + } + + {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) + + assert transformed == [ + %{ + filters: [ + %{dimension: "country", operator: "includingRegex", expression: "EST|POL"} + ] + } + ] + end + + test "filters can be combined" do + filters = %{ + "visit:entry_page" => {:matches, "*web-analytics*"}, + "visit:screen" => {:is, "Desktop"}, + "visit:country" => {:member, ["EE", "PL"]} + } + + {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) + + assert transformed == [ + %{ + filters: [ + %{dimension: "device", expression: "DESKTOP"}, + %{ + dimension: "page", + operator: "includingRegex", + expression: "^https://plausible\\.io.*web\\-analytics.*$" + }, + %{dimension: "country", operator: "includingRegex", expression: "EST|POL"} + ] + } + ] + end + + test "when unsupported filter is included the whole set becomes invalid" do + filters = %{ + "visit:entry_page" => {:matches, "*web-analytics*"}, + "visit:screen" => {:is, "Desktop"}, + "visit:country" => {:member, ["EE", "PL"]}, + "visit:utm_medium" => {:is, "facebook"} + } + + assert :unsupported_filters = Filters.transform("sc-domain:plausible.io", filters) + end +end diff --git a/test/plausible_web/controllers/api/stats_controller/sources_test.exs b/test/plausible_web/controllers/api/stats_controller/sources_test.exs index 4d4481c4c08e8..3a8de5c97f695 100644 --- a/test/plausible_web/controllers/api/stats_controller/sources_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/sources_test.exs @@ -1607,8 +1607,6 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do end test "gets keywords from Google", %{conn: conn, user: user, site: site} do - insert(:google_auth, user: user, user: user, site: site, property: "sc-domain:example.com") - populate_stats(site, [ build(:pageview, referrer_source: "DuckDuckGo", @@ -1627,10 +1625,7 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do conn = get(conn, "/api/stats/#{site.domain}/referrers/Google?period=day") {:ok, terms} = Plausible.Google.API.Mock.fetch_stats(nil, nil, nil) - assert json_response(conn, 200) == %{ - "total_visitors" => 2, - "search_terms" => terms - } + assert json_response(conn, 200) == %{"search_terms" => terms} end test "works when filter expression is provided for source", %{ diff --git a/tracker/src/plausible.js b/tracker/src/plausible.js index 68c85a8401602..be777c01d772e 100644 --- a/tracker/src/plausible.js +++ b/tracker/src/plausible.js @@ -115,7 +115,7 @@ request.onreadystatechange = function() { if (request.readyState === 4) { - options && options.callback && options.callback() + options && options.callback && options.callback({status: request.status}) } } }