Improve UI/UX of imports view and GA import flow (#4017)

* Add runtime config option for enabled/disabling csv imports and exports

* Use the new option to toggle rendering exports UI

* Disable import buttons when at maximum imports or when option disabled for CSV

* Improve forms for GA import flow

* Add test for maximum imports reached

* Remove "Changed your mind?" prefixing back button

* Hide UA imports in Integrations when `imports_exports` flag is enabled

* Implement `csv_imports_exports` feature flag

* Revert "Add runtime config option for enabled/disabling csv imports and exports"

This reverts commit e30f202dd3.

* Send import notification email only to the user who ran the import

* Improve rendering of disabled button state

* Put import status heroicon in front of import label

lib/plausible_web/components/generic.ex

@@ -71,13 +71,21 @@ defmodule PlausibleWeb.Components.Generic do
   def button_link(assigns) do
     theme_class =
       if assigns.disabled do
-        "bg-gray-400 text-white dark:text-white dark:text-gray-400 dark:bg-gray-700 pointer-events-none cursor-default"
+        "bg-gray-400 text-white dark:text-white dark:text-gray-400 dark:bg-gray-700 cursor-not-allowed"
       else
         @button_themes[assigns.theme]
       end
 
+    onclick =
+      if assigns.disabled do
+        "return false;"
+      else
+        assigns[:onclick]
+      end
+
     assigns =
       assign(assigns,
+        onclick: onclick,
         button_base_class: @button_base_class,
         theme_class: theme_class
       )
@@ -85,6 +93,7 @@ defmodule PlausibleWeb.Components.Generic do
     ~H"""
     <.link
       href={@href}
+      onclick={@onclick}
       class={[
         @button_base_class,
         @theme_class,

lib/plausible_web/live/imports_exports_settings.ex

@@ -48,35 +48,59 @@ defmodule PlausibleWeb.Live.ImportsExportsSettings do
         &(&1.live_status in [SiteImport.pending(), SiteImport.importing()])
       )
 
-    assigns = assign(assigns, :import_in_progress?, import_in_progress?)
+    at_maximum? = length(assigns.site_imports) >= assigns.max_imports
+
+    csv_imports_exports_enabled? = FunWithFlags.enabled?(:csv_imports_exports, for: assigns.site)
+
+    import_warning =
+      cond do
+        import_in_progress? ->
+          "No new imports can be started until the import in progress is completed or cancelled."
+
+        at_maximum? ->
+          "Maximum of #{assigns.max_imports} imports is reached. " <>
+            "Delete or cancel an existing import to start a new one."
+
+        true ->
+          nil
+      end
+
+    assigns =
+      assign(assigns,
+        import_in_progress?: import_in_progress?,
+        at_maximum?: at_maximum?,
+        import_warning: import_warning,
+        csv_imports_exports_enabled?: csv_imports_exports_enabled?
+      )
 
     ~H"""
     <div class="mt-5 flex gap-x-4">
       <.button_link
         class="w-36 h-20"
         theme="bright"
-        disabled={@import_in_progress?}
+        disabled={@import_in_progress? or @at_maximum?}
         href={Plausible.Google.API.import_authorize_url(@site.id, "import", legacy: false)}
       >
         <img src="/images/icon/google_analytics_logo.svg" alt="Google Analytics import" />
       </.button_link>
 
       <.button_link
+        :if={@csv_imports_exports_enabled?}
         class="w-36 h-20"
         theme="bright"
-        disabled={@import_in_progress?}
+        disabled={@import_in_progress? or @at_maximum?}
         href={"/#{URI.encode_www_form(@site.domain)}/settings/import"}
       >
         <img class="h-16" src="/images/icon/csv_logo.svg" alt="New CSV import" />
       </.button_link>
     </div>
 
-    <p :if={@import_in_progress?} class="mt-4 text-red-400 text-sm">
+    <p :if={@import_warning} class="mt-4 text-gray-400 text-sm italic">
-      No new imports can be started until the import in progress is completed or cancelled.
+      <%= @import_warning %>
     </p>
 
     <header class="relative border-b border-gray-200 pb-4">
-      <h3 class="mt-8 text-md leading-6 font-medium text-gray-900 dark:text-gray-100">
+      <h3 class="mt-6 text-md leading-6 font-medium text-gray-900 dark:text-gray-100">
         Existing Imports
       </h3>
       <p class="mt-1 text-sm leading-5 text-gray-500 dark:text-gray-200">
@@ -94,12 +118,6 @@ defmodule PlausibleWeb.Live.ImportsExportsSettings do
       <li :for={entry <- @site_imports} class="py-4 flex items-center justify-between space-x-4">
         <div class="flex flex-col">
           <p class="text-sm leading-5 font-medium text-gray-900 dark:text-gray-100">
-            <%= Plausible.Imported.SiteImport.label(entry.site_import) %>
-            <span :if={entry.live_status == SiteImport.completed()} class="text-xs font-normal">
-              (<%= PlausibleWeb.StatsView.large_number_format(
-                Map.get(@pageview_counts, entry.site_import.id, 0)
-              ) %> page views)
-            </span>
             <Heroicons.clock
               :if={entry.live_status == SiteImport.pending()}
               class="inline-block h-6 w-5 text-indigo-600 dark:text-green-600"
@@ -116,6 +134,12 @@ defmodule PlausibleWeb.Live.ImportsExportsSettings do
               :if={entry.live_status == SiteImport.failed()}
               class="inline-block h-6 w-5 text-indigo-600 dark:text-green-600"
             />
+            <%= Plausible.Imported.SiteImport.label(entry.site_import) %>
+            <span :if={entry.live_status == SiteImport.completed()} class="text-xs font-normal">
+              (<%= PlausibleWeb.StatsView.large_number_format(
+                Map.get(@pageview_counts, entry.site_import.id, 0)
+              ) %> page views)
+            </span>
           </p>
           <p class="text-sm leading-5 text-gray-500 dark:text-gray-200">
             From <%= format_date(entry.site_import.start_date) %> to <%= format_date(

lib/plausible_web/templates/google_analytics/confirm.html.heex

@@ -1,4 +1,4 @@
-<%= form_for @conn, Routes.google_analytics_path(@conn, :import, @site.domain), [class: "max-w-md w-full mx-auto bg-white dark:bg-gray-800 shadow-md rounded px-8 pt-6 pb-8 mb-4 mt-8"], fn f -> %>
+<%= form_for @conn, Routes.google_analytics_path(@conn, :import, @site.domain), [onsubmit: "confirmButton.disabled = true; return true;", class: "max-w-md w-full mx-auto bg-white dark:bg-gray-800 shadow-md rounded px-8 pt-6 pb-8 mb-4 mt-8"], fn f -> %>
   <h2 class="text-xl font-black dark:text-gray-100">Import from Google Analytics</h2>
 
   <%= hidden_input(f, :access_token, value: @access_token) %>
@@ -48,5 +48,33 @@
     </div>
   </div>
 
-  <%= submit("Confirm import", class: "button mt-6") %>
+  <div class="mt-6 flex flex-col-reverse sm:flex-row justify-between items-center">
+    <p class="text-sm mt-4 sm:mt-0 dark:text-gray-100">
+      <a
+        href={
+          Routes.google_analytics_path(@conn, :property_or_view, @site.domain,
+            property_or_view: @selected_property_or_view,
+            access_token: @access_token,
+            refresh_token: @refresh_token,
+            expires_at: @expires_at,
+            legacy: @legacy
+          )
+        }
+        class="underline text-indigo-600"
+      >
+        Go back
+      </a>
+    </p>
+
+    <%= submit(name: "confirmButton", class: "button sm:w-auto w-full [&>span.label-enabled]:block [&>span.label-disabled]:hidden [&[disabled]>span.label-enabled]:hidden [&[disabled]>span.label-disabled]:block") do %>
+      <span class="label-enabled pointer-events-none">
+        Confirm import
+      </span>
+
+      <span class="label-disabled">
+        <PlausibleWeb.Components.Generic.spinner class="inline-block h-5 w-5 mr-2 text-white dark:text-gray-400" />
+        Starting import...
+      </span>
+    <% end %>
+  </div>
 <% end %>

lib/plausible_web/templates/google_analytics/property_or_view_form.html.heex

@@ -1,4 +1,4 @@
-<%= form_for @conn, Routes.google_analytics_path(@conn, :property_or_view, @site.domain), [class: "max-w-md w-full mx-auto bg-white dark:bg-gray-800 shadow-md rounded px-8 pt-6 pb-8 mb-4 mt-8"], fn f -> %>
+<%= form_for @conn, Routes.google_analytics_path(@conn, :property_or_view, @site.domain), [onsubmit: "continueButton.disabled = true; return true;", class: "max-w-md w-full mx-auto bg-white dark:bg-gray-800 shadow-md rounded px-8 pt-6 pb-8 mb-4 mt-8"], fn f -> %>
   <h2 class="text-xl font-black dark:text-gray-100">Import from Google Analytics</h2>
 
   <%= hidden_input(f, :access_token, value: @access_token) %>
@@ -19,5 +19,25 @@
     <%= styled_error(@conn.assigns[:selected_property_or_view_error]) %>
   </div>
 
-  <%= submit("Continue ->", class: "button mt-6") %>
+  <div class="mt-6 flex flex-col-reverse sm:flex-row justify-between items-center">
+    <p class="text-sm mt-4 sm:mt-0 dark:text-gray-100">
+      <a
+        href={Routes.site_path(@conn, :settings_imports_exports, @site.domain)}
+        class="underline text-indigo-600"
+      >
+        Go back
+      </a>
+    </p>
+
+    <%= submit(name: "continueButton", class: "button sm:w-auto w-full [&>span.label-enabled]:block [&>span.label-disabled]:hidden [&[disabled]>span.label-enabled]:hidden [&[disabled]>span.label-disabled]:block") do %>
+      <span class="label-enabled pointer-events-none">
+        Continue ->
+      </span>
+
+      <span class="label-disabled">
+        <PlausibleWeb.Components.Generic.spinner class="inline-block h-5 w-5 mr-2 text-white dark:text-gray-400" />
+        Checking...
+      </span>
+    <% end %>
+  </div>
 <% end %>

lib/plausible_web/templates/google_analytics/user_metric_form.html.heex

@@ -32,17 +32,36 @@
     </p>
   </div>
 
-  <%= link("Continue ->",
+  <div class="mt-6 flex flex-col-reverse sm:flex-row justify-between items-center">
-    to:
+    <p class="text-sm mt-4 sm:mt-0 dark:text-gray-100">
-      Routes.google_analytics_path(@conn, :confirm, @site.domain,
+      <a
-        property_or_view: @property_or_view,
+        href={
-        access_token: @access_token,
+          Routes.google_analytics_path(@conn, :property_or_view, @site.domain,
-        refresh_token: @refresh_token,
+            property_or_view: @property_or_view,
-        expires_at: @expires_at,
+            access_token: @access_token,
-        start_date: @start_date,
+            refresh_token: @refresh_token,
-        end_date: @end_date,
+            expires_at: @expires_at,
-        legacy: @legacy
+            legacy: @legacy
-      ),
+          )
-    class: "button mt-6"
+        }
-  ) %>
+        class="underline text-indigo-600"
+      >
+        Go back
+      </a>
+    </p>
+
+    <%= link("Continue ->",
+      to:
+        Routes.google_analytics_path(@conn, :confirm, @site.domain,
+          property_or_view: @property_or_view,
+          access_token: @access_token,
+          refresh_token: @refresh_token,
+          expires_at: @expires_at,
+          start_date: @start_date,
+          end_date: @end_date,
+          legacy: @legacy
+        ),
+      class: "button sm:w-auto w-full"
+    ) %>
+  </div>
 </div>

lib/plausible_web/templates/site/settings_imports_exports.html.heex

@@ -15,21 +15,23 @@
   ) %>
 </div>
 
-<div class="shadow bg-white dark:bg-gray-800 dark:text-gray-200 sm:rounded-md sm:overflow-hidden py-6 px-4 sm:p-6">
+<%= if FunWithFlags.enabled?(:csv_imports_exports, for: @site) do %>
-  <header class="relative border-b border-gray-200 pb-4 mb-5">
+  <div class="shadow bg-white dark:bg-gray-800 dark:text-gray-200 sm:rounded-md sm:overflow-hidden py-6 px-4 sm:p-6">
-    <h2 class="text-lg leading-6 font-medium text-gray-900 dark:text-gray-100">
+    <header class="relative border-b border-gray-200 pb-4 mb-5">
-      Export Data
+      <h2 class="text-lg leading-6 font-medium text-gray-900 dark:text-gray-100">
-    </h2>
+        Export Data
-    <p class="mt-1 text-sm leading-5 text-gray-500 dark:text-gray-200">
+      </h2>
-      Export all your data into CSV format.
+      <p class="mt-1 text-sm leading-5 text-gray-500 dark:text-gray-200">
-    </p>
+        Export all your data into CSV format.
-  </header>
+      </p>
+    </header>
 
-  <%= live_render(@conn, PlausibleWeb.Live.CSVExport,
+    <%= live_render(@conn, PlausibleWeb.Live.CSVExport,
-    session: %{
+      session: %{
-      "site_id" => @site.id,
+        "site_id" => @site.id,
-      "email_to" => @current_user.email,
+        "email_to" => @current_user.email,
-      "storage" => on_full_build(do: "s3", else: "local")
+        "storage" => on_full_build(do: "s3", else: "local")
-    }
+      }
-  ) %>
+    ) %>
-</div>
+  </div>
+<% end %>

lib/plausible_web/templates/site/settings_integrations.html.heex

@@ -2,10 +2,13 @@
   site={@site}
   search_console_domains={@search_console_domains}
 />
-<PlausibleWeb.Components.Settings.settings_google_import
+
-  site={@site}
+<%= if not FunWithFlags.enabled?(:imports_exports, for: @site) do %>
-  imported_pageviews={@imported_pageviews}
+  <PlausibleWeb.Components.Settings.settings_google_import
-/>
+    site={@site}
+    imported_pageviews={@imported_pageviews}
+  />
+<% end %>
 
 <section
   :if={@has_plugins_tokens? || @conn.query_params["new_token"]}

lib/workers/import_analytics.ex

@@ -54,14 +54,10 @@ defmodule Plausible.Workers.ImportAnalytics do
   end
 
   def import_complete(site_import) do
-    site_import = Repo.preload(site_import, site: [memberships: :user])
+    site_import = Repo.preload(site_import, [:site, :imported_by])
 
-    Enum.each(site_import.site.memberships, fn membership ->
+    PlausibleWeb.Email.import_success(site_import, site_import.imported_by)
-      if membership.role in [:owner, :admin] do
+    |> Plausible.Mailer.send()
-        PlausibleWeb.Email.import_success(site_import, membership.user)
-        |> Plausible.Mailer.send()
-      end
-    end)
 
     Plausible.Sites.clear_stats_start_date!(site_import.site)
 
@@ -84,15 +80,11 @@ defmodule Plausible.Workers.ImportAnalytics do
     site_import =
       site_import
       |> import_api.mark_failed()
-      |> Repo.preload(site: [memberships: :user])
+      |> Repo.preload([:site, :imported_by])
 
     Importer.notify(site_import, :fail)
 
-    Enum.each(site_import.site.memberships, fn membership ->
+    PlausibleWeb.Email.import_failure(site_import, site_import.imported_by)
-      if membership.role in [:owner, :admin] do
+    |> Plausible.Mailer.send()
-        PlausibleWeb.Email.import_failure(site_import, membership.user)
-        |> Plausible.Mailer.send()
-      end
-    end)
   end
 end

priv/repo/seeds.exs

@@ -11,6 +11,7 @@
 # and so on) as they will fail if something goes wrong.
 
 FunWithFlags.enable(:imports_exports)
+FunWithFlags.enable(:csv_imports_exports)
 FunWithFlags.enable(:hostname_filter)
 FunWithFlags.enable(:shield_hostnames)
 

test/plausible_web/controllers/site_controller_test.exs

@@ -681,6 +681,18 @@ defmodule PlausibleWeb.SiteControllerTest do
       assert resp =~ "(98 page views)"
     end
 
+    test "disables import buttons when imports are at maximum", %{conn: conn, site: site} do
+      insert_list(Plausible.Imported.max_complete_imports(), :site_import,
+        site: site,
+        status: SiteImport.completed()
+      )
+
+      conn = get(conn, "/#{site.domain}/settings/imports-exports")
+
+      assert html_response(conn, 200) =~
+               "Maximum of #{Plausible.Imported.max_complete_imports()} imports is reached."
+    end
+
     test "disables import buttons when there's import in progress", %{conn: conn, site: site} do
       _site_import1 = insert(:site_import, site: site, status: SiteImport.completed())
       _site_import2 = insert(:site_import, site: site, status: SiteImport.importing())

test/test_helper.exs

@@ -6,6 +6,7 @@ end
 Mox.defmock(Plausible.HTTPClient.Mock, for: Plausible.HTTPClient.Interface)
 Application.ensure_all_started(:double)
 FunWithFlags.enable(:imports_exports)
+FunWithFlags.enable(:csv_imports_exports)
 
 # Temporary flag to test `experimental_reduced_joins` flag on all tests.
 if System.get_env("TEST_EXPERIMENTAL_REDUCED_JOINS") == "1" do

test/workers/import_analytics_test.exs

@@ -80,6 +80,34 @@ defmodule Plausible.Workers.ImportAnalyticsTest do
       )
     end
 
