Conversation
|
25c2cdd to
c603c59
Compare
| <div class="flex items-center justify-between gap-4 mt-2"> | ||
| <.input | ||
| type="radio" | ||
| class="block h-5 w-5 dark:bg-gray-700 border-gray-300 text-indigo-600 focus:ring-indigo-600" |
There was a problem hiding this comment.
issue, blocking: This class doesn't take effect, can put h-15 here and nothing happens, it is overridden. Same issue for all instances of .input type="radio" component in this file.
| |> Enum.with_index(1) | ||
| |> Enum.map(fn {step, idx} -> %{step | step_order: idx} end) | ||
|
|
||
| %{funnel | funnel_type: :sequential, steps: steps} |
There was a problem hiding this comment.
issue, non-blocking: This temporary funnel has contradictory values: it's first_and_last?: true, funnel_type: :sequential.
It seems the virtual field :funnel_type field creates a whole class of these integrity problems. There's a few others spots where they lurk in wait.
Can we migrate to a single persisted :funnel_type field for this feature, then migrate out to separate columns again if it indeed turns out we need something else?
- migration: create
funnel_typecolumn - deploy code change: when persisting funnels, set funnel_type based on strict_order
- migration: backfill
funnel_typebased onstrict_order - deploy code change: this PR, rewires everything to purely
funnel_typeand implements the new feature - migration: drop
strict_ordercolumn
There was a problem hiding this comment.
With a fresh eye, yeah, I agree, it's best to switch to the new field completely. However, with the abstracting layer already in place, it can be done simpler:
- add
funnel_typein addition to existingstrict_orderand newly addedfirst_and_last - turn
funnel_typeinto a non-virtual schema field; now everything should work normally when deployed in the current state with 3b76140 and 0c30a5c applied - backfill
funnel_type(data script TBD) - modify
Funnel.changesetandFunnels.base_get_queryto populate and readfunnel_typedirectly instead of using flag fields - remove
first_and_lastandstrict_order
38ee85d to
cbc733f
Compare
Co-authored-by: Artur Pata <4290938+apata@users.noreply.github.com>
0c30a5c to
5b023fd
Compare
3 participants
Changes
This PR implements support for a flexible funnel, where only first and last step are mandatory, while the rest is optional. It's a special, simplified case of an open funnel.
On the implementation side, another shrunk version of the queried funnel is computed - only with 2 steps - the first and last one. The metrics computed for the last step of the short funnel replace the metrics of the last step in the original funnel. And that's basically it.
The UI for configuring funnels is revised as well - now instead of a strict funnel toggle, a user can switch the funnel to work in one of three modes by setting its type:
The current implementation treats
funnel_typeas a computed field, abstracting the existingstrict_orderand the newfirst_an_lastflags. While flags could be replaced with a single enum field, I'm not sure if that's the way to go, yet. Funnel may undergo more changes some time soon and might require further revisions of how we store the configuration.The only visible change on the dashboard is mostly around the sequential/flexible/strict funnel indication copy shown in place of former strict funnel label.
Luckily, our public API did not expose "strict_order" as an option in funnel creation yet, so there's no problem of dealing with a potential breaking change - yet.
The migration will be extracted and released first once this PR gets approved.
Tests
Dark mode