diff --git a/assets/js/liveview/suggestions_dropdown.js b/assets/js/liveview/suggestions_dropdown.js index e18999235..30ea783c0 100644 --- a/assets/js/liveview/suggestions_dropdown.js +++ b/assets/js/liveview/suggestions_dropdown.js @@ -4,15 +4,54 @@ let suggestionsDropdown = function(id) { return { isOpen: false, id: id, - open() { this.isOpen = true }, - close() { this.isOpen = false }, - focus: 0, + focus: null, setFocus(f) { this.focus = f; }, + initFocus() { + if (this.focus === null) { + this.setFocus(this.leastFocusableIndex()) + } + }, + open() { + this.initFocus() + this.isOpen = true + }, + suggestionsCount() { + return this.$refs.suggestions?.querySelectorAll('li').length + }, + hasCreatableOption() { + return this.$refs.suggestions?.querySelector('li').classList.contains("creatable") + }, + leastFocusableIndex() { + if (this.suggestionsCount() === 0) { + return 0 + } + return this.hasCreatableOption() ? 0 : 1 + }, + maxFocusableIndex() { + return this.hasCreatableOption() ? this.suggestionsCount() - 1 : this.suggestionsCount() + }, + nextFocusableIndex() { + const currentFocus = this.focus + return currentFocus + 1 > this.maxFocusableIndex() ? this.leastFocusableIndex() : currentFocus + 1 + }, + prevFocusableIndex() { + const currentFocus = this.focus + return currentFocus - 1 >= this.leastFocusableIndex() ? currentFocus - 1 : this.maxFocusableIndex() + }, + close(e) { + // Pressing Escape should not propagate to window, + // so we'll only close the suggestions pop-up + if (this.isOpen && e.key === "Escape") { + e.stopPropagation() + } + this.isOpen = false + }, select() { this.$refs[`dropdown-${this.id}-option-${this.focus}`]?.click() - this.focusPrev() + this.close() + document.getElementById(this.id).blur() }, scrollTo(idx) { this.$refs[`dropdown-${this.id}-option-${idx}`]?.scrollIntoView( @@ -20,23 +59,21 @@ let suggestionsDropdown = function(id) { ) }, focusNext() { - const nextIndex = this.focus + 1 - const total = this.$refs.suggestions?.childElementCount ?? 0 + const nextIndex = this.nextFocusableIndex() if (!this.isOpen) this.open() - if (nextIndex < total) { - this.setFocus(nextIndex) - this.scrollTo(nextIndex); - } + this.setFocus(nextIndex) + this.scrollTo(nextIndex) }, focusPrev() { - const nextIndex = this.focus - 1 - if (this.isOpen && nextIndex >= 0) { - this.setFocus(nextIndex) - this.scrollTo(nextIndex) - } - }, + const prevIndex = this.prevFocusableIndex() + + if (!this.isOpen) this.open() + + this.setFocus(prevIndex) + this.scrollTo(prevIndex) + } } } diff --git a/lib/plausible_web/controllers/site_controller.ex b/lib/plausible_web/controllers/site_controller.ex index f9aaa993f..24c37ebbc 100644 --- a/lib/plausible_web/controllers/site_controller.ex +++ b/lib/plausible_web/controllers/site_controller.ex @@ -1,7 +1,7 @@ defmodule PlausibleWeb.SiteController do use PlausibleWeb, :controller use Plausible.Repo - alias Plausible.{Sites, Goals} + alias Plausible.Sites plug PlausibleWeb.RequireAccountPlug @@ -216,32 +216,26 @@ defmodule PlausibleWeb.SiteController do def settings_goals(conn, _params) do site = conn.assigns[:site] |> Repo.preload(:custom_domain) - goals = Goals.for_site(site, preload_funnels?: true) conn |> assign(:skip_plausible_tracking, true) |> render("settings_goals.html", site: site, - goals: goals, connect_live_socket: true, layout: {PlausibleWeb.LayoutView, "site_settings.html"} ) end def settings_funnels(conn, _params) do - if Plausible.Funnels.enabled_for?(conn.assigns[:current_user]) do - site = conn.assigns[:site] |> Repo.preload(:custom_domain) + site = conn.assigns[:site] |> Repo.preload(:custom_domain) - conn - |> assign(:skip_plausible_tracking, true) - |> render("settings_funnels.html", - site: site, - connect_live_socket: true, - layout: {PlausibleWeb.LayoutView, "site_settings.html"} - ) - else - conn |> Plug.Conn.put_status(401) |> Plug.Conn.halt() - end + conn + |> assign(:skip_plausible_tracking, true) + |> render("settings_funnels.html", + site: site, + connect_live_socket: true, + layout: {PlausibleWeb.LayoutView, "site_settings.html"} + ) end def settings_props(conn, _params) do diff --git a/lib/plausible_web/controllers/stats_controller.ex b/lib/plausible_web/controllers/stats_controller.ex index 205ee6bb8..dc21f7cd4 100644 --- a/lib/plausible_web/controllers/stats_controller.ex +++ b/lib/plausible_web/controllers/stats_controller.ex @@ -335,10 +335,8 @@ defmodule PlausibleWeb.StatsController do defp shared_link_cookie_name(slug), do: "shared-link-" <> slug - defp get_flags(user) do - %{ - funnels: Plausible.Funnels.enabled_for?(user) - } + defp get_flags(_user) do + %{} end defp is_dbip() do diff --git a/lib/plausible_web/live/components/combo_box.ex b/lib/plausible_web/live/components/combo_box.ex index 7b98a7522..4bd85733b 100644 --- a/lib/plausible_web/live/components/combo_box.ex +++ b/lib/plausible_web/live/components/combo_box.ex @@ -17,16 +17,24 @@ defmodule PlausibleWeb.Live.Components.ComboBox do Any function can be supplied via `suggest_fun` attribute - see the provided `ComboBox.StaticSearch`. - In case the `suggest_fun` runs an operation that could be deferred, - the `async=true` attr calls it in a background Task and updates the - suggestions asynchronously. + In most cases the `suggest_fun` runs an operation that could be deferred, + so by default, the `async={true}` attr calls it in a background Task + and updates the suggestions asynchronously. This way, you can render + the component without having to wait for suggestions to load. - Similarly, the initial `options` don't have to be provided up-front - if e.g. querying the database for suggestions at initial render is - undesirable. In such case, lack of `options` attr value combined - with `async=true` will call `suggest_fun.("", [])` asynchronously - - that special clause can be used to provide the initial set - of suggestions updated right after the initial render. + If you explicitly need to make the operation sychronous, you may + pass `async={false}` option. + + If your initial `options` are not provided up-front at initial render, + lack of `options` attr value combined with `async=true` calls the + `suggest_fun.("", [])` asynchronously - that special clause can be used + to provide the initial set of suggestions updated right after the initial render. + + To simplify integration testing, suggestions load up synchronously during + tests. This lets you skip waiting for suggestions messages + to arrive. The asynchronous behaviour alone is already tested in + ComboBox own test suite, so there is no need for additional + verification. """ use Phoenix.LiveComponent alias Phoenix.LiveView.JS @@ -60,7 +68,7 @@ defmodule PlausibleWeb.Live.Components.ComboBox do attr(:required, :boolean, default: false) attr(:creatable, :boolean, default: false) attr(:errors, :list, default: []) - attr(:async, :boolean, default: false) + attr(:async, :boolean, default: Mix.env() != :test) def render(assigns) do assigns = @@ -195,6 +203,12 @@ defmodule PlausibleWeb.Live.Components.ComboBox do > No matches found. Try searching for something different. +
+ Create an item by typing. +
""" end @@ -211,7 +225,10 @@ defmodule PlausibleWeb.Live.Components.ComboBox do ~H"""
  • -
  • - Max results reached. Refine your search by typing in goal name. -
  • +
    + Max results reached. Refine your search by typing. +
    """ end diff --git a/lib/plausible_web/live/funnel_settings.ex b/lib/plausible_web/live/funnel_settings.ex index 245b82a16..f60cd27b7 100644 --- a/lib/plausible_web/live/funnel_settings.ex +++ b/lib/plausible_web/live/funnel_settings.ex @@ -11,22 +11,25 @@ defmodule PlausibleWeb.Live.FunnelSettings do def mount( _params, - %{"site_id" => _site_id, "domain" => domain, "current_user_id" => user_id}, + %{"site_id" => site_id, "domain" => domain, "current_user_id" => user_id}, socket ) do - true = Plausible.Funnels.enabled_for?("user:#{user_id}") - - site = Sites.get_for_user!(user_id, domain, [:owner, :admin, :super_admin]) - - funnels = Funnels.list(site) - goal_count = Goals.count(site) + socket = + socket + |> assign_new(:site, fn -> + Sites.get_for_user!(user_id, domain, [:owner, :admin, :super_admin]) + end) + |> assign_new(:funnels, fn %{site: site} -> + Funnels.list(site) + end) + |> assign_new(:goal_count, fn %{site: site} -> + Goals.count(site) + end) {:ok, assign(socket, - site_id: site.id, - domain: site.domain, - funnels: funnels, - goal_count: goal_count, + site_id: site_id, + domain: domain, add_funnel?: false, current_user_id: user_id )} diff --git a/lib/plausible_web/live/goal_settings.ex b/lib/plausible_web/live/goal_settings.ex index 1a7279a3a..63e9264a0 100644 --- a/lib/plausible_web/live/goal_settings.ex +++ b/lib/plausible_web/live/goal_settings.ex @@ -11,19 +11,23 @@ defmodule PlausibleWeb.Live.GoalSettings do def mount( _params, - %{"site_id" => _site_id, "domain" => domain, "current_user_id" => user_id}, + %{"site_id" => site_id, "domain" => domain, "current_user_id" => user_id}, socket ) do - site = Sites.get_for_user!(user_id, domain, [:owner, :admin, :super_admin]) - - goals = Goals.for_site(site, preload_funnels?: true) + socket = + socket + |> assign_new(:site, fn -> + Sites.get_for_user!(user_id, domain, [:owner, :admin, :super_admin]) + end) + |> assign_new(:all_goals, fn %{site: site} -> + Goals.for_site(site, preload_funnels?: true) + end) {:ok, assign(socket, - site_id: site.id, - domain: site.domain, - all_goals: goals, - displayed_goals: goals, + site_id: site_id, + domain: domain, + displayed_goals: socket.assigns.all_goals, add_goal?: false, current_user_id: user_id, filter_text: "" diff --git a/lib/plausible_web/live/goal_settings/form.ex b/lib/plausible_web/live/goal_settings/form.ex index 671f33b16..d3ba08706 100644 --- a/lib/plausible_web/live/goal_settings/form.ex +++ b/lib/plausible_web/live/goal_settings/form.ex @@ -86,7 +86,6 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do ]} module={ComboBox} suggest_fun={fn input, options -> suggest_page_paths(input, options, @site) end} - async={true} creatable /> @@ -185,7 +184,6 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do ComboBox.StaticSearch.suggest(input, options, weight_threshold: 0.8) end } - async={true} /> diff --git a/lib/plausible_web/live/props_settings.ex b/lib/plausible_web/live/props_settings.ex index 78d352ea2..b0049d73e 100644 --- a/lib/plausible_web/live/props_settings.ex +++ b/lib/plausible_web/live/props_settings.ex @@ -9,27 +9,28 @@ defmodule PlausibleWeb.Live.PropsSettings do def mount( _params, - %{"site_id" => _site_id, "domain" => domain, "current_user_id" => user_id}, + %{"site_id" => site_id, "domain" => domain, "current_user_id" => user_id}, socket ) do - site = - if Plausible.Auth.is_super_admin?(user_id) do - Plausible.Sites.get_by_domain(domain) - else - Plausible.Sites.get_for_user!(user_id, domain, [:owner, :admin]) - end - - suggestions = - site - |> Plausible.Props.suggest_keys_to_allow() - |> Enum.map(&{&1, &1}) + socket = + socket + |> assign_new(:site, fn -> + Plausible.Sites.get_for_user!(user_id, domain, [:owner, :admin, :super_admin]) + end) + |> assign_new(:all_props, fn %{site: site} -> + site.allowed_event_props || [] + end) + |> assign_new(:displayed_props, fn %{all_props: props} -> + props + end) {:ok, assign(socket, - site: site, + site_id: site_id, + domain: domain, current_user_id: user_id, - form: new_form(site), - suggestions: suggestions + add_prop?: false, + filter_text: "" )} end @@ -37,90 +38,27 @@ defmodule PlausibleWeb.Live.PropsSettings do ~H"""
    <.live_component id="embedded_liveview_flash" module={PlausibleWeb.Live.Flash} flash={@flash} /> + <%= if @add_prop? do %> + <%= live_render( + @socket, + PlausibleWeb.Live.PropsSettings.Form, + id: "props-form", + session: %{ + "current_user_id" => @current_user_id, + "domain" => @domain, + "site_id" => @site_id, + "rendered_by" => self() + } + ) %> + <% end %> -

    - Configured properties -

    - -

    - In order for the properties to show up on your dashboard, you need to - explicitly add them below first -

    - - <.form :let={f} for={@form} id="props-form" phx-submit="allow" class="mt-5"> -
    - <.live_component - id={:prop_input} - submit_name="prop" - class="flex-1" - module={ComboBox} - suggest_fun={&ComboBox.StaticSearch.suggest/2} - options={@suggestions} - required - creatable - /> - - -
    - -
    0} id="prop-errors" role="alert"> - <%= PlausibleWeb.ErrorHelpers.error_tag(f, :allowed_event_props) %> -
    - - - - -
    - <%= if is_list(@site.allowed_event_props) && length(@site.allowed_event_props) > 0 do %> - - <% else %> -

    - No properties configured for this site yet -

    - <% end %> -
    + <.live_component + module={PlausibleWeb.Live.PropsSettings.List} + id="props-list" + props={@displayed_props} + domain={@domain} + filter_text={@filter_text} + />
    """ end @@ -133,8 +71,7 @@ defmodule PlausibleWeb.Live.PropsSettings do {:noreply, assign(socket, site: site, - form: new_form(site), - suggestions: rebuild_suggestions(socket.assigns.suggestions, site.allowed_event_props) + form: new_form(site) )} {:error, changeset} -> @@ -145,9 +82,38 @@ defmodule PlausibleWeb.Live.PropsSettings do end end - def handle_event("disallow", %{"prop" => prop}, socket) do + def handle_event("add-prop", _value, socket) do + {:noreply, assign(socket, add_prop?: true)} + end + + def handle_event("filter", %{"filter-text" => filter_text}, socket) do + new_list = + ComboBox.StaticSearch.suggest( + filter_text, + socket.assigns.all_props + ) + + {:noreply, assign(socket, displayed_props: new_list, filter_text: filter_text)} + end + + def handle_event("reset-filter-text", _params, socket) do + {:noreply, assign(socket, filter_text: "", displayed_props: socket.assigns.all_props)} + end + + def handle_event("disallow-prop", %{"prop" => prop}, socket) do {:ok, site} = Plausible.Props.disallow(socket.assigns.site, prop) - {:noreply, assign(socket, site: site)} + + socket = + socket + |> put_flash(:success, "Property removed successfully") + |> assign( + all_props: Enum.reject(socket.assigns.all_props, &(&1 == prop)), + displayed_props: Enum.reject(socket.assigns.displayed_props, &(&1 == prop)), + site: site + ) + + Process.send_after(self(), :clear_flash, 5000) + {:noreply, socket} end def handle_event("allow-existing-props", _params, socket) do @@ -155,21 +121,52 @@ defmodule PlausibleWeb.Live.PropsSettings do {:noreply, assign(socket, - site: site, - suggestions: rebuild_suggestions(socket.assigns.suggestions, site.allowed_event_props) + site: site )} end - defp rebuild_suggestions(suggestions, allowed_event_props) do - allowed_event_props = allowed_event_props || [] + def handle_info(:cancel_add_prop, socket) do + {:noreply, assign(socket, add_prop?: false)} + end - suggestions = - for {suggestion, _} <- suggestions, - suggestion not in allowed_event_props, - do: {suggestion, suggestion} + def handle_info({:props_allowed, props}, socket) when is_list(props) do + socket = + socket + |> assign( + add_prop?: false, + filter_text: "", + all_props: props, + displayed_props: props, + site: %{socket.assigns.site | allowed_event_props: props} + ) + |> put_flash(:success, "Properties added successfully") - send_update(ComboBox, id: :prop_input, suggestions: suggestions) - suggestions + {:noreply, socket} + end + + def handle_info( + {:prop_allowed, prop}, + %{assigns: %{site: site}} = socket + ) + when is_binary(prop) do + allowed_event_props = [prop | site.allowed_event_props] + + socket = + socket + |> assign( + add_prop?: false, + filter_text: "", + all_props: allowed_event_props, + displayed_props: allowed_event_props, + site: %{site | allowed_event_props: allowed_event_props} + ) + |> put_flash(:success, "Property added successfully") + + {:noreply, socket} + end + + def handle_info(:clear_flash, socket) do + {:noreply, clear_flash(socket)} end defp new_form(site) do diff --git a/lib/plausible_web/live/props_settings/form.ex b/lib/plausible_web/live/props_settings/form.ex new file mode 100644 index 000000000..f38cae2ab --- /dev/null +++ b/lib/plausible_web/live/props_settings/form.ex @@ -0,0 +1,159 @@ +defmodule PlausibleWeb.Live.PropsSettings.Form do + @moduledoc """ + Live view for the custom props creation form + """ + use Phoenix.LiveView + import PlausibleWeb.Live.Components.Form + alias PlausibleWeb.Live.Components.ComboBox + + def mount( + _params, + %{ + "site_id" => _site_id, + "current_user_id" => user_id, + "domain" => domain, + "rendered_by" => pid + }, + socket + ) do + site = Plausible.Sites.get_for_user!(user_id, domain, [:owner, :admin, :super_admin]) + + form = new_form(site) + + {:ok, + assign(socket, + form: form, + domain: domain, + rendered_by: pid, + prop_key_options_count: 0, + site: site + )} + end + + def render(assigns) do + ~H""" +
    +
    +
    +
    + <.form + :let={f} + for={@form} + 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" + phx-submit="allow-prop" + phx-click-away="cancel-allow-prop" + > +

    Add Property for <%= @domain %>

    + +
    + <.label for="prop_input"> + Property + + + <.live_component + id="prop_input" + submit_name="prop" + class={[ + "py-2" + ]} + module={ComboBox} + suggest_fun={ + pid = self() + + fn + "", [] -> + options = + @site + |> Plausible.Props.suggest_keys_to_allow() + |> Enum.map(&{&1, &1}) + + send(pid, {:update_prop_key_options_count, Enum.count(options)}) + + options + + input, options -> + ComboBox.StaticSearch.suggest(input, options) + end + } + creatable + /> + + <.error :for={{msg, opts} <- f[:allowed_event_props].errors}> + <%= Enum.reduce(opts, msg, fn {key, value}, acc -> + String.replace(acc, "%{#{key}}", fn _ -> to_string(value) end) + end) %> + +
    + +
    + +
    + + + +
    +
    + """ + end + + def handle_info({:update_prop_key_options_count, count}, socket) do + {:noreply, assign(socket, prop_key_options_count: count)} + end + + def handle_info({:selection_made, %{submit_value: _prop}}, socket) do + {:noreply, socket} + end + + def handle_event("allow-prop", %{"prop" => prop}, socket) do + case Plausible.Props.allow(socket.assigns.site, prop) do + {:ok, site} -> + send(socket.assigns.rendered_by, {:prop_allowed, prop}) + + {:noreply, + assign(socket, + site: site, + form: new_form(site) + )} + + {:error, changeset} -> + {:noreply, + assign(socket, + form: to_form(Map.put(changeset, :action, :validate)) + )} + end + end + + def handle_event("allow-existing-props", _params, socket) do + {:ok, site} = Plausible.Props.allow_existing_props(socket.assigns.site) + send(socket.assigns.rendered_by, {:props_allowed, site.allowed_event_props}) + + {:noreply, + assign(socket, + site: site, + form: new_form(site), + prop_key_options_count: 0 + )} + end + + def handle_event("cancel-allow-prop", _value, socket) do + send(socket.assigns.rendered_by, :cancel_add_prop) + {:noreply, socket} + end + + defp new_form(site) do + to_form(Plausible.Props.allow_changeset(site, [])) + end +end diff --git a/lib/plausible_web/live/props_settings/list.ex b/lib/plausible_web/live/props_settings/list.ex new file mode 100644 index 000000000..456635bc7 --- /dev/null +++ b/lib/plausible_web/live/props_settings/list.ex @@ -0,0 +1,100 @@ +defmodule PlausibleWeb.Live.PropsSettings.List do + @moduledoc """ + Phoenix LiveComponent module that renders a list of custom properties + """ + use Phoenix.LiveComponent + use Phoenix.HTML + + use Plausible.Funnel + + attr(:props, :list, required: true) + attr(:domain, :string, required: true) + attr(:filter_text, :string) + + def render(assigns) do + ~H""" +
    +
    +
    +
    +
    +
    + +
    + +
    + + +
    +
    +
    + +
    +
    + <%= if is_list(@props) && length(@props) > 0 do %> + + <% else %> +

    + + No properties found for this site. Please refine or + + reset your search. + + + + No properties configured for this site. + +

    + <% end %> +
    + """ + end + + defp delete_confirmation_text(prop) do + """ + Are you sure you want to remove the following property: + + #{prop} + + This will just affect the UI, all of your analytics data will stay intact. + """ + end +end diff --git a/lib/plausible_web/templates/site/settings_props.html.heex b/lib/plausible_web/templates/site/settings_props.html.heex index 7b3f7b941..c373b4b09 100644 --- a/lib/plausible_web/templates/site/settings_props.html.heex +++ b/lib/plausible_web/templates/site/settings_props.html.heex @@ -18,10 +18,14 @@ Custom Properties -

    - Attach custom properties when sending a pageview or an event to - create custom metrics -

    +

    + Attach Custom Properties when sending a Pageview or an Event to + create custom metrics. +

    +

    + In order for the properties to show up on your dashboard, you need to + explicitly add them below first. +

    <.link @@ -48,7 +52,7 @@ <%= live_render(@conn, PlausibleWeb.Live.PropsSettings, diff --git a/lib/plausible_web/views/layout_view.ex b/lib/plausible_web/views/layout_view.ex index f67c77896..7d1361583 100644 --- a/lib/plausible_web/views/layout_view.ex +++ b/lib/plausible_web/views/layout_view.ex @@ -27,9 +27,7 @@ defmodule PlausibleWeb.LayoutView do [key: "People", value: "people"], [key: "Visibility", value: "visibility"], [key: "Goals", value: "goals"], - if Plausible.Funnels.enabled_for?(conn.assigns[:current_user]) do - [key: "Funnels", value: "funnels"] - end, + [key: "Funnels", value: "funnels"], [key: "Custom Properties", value: "properties"], [key: "Search Console", value: "search-console"], [key: "Email Reports", value: "email-reports"], diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index f36587186..98c6b8ea6 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -12,8 +12,6 @@ user = Plausible.Factory.insert(:user, email: "user@plausible.test", password: "plausible") -FunWithFlags.enable(:funnels) - native_stats_range = Date.range( Date.add(Date.utc_today(), -720), @@ -207,9 +205,12 @@ native_stats_range operating_system: Enum.random(["Windows", "macOS", "Linux"]), operating_system_version: to_string(Enum.random(0..15)), user_id: Enum.random(1..1200), - "meta.key": ["url"], + "meta.key": ["url", "logged_in", "is_customer", "amount"], "meta.value": [ - Enum.random(long_random_urls) + Enum.random(long_random_urls), + Enum.random(["true", "false"]), + Enum.random(["true", "false"]), + to_string(Enum.random(1..9000)) ] ] |> Keyword.merge(geolocation) diff --git a/test/plausible_web/live/components/combo_box_test.exs b/test/plausible_web/live/components/combo_box_test.exs index 1220ee50f..1cc76fe6c 100644 --- a/test/plausible_web/live/components/combo_box_test.exs +++ b/test/plausible_web/live/components/combo_box_test.exs @@ -57,13 +57,6 @@ defmodule PlausibleWeb.Live.Components.ComboBoxTest do assert text_of_attr(li2, "x-bind:class") =~ "focus === 2" end - test "Alpine.js: component refers to window.suggestionsDropdown" do - assert new_options(2) - |> render_sample_component() - |> find("div#input-picker-main-test-component") - |> text_of_attr("x-data") =~ "window.suggestionsDropdown('test-component')" - end - test "Alpine.js: component sets up keyboard navigation" do main = new_options(2) @@ -123,6 +116,15 @@ defmodule PlausibleWeb.Live.Components.ComboBoxTest do "No matches found. Try searching for something different." end + test "when no options available, hints the user to create one by typing" do + doc = + render_sample_component([], + creatable: true + ) + + assert text_of_element(doc, "#dropdown-test-component div") == "Create an item by typing." + end + test "makes the html input required when required option is passed" do input_query = "input[type=text][required]" assert render_sample_component([], required: true) |> element_exists?(input_query) diff --git a/test/plausible_web/live/goal_settings/form_test.exs b/test/plausible_web/live/goal_settings/form_test.exs index 853426871..9a6d7c801 100644 --- a/test/plausible_web/live/goal_settings/form_test.exs +++ b/test/plausible_web/live/goal_settings/form_test.exs @@ -95,20 +95,13 @@ defmodule PlausibleWeb.Live.GoalSettings.FormTest do test "currency combo works", %{conn: conn, site: site} do lv = get_liveview(conn, site) - # Account for asynchronous updates. Here, the options, while static, - # are still a sizeable payload that we can defer before the user manages - # to click "this is revenue goal" switch. - # There's also throttling applied to the ComboBox. - :timer.sleep(200) type_into_combo(lv, "currency_input", "Polish") - :timer.sleep(200) html = render(lv) assert element_exists?(html, ~s/a[phx-value-display-value="PLN - Polish Zloty"]/) refute element_exists?(html, ~s/a[phx-value-display-value="EUR - Euro"]/) type_into_combo(lv, "currency_input", "Euro") - :timer.sleep(200) html = render(lv) refute element_exists?(html, ~s/a[phx-value-display-value="PLN - Polish Zloty"]/) @@ -135,16 +128,12 @@ defmodule PlausibleWeb.Live.GoalSettings.FormTest do type_into_combo(lv, "page_path_input", "/go/to/p") - # Account some large-ish margin for Clickhouse latency when providing suggestions asynchronously - # Might be too much, but also not enough on sluggish CI - :timer.sleep(350) html = render(lv) assert html =~ "Create "/go/to/p"" assert html =~ "/go/to/page/1" refute html =~ "/go/home" type_into_combo(lv, "page_path_input", "/go/h") - :timer.sleep(350) html = render(lv) assert html =~ "/go/home" refute html =~ "/go/to/page/1" diff --git a/test/plausible_web/live/props_settings/form_test.exs b/test/plausible_web/live/props_settings/form_test.exs new file mode 100644 index 000000000..dedefc609 --- /dev/null +++ b/test/plausible_web/live/props_settings/form_test.exs @@ -0,0 +1,156 @@ +defmodule PlausibleWeb.Live.PropsSettings.FormTest do + use PlausibleWeb.ConnCase, async: true + import Phoenix.LiveViewTest + import Plausible.Test.Support.HTML + + describe "Props submission" do + setup [:create_user, :log_in, :create_site] + + test "renders form fields", %{conn: conn, site: site} do + lv = get_liveview(conn, site) + html = render(lv) + + assert element_exists?(html, "form input[type=text][name=display-prop_input]") + assert element_exists?(html, "form input[type=hidden][name=prop]") + end + + test "renders error on empty submission", %{conn: conn, site: site} do + lv = get_liveview(conn, site) + html = lv |> element("form") |> render_submit() + assert html =~ "must be between 1 and 300 characters" + end + + test "renders error on whitespace submission", %{conn: conn, site: site} do + lv = get_liveview(conn, site) + html = lv |> element("form") |> render_submit(%{prop: " "}) + assert html =~ "must be between 1 and 300 characters" + end + + test "renders 'Create' suggestion", %{conn: conn, site: site} do + lv = get_liveview(conn, site) + type_into_combo(lv, "#prop_input", "Hello world") + html = render(lv) + assert text_of_element(html, "#dropdown-prop_input-option-0 a") == ~s/Create "Hello world"/ + end + + test "clicking suggestion fills out input", %{conn: conn, site: site} = context do + seed_props(context) + lv = get_liveview(conn, site) + type_into_combo(lv, "#prop_input", "amo") + + doc = + lv + |> element(~s/ul#dropdown-prop_input li#dropdown-prop_input-option-1 a/) + |> render_click() + + assert element_exists?(doc, ~s/input[type="hidden"][value="amount"]/) + end + + test "allowing a single property", %{conn: conn, site: site} do + {parent, lv} = get_liveview(conn, site, with_parent?: true) + refute render(parent) =~ "foobarbaz" + lv |> element("form") |> render_submit(%{prop: "foobarbaz"}) + parent_html = render(parent) + assert text_of_element(parent_html, "#prop-0") == "foobarbaz" + + site = Plausible.Repo.reload!(site) + assert site.allowed_event_props == ["foobarbaz"] + end + + test "allowing existing properties", %{conn: conn, site: site} = context do + seed_props(context) + {parent, lv} = get_liveview(conn, site, with_parent?: true) + parent_html = render(parent) + refute element_exists?(parent_html, "#prop-0") + refute element_exists?(parent_html, "#prop-1") + refute element_exists?(parent_html, "#prop-2") + + lv + |> element(~s/button[phx-click="allow-existing-props"]/) + |> render_click() + + parent_html = render(parent) + assert text_of_element(parent_html, "#prop-0") == "amount" + assert text_of_element(parent_html, "#prop-1") == "logged_in" + assert text_of_element(parent_html, "#prop-2") == "is_customer" + + site = Plausible.Repo.reload!(site) + assert site.allowed_event_props == ["amount", "logged_in", "is_customer"] + end + + test "does not show allow existing props button when there are no events with props", %{ + conn: conn, + site: site + } do + lv = get_liveview(conn, site) + refute element_exists?(render(lv), ~s/button[phx-click="allow-existing-props"]/) + end + + test "does not show allow existing props button after adding all suggestions", + %{ + conn: conn, + site: site + } = context do + seed_props(context) + + conn + |> get_liveview(site) + |> element(~s/button[phx-click="allow-existing-props"]/) + |> render_click() + + site = Plausible.Repo.reload!(site) + assert site.allowed_event_props == ["amount", "logged_in", "is_customer"] + + html = + conn + |> get_liveview(site) + |> render() + + refute element_exists?(html, ~s/button[phx-click="allow-existing-props"]/) + end + end + + defp seed_props(%{site: site}) do + populate_stats(site, [ + build(:event, + name: "Payment", + "meta.key": ["amount"], + "meta.value": ["500"] + ), + build(:event, + name: "Payment", + "meta.key": ["amount", "logged_in"], + "meta.value": ["100", "false"] + ), + build(:event, + name: "Payment", + "meta.key": ["amount", "is_customer"], + "meta.value": ["100", "false"] + ) + ]) + + :ok + end + + defp get_liveview(conn, site, opts \\ []) do + conn = assign(conn, :live_module, PlausibleWeb.Live.PropsSettings) + {:ok, lv, _html} = live(conn, "/#{site.domain}/settings/properties") + lv |> element(~s/button[phx-click="add-prop"]/) |> render_click() + assert form_view = find_live_child(lv, "props-form") + + if opts[:with_parent?] do + {lv, form_view} + else + form_view + end + end + + defp type_into_combo(lv, id, text) do + lv + |> element("input##{id}") + |> render_change(%{ + "_target" => ["display-#{id}"], + "display-#{id}" => "#{text}" + }) + end +end diff --git a/test/plausible_web/live/props_settings_test.exs b/test/plausible_web/live/props_settings_test.exs index 5324a73e5..c0739724e 100644 --- a/test/plausible_web/live/props_settings_test.exs +++ b/test/plausible_web/live/props_settings_test.exs @@ -1,202 +1,177 @@ -defmodule PlausibleWeb.Live.PropsSettings.FormTest do +defmodule PlausibleWeb.Live.PropsSettingsTest do use PlausibleWeb.ConnCase, async: true import Phoenix.LiveViewTest import Plausible.Test.Support.HTML - defp seed(%{site: site}) do - populate_stats(site, [ - build(:event, - name: "Payment", - "meta.key": ["amount"], - "meta.value": ["500"] - ), - build(:event, - name: "Payment", - "meta.key": ["amount", "logged_in"], - "meta.value": ["100", "false"] - ), - build(:event, - name: "Payment", - "meta.key": ["amount", "is_customer"], - "meta.value": ["100", "false"] - ) - ]) + describe "GET /:website/settings/properties" do + setup [:create_user, :log_in, :create_site] - :ok + test "lists props for the site and renders links", %{conn: conn, site: site} do + {:ok, site} = Plausible.Props.allow(site, ["amount", "logged_in", "is_customer"]) + conn = get(conn, "/#{site.domain}/settings/properties") + + resp = html_response(conn, 200) + assert resp =~ "Attach Custom Properties" + + assert element_exists?( + resp, + ~s|a[href="https://plausible.io/docs/custom-props/introduction"]| + ) + + assert resp =~ "amount" + assert resp =~ "logged_in" + assert resp =~ "is_customer" + end + + test "lists props with disallow actions", %{conn: conn, site: site} do + {:ok, site} = Plausible.Props.allow(site, ["amount", "logged_in", "is_customer"]) + conn = get(conn, "/#{site.domain}/settings/properties") + resp = html_response(conn, 200) + + for p <- site.allowed_event_props do + assert element_exists?( + resp, + ~s/button[phx-click="disallow-prop"][phx-value-prop=#{p}]#disallow-prop-#{p}/ + ) + end + end + + test "if no props are allowed, a proper info is displayed", %{conn: conn, site: site} do + conn = get(conn, "/#{site.domain}/settings/properties") + resp = html_response(conn, 200) + assert resp =~ "No properties configured for this site" + end + + test "if props are enabled, no info about missing props is displayed", %{ + conn: conn, + site: site + } do + {:ok, site} = Plausible.Props.allow(site, ["amount", "logged_in", "is_customer"]) + conn = get(conn, "/#{site.domain}/settings/properties") + resp = html_response(conn, 200) + refute resp =~ "No properties configured for this site" + end + + test "add property button is rendered", %{conn: conn, site: site} do + conn = get(conn, "/#{site.domain}/settings/properties") + resp = html_response(conn, 200) + assert element_exists?(resp, ~s/button[phx-click="add-prop"]/) + end + + test "search props input is rendered", %{conn: conn, site: site} do + conn = get(conn, "/#{site.domain}/settings/properties") + resp = html_response(conn, 200) + assert element_exists?(resp, ~s/input[type="text"]#filter-text/) + assert element_exists?(resp, ~s/form[phx-change="filter"]#filter-form/) + end end - setup [:create_user, :log_in, :create_site, :seed] + # validating input + # clicking suggestions fills out input + # adding props + # error when reached props limit + # clearserror when fixed input + # removal + # removal shows confirmation + # allow existing props: shows/hides + # after adding all suggestions no allow existing props - test "shows message when site has no allowed properties", %{conn: conn, site: site} do - {:ok, _lv, doc} = get_liveview(conn, site) - assert doc =~ "No properties configured for this site yet" + describe "PropsSettings live view" do + setup [:create_user, :log_in, :create_site] + + test "allows prop removal", %{conn: conn, site: site} do + {:ok, site} = Plausible.Props.allow(site, ["amount", "logged_in"]) + {lv, html} = get_liveview(conn, site, with_html?: true) + + assert html =~ "amount" + assert html =~ "logged_in" + + html = lv |> element(~s/button#disallow-prop-amount/) |> render_click() + + refute html =~ "amount" + assert html =~ "logged_in" + + html = get(conn, "/#{site.domain}/settings/properties") |> html_response(200) + + refute html =~ "amount" + assert html =~ "logged_in" + end + + test "allows props filtering / search", %{conn: conn, site: site} do + {:ok, site} = Plausible.Props.allow(site, ["amount", "logged_in", "is_customer"]) + {lv, html} = get_liveview(conn, site, with_html?: true) + + assert html =~ to_string("amount") + assert html =~ to_string("logged_in") + assert html =~ to_string("is_customer") + + html = type_into_search(lv, "is_customer") + + refute html =~ to_string("amount") + refute html =~ to_string("logged_in") + assert html =~ to_string("is_customer") + end + + test "allows resetting filter text via backspace icon", %{conn: conn, site: site} do + {:ok, site} = Plausible.Props.allow(site, ["amount", "logged_in", "is_customer"]) + {lv, html} = get_liveview(conn, site, with_html?: true) + + refute element_exists?(html, ~s/svg[phx-click="reset-filter-text"]#reset-filter/) + + html = type_into_search(lv, to_string("is_customer")) + + assert element_exists?(html, ~s/svg[phx-click="reset-filter-text"]#reset-filter/) + + html = lv |> element(~s/svg#reset-filter/) |> render_click() + + assert html =~ to_string("is_customer") + assert html =~ to_string("amount") + assert html =~ to_string("logged_in") + end + + test "allows resetting filter text via no match link", %{conn: conn, site: site} do + lv = get_liveview(conn, site) + html = type_into_search(lv, "Definitely this is not going to render any matches") + + assert html =~ "No properties found for this site. Please refine or" + assert html =~ "reset your search" + + assert element_exists?(html, ~s/a[phx-click="reset-filter-text"]#reset-filter-hint/) + html = lv |> element(~s/a#reset-filter-hint/) |> render_click() + + refute html =~ "No properties found for this site. Please refine or" + end + + test "clicking Add Property button renders the form view", %{conn: conn, site: site} do + lv = get_liveview(conn, site) + html = lv |> element(~s/button[phx-click="add-prop"]/) |> render_click() + + assert html =~ "Add Property for #{site.domain}" + + assert element_exists?( + html, + ~s/div#props-form form[phx-submit="allow-prop"][phx-click-away="cancel-allow-prop"]/ + ) + end end - test "renders dropdown with suggestions", %{conn: conn, site: site} do - {:ok, _lv, doc} = get_liveview(conn, site) - - assert text_of_element(doc, ~s/ul#dropdown-prop_input li#dropdown-prop_input-option-1/) == - "amount" - - assert text_of_element(doc, ~s/ul#dropdown-prop_input li#dropdown-prop_input-option-2/) == - "logged_in" - - assert text_of_element(doc, ~s/ul#dropdown-prop_input li#dropdown-prop_input-option-3/) == - "is_customer" - end - - test "input is a required field", %{conn: conn, site: site} do - {:ok, _lv, doc} = get_liveview(conn, site) - assert element_exists?(doc, ~s/input#prop_input[required]/) - end - - test "clicking suggestion fills out input", %{conn: conn, site: site} do - {:ok, lv, _doc} = get_liveview(conn, site) - - doc = - lv - |> element(~s/ul#dropdown-prop_input li#dropdown-prop_input-option-1 a/) - |> render_click() - - assert element_exists?(doc, ~s/input[type="hidden"][value="amount"]/) - end - - test "saving from suggestion adds to the list", %{conn: conn, site: site} do - {:ok, lv, _doc} = get_liveview(conn, site) - - doc = select_and_submit(lv, 1) - - assert text_of_element(doc, ~s/ul#allowed-props li#prop-0 span/) == "amount" - refute doc =~ "No properties configured for this site yet" - end - - test "saving from manual input adds to the list", %{conn: conn, site: site} do - {:ok, lv, _doc} = get_liveview(conn, site) - - type_into_combo(lv, "Operating System") - - doc = - lv - |> form("#props-form") - |> render_submit() - - assert text_of_element(doc, ~s/ul#allowed-props li#prop-0 span/) == "Operating System" - refute doc =~ "No properties configured for this site yet" - end - - test "shows error when input is invalid", %{conn: conn, site: site} do - {:ok, lv, _doc} = get_liveview(conn, site) - - type_into_combo(lv, " ") - doc = lv |> form("#props-form") |> render_submit() - - assert text_of_element(doc, ~s/div#prop-errors div/) == "must be between 1 and 300 characters" - assert doc =~ "No properties configured for this site yet" - end - - test "shows error when reached prop limit", %{conn: conn, site: site} do - props = for i <- 1..300, do: "my-prop-#{i}" - {:ok, site} = Plausible.Props.allow(site, props) - {:ok, lv, _doc} = get_liveview(conn, site) - - type_into_combo(lv, "my-prop-301") - doc = lv |> form("#props-form") |> render_submit() - - assert text_of_element(doc, ~s/div#prop-errors div/) == "should have at most 300 item(s)" - end - - test "clears error message when user fixes input", %{conn: conn, site: site} do - {:ok, lv, _doc} = get_liveview(conn, site) - - type_into_combo(lv, " ") - doc = lv |> form("#props-form") |> render_submit() - assert text_of_element(doc, ~s/div#prop-errors div/) == "must be between 1 and 300 characters" - - type_into_combo(lv, "my-prop") - doc = lv |> form("#props-form") |> render_submit() - refute element_exists?(doc, ~s/div#prop-errors/) - end - - test "clicking remove button removes from the list", %{conn: conn, site: site} do - {:ok, site} = Plausible.Props.allow(site, "my-prop") - {:ok, lv, doc} = get_liveview(conn, site) - - assert text_of_element(doc, ~s/ul#allowed-props li#prop-0 span/) == "my-prop" - - doc = - lv - |> element(~s/ul#allowed-props li#prop-0 button[phx-click="disallow"]/) - |> render_click() - - refute element_exists?(doc, ~s/ul#allowed-props li#prop-0 span/) - assert doc =~ "No properties configured for this site yet" - end - - test "remove button shows a confirmation popup", %{conn: conn, site: site} do - {:ok, site} = Plausible.Props.allow(site, "my-prop") - {:ok, _lv, doc} = get_liveview(conn, site) - - assert "Are you sure you want to remove property 'my-prop'? This will just affect the UI, all of your analytics data will stay intact." == - doc - |> Floki.find(~s/ul#allowed-props li#prop-0 button[phx-click="disallow"]/) - |> text_of_attr("data-confirm") - end - - test "clicking allow existing props button saves props from events", %{conn: conn, site: site} do - {:ok, lv, _doc} = get_liveview(conn, site) - - doc = - lv - |> element(~s/button[phx-click="allow-existing-props"]/) - |> render_click() - - assert text_of_element(doc, ~s/ul#allowed-props li#prop-0 span/) == "amount" - assert text_of_element(doc, ~s/ul#allowed-props li#prop-1 span/) == "logged_in" - assert text_of_element(doc, ~s/ul#allowed-props li#prop-2 span/) == "is_customer" - end - - test "does not show allow existing props button when there are no events with props", %{ - conn: conn, - user: user - } do - {:ok, _lv, doc} = get_liveview(conn, insert(:site, members: [user])) - refute element_exists?(doc, ~s/button[phx-click="allow-existing-props"]/) - end - - test "does not show allow existing props button after adding all suggestions", %{ - conn: conn, - site: site - } do - {:ok, lv, _doc} = get_liveview(conn, site) - - _doc = select_and_submit(lv, 1) - _doc = select_and_submit(lv, 1) - doc = select_and_submit(lv, 1) - - refute element_exists?(doc, ~s/button[phx-click="allow-existing-props"]/) - end - - defp get_liveview(conn, site) do + defp get_liveview(conn, site, opts \\ []) do conn = assign(conn, :live_module, PlausibleWeb.Live.PropsSettings) - {:ok, _lv, _doc} = live(conn, "/#{site.domain}/settings/properties") + {:ok, lv, html} = live(conn, "/#{site.domain}/settings/properties") + + if Keyword.get(opts, :with_html?) do + {lv, html} + else + lv + end end - defp select_and_submit(lv, suggestion_index) do + defp type_into_search(lv, text) do lv - |> element(~s/ul#dropdown-prop_input li#dropdown-prop_input-option-#{suggestion_index} a/) - |> render_click() - - lv - |> form("#props-form") - |> render_submit() - end - - defp type_into_combo(lv, input) do - lv - |> element("input#prop_input") + |> element("form#filter-form") |> render_change(%{ - "_target" => ["display-prop_input"], - "display-prop_input" => input + "_target" => ["filter-text"], + "filter-text" => "#{text}" }) end end