+    test "send email after successful import only to the user who ran the import", %{
+      import_opts: import_opts
+    } do
+      owner = insert(:user, trial_expiry_date: Timex.today() |> Timex.shift(days: 1))
+      site = insert(:site, members: [owner])
+
+      importing_user = insert(:user)
+
+      insert(:site_membership, site: site, user: importing_user, role: :admin)
+
+      {:ok, job} = Plausible.Imported.NoopImporter.new_import(site, importing_user, import_opts)
+
+      assert :ok =
+               job
+               |> Repo.reload!()
+               |> ImportAnalytics.perform()
+
+      assert_email_delivered_with(
+        to: [importing_user],
+        subject: "Noop data imported for #{site.domain}"
+      )
+
+      refute_email_delivered_with(
+        to: [owner],
+        subject: "Noop data imported for #{site.domain}"
+      )
+    end
+
     test "updates site import record after failed import", %{import_opts: import_opts} do
       user = insert(:user, trial_expiry_date: Timex.today() |> Timex.shift(days: 1))
       site = insert(:site, members: [user])
@@ -137,6 +165,35 @@ defmodule Plausible.Workers.ImportAnalyticsTest do
         subject: "Noop import failed for #{site.domain}"
       )
     end
+
+    test "sends email after failed import only to the user who ran the import", %{
+      import_opts: import_opts
+    } do
+      owner = insert(:user, trial_expiry_date: Timex.today() |> Timex.shift(days: 1))
+      site = insert(:site, members: [owner])
+      import_opts = Keyword.put(import_opts, :error, true)
+
+      importing_user = insert(:user)
+
+      insert(:site_membership, site: site, user: importing_user, role: :admin)
+
+      {:ok, job} = Plausible.Imported.NoopImporter.new_import(site, importing_user, import_opts)
+
+      assert {:discard, _} =
+               job
+               |> Repo.reload!()
+               |> ImportAnalytics.perform()
+
+      assert_email_delivered_with(
+        to: [importing_user],
+        subject: "Noop import failed for #{site.domain}"
+      )
+
+      refute_email_delivered_with(
+        to: [owner],
+        subject: "Noop import failed for #{site.domain}"
+      )
+    end
   end
 
   describe "perform/1 notifications" do