Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR plausible#5866

Type: Clean (correct implementation)

Original PR Title: Consolidated View life cycle + billing integration
Original PR Description: ### Changes

This PR integrates consolidated view life cycle with billing properties.
It relies on two migrations (included here still), extracted to separate PRs:

Main changes:

  • new feature flag: :consolidated_view - we no longer display anything based on super_admin role; flag is not setup on production yet, meaning existing consolidated views will disappear (and no CTAs will be shown).
  • new billing feature: ConsolidatedView
  • all existing plans are updated, so that business tier includes the newly added features - either through static definitions or database migration
  • whenever /sites is visited an attempt is made to create consolidated view. If the team is eligible, and the feature flag is raised, the consolidated view card is shown. Otherwise a CTA card is displayed (courtesy of @sanne-san)
    • CTA card has several variants, depending on the context, but eventually leads to upgrade. Once the upgrade is performed, /sites will include the consolidated view card.
    • CTA card can be permanently dismissed per user and team; because local storage and cookies turned out to be very difficult to work with in this setup, the decision was made to introduce a new postgres table capable of storing user preferences in a team context (it's almost a 1:1 copy of the existing site user preferences table)
    • When CTA is dismissed, the "New Site" button becomes a prima drop-down (courtesy of @ukutaht) from which the user may restore it (by selecting "+ New consolidated view" dropdown item)
    • CRM now presents a more elaborate alert on deleting consolidated view, as well as the availability status (whether upgrade is required)
    • some minor improvements BTW:
      • shared links enforce regular sites,
      • accessing a consolidated view via pre-existing URL, while ineligible, redirects to /sites
      • at least 2 sites are now required to create (or even suggest) a consolidated view
      • ConsolidatedView.enabled?/1 has been removed, since enabling doesn't mean availability

TODO:

  • staging preview is currently unavailable due to pending migrations
  • there are visual issues with card plots and overlapping dropdowns
record-2025-11-10-10-29-34-year.node.norm.mp4
  • the dropdown doesn't respect dark mode
image

Tests

  • Automated tests have been added
  • This PR does not require tests

Changelog

  • Entry has been added to changelog
  • This PR does not make a user-facing change

Documentation

  • Docs have been updated
  • This change does not need a documentation update

Dark mode

  • The UI has been tested both in dark and light mode
  • This PR does not change the UI

Original PR URL: plausible#5866


PR Type

Enhancement


Description

  • Refactored consolidated view lifecycle to use feature flags instead of super_admin role

  • Added user preference storage for CTA dismissal state per team membership

  • Integrated billing feature availability checks for consolidated view access

  • Enhanced UI with dropdown menu for site management and dismissible CTA cards

  • Updated all business plan definitions to include consolidated view feature


Diagram Walkthrough

flowchart LR
  A["Feature Flag<br/>consolidated_view"] -->|enabled?| B["Billing Check<br/>ConsolidatedView"]
  B -->|available| C["View Enabled<br/>ok_to_display?"]
  D["User Preferences<br/>CTA dismissed?"] -->|state| E["CTA Card<br/>or View Card"]
  C -->|true| E
  F["Business Plan"] -->|includes| B
Loading

File Walkthrough

Relevant files
Enhancement
12 files
consolidated_view.ex
Refactored view lifecycle with feature flags and billing checks
+61/-29 
consolidated_views.ex
Added availability status display and enhanced delete confirmation
+10/-1   
feature.ex
Added ConsolidatedView as new billing feature module         
+11/-1   
memberships.ex
Added preference storage and retrieval for team memberships
+31/-0   
user_preference.ex
New schema for storing team-specific user preferences       
+28/-0   
prima_dropdown.ex
New dropdown component wrapper with Prima integration       
+47/-0   
stats_controller.ex
Updated consolidated view availability checks and shared link handling
+10/-12 
sites.ex
Refactored consolidated view initialization with CTA management and
dropdown UI
+204/-61
capabilities.ex
Added ConsolidatedView to API capabilities schema               
+2/-1     
live_socket.js
Added Dropdown hook from Prima library                                     
+2/-2     
dashboard.tsx
Updated data attribute name for consolidated view availability
+1/-1     
stats.html.heex
Updated template data attribute for consolidated view availability
+1/-1     
Formatting
1 files
generic.ex
Updated filter bar styling with improved responsive layout
+2/-2     
Dependencies
1 files
mix.exs
Updated Prima dependency to version 0.1.8                               
+1/-1     
Configuration changes
5 files
seeds.exs
Enabled consolidated_view feature flag in seeds                   
+2/-0     
test_helper.exs
Updated test setup to enable consolidated_view flag           
+1/-2     
plans_v3.json
Added consolidated_view feature to all business plans       
+17/-9   
plans_v4.json
Added consolidated_view feature to all business plans       
+17/-9   
plans_v5.json
Added consolidated_view feature to all business plans       
+17/-9   
Tests
18 files
feature_test.exs
Added ConsolidatedView to business feature tests                 
+3/-2     
plan_benefits_test.exs
Added Consolidated View to business plan benefits               
+6/-3     
cache_test.exs
Updated tests to require minimum 2 sites for consolidation
+2/-1     
consolidated_view_test.exs
Comprehensive tests for CTA state and availability checks
+100/-8 
site_removal_test.exs
Updated tests for consolidated view disabling logic           
+7/-4     
sites_test.exs
Updated consolidated view stats date reset tests                 
+1/-0     
consolidated_view_sync_test.exs
Removed obsolete test file for old ok_to_display signature
+0/-55   
consolidated_view_test.exs
Updated tests to require minimum 2 sites for consolidation
+3/-0     
query_parser_test.exs
Updated query parser tests for consolidated site IDs         
+7/-3     
query_test.exs
Updated query tests for consolidated site IDs handling     
+4/-2     
transfer_test.exs
Updated site transfer tests for consolidated view logic   
+3/-2     
external_sites_controller_sites_crud_api_test.exs
Added ConsolidatedView feature to enterprise plan test setup
+2/-1     
site_controller_test.exs
Updated consolidated view settings tests with site requirements
+4/-1     
stats_controller_test.exs
Updated stats controller tests for availability checks     
+20/-1   
teams_test.exs
Updated customer support team tests for consolidated views
+17/-39 
form_test.exs
Updated goal form tests for consolidated view requirements
+1/-0     
sites_test.exs
Comprehensive tests for CTA display, dismissal, and restoration
+204/-14
capabilities_test.exs
Added ConsolidatedView to capabilities API tests                 
+10/-5   

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logs: New user-triggered events to dismiss/restore the consolidated view CTA are handled without
any audit logging of actor, team, action, or outcome.

Referred Code
  def handle_event("consolidated-view-cta-dismiss", _, socket) do
    :ok =
      Plausible.ConsolidatedView.dismiss_cta(
        socket.assigns.current_user,
        socket.assigns.current_team
      )

    {:noreply, assign(socket, :consolidated_view_cta_dismissed?, true)}
  end

  def handle_event("consolidated-view-cta-restore", _, socket) do
    :ok =
      Plausible.ConsolidatedView.restore_cta(
        socket.assigns.current_user,
        socket.assigns.current_team
      )

    {:noreply, assign(socket, :consolidated_view_cta_dismissed?, false)}
  end
end

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing fallback: Preference getters/setters and consolidated view enablement rely on external calls and
flags without explicit error handling or fallback behavior beyond returning errors, which
may leave UI in inconsistent state if dependencies fail.

Referred Code
@spec cta_dismissed?(User.t(), Team.t()) :: boolean()
def cta_dismissed?(%User{} = user, %Team{} = team) do
  {:ok, team_membership} = Teams.Memberships.get_team_membership(team, user)
  Teams.Memberships.get_preference(team_membership, :consolidated_view_cta_dismissed)
end

@spec dismiss_cta(User.t(), Team.t()) :: :ok
def dismiss_cta(%User{} = user, %Team{} = team) do
  {:ok, team_membership} = Teams.Memberships.get_team_membership(team, user)
  Teams.Memberships.set_preference(team_membership, :consolidated_view_cta_dismissed, true)

  :ok
end

@spec restore_cta(User.t(), Team.t()) :: :ok
def restore_cta(%User{} = user, %Team{} = team) do
  {:ok, team_membership} = Teams.Memberships.get_team_membership(team, user)

  Teams.Memberships.set_preference(
    team_membership,
    :consolidated_view_cta_dismissed,


 ... (clipped 5 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Preference upsert: The upsert for team membership user preferences writes user-controlled state without
explicit authorization checks in this diff, requiring verification that callers enforce
membership/permission validation.

Referred Code
@spec set_preference(Teams.Membership.t(), atom(), any()) ::
        Teams.Memberships.UserPreference.t()
def set_preference(team_membership, option, value)
    when option in Teams.Memberships.UserPreference.options() do
  team_membership
  |> Teams.Memberships.UserPreference.changeset(%{option => value})
  |> Repo.insert!(
    conflict_target: [:team_membership_id],
    on_conflict:
      from(p in Teams.Memberships.UserPreference, update: [set: [{^option, ^value}]]),
    returning: true
  )
end

@spec get_preference(Teams.Membership.t(), atom()) :: any()
def get_preference(team_membership, option)
    when option in Teams.Memberships.UserPreference.options() do
  defaults = %Teams.Memberships.UserPreference{}

  query =
    from(


 ... (clipped 8 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Implicitly creating views can be problematic

Refactor the consolidated view creation to be an explicit user action (e.g., a
button click) instead of an implicit action on every /sites page load. This
separates creation from display logic, improving performance and simplifying the
code.

Examples:

lib/plausible_web/live/sites.ex [1056-1082]
    defp init_consolidated_view_assigns(user, team) do
      if ConsolidatedView.flag_enabled?(team) do
        case ConsolidatedView.enable(team) do
          {:ok, view} ->
            %{
              consolidated_view: view,
              can_manage_consolidated_view?: ConsolidatedView.can_manage?(user, team),
              consolidated_stats: :loading,
              no_consolidated_view_reason: nil,
              consolidated_view_cta_dismissed?: ConsolidatedView.cta_dismissed?(user, team)

 ... (clipped 17 lines)

Solution Walkthrough:

Before:

# in lib/plausible_web/live/sites.ex

def mount(..., socket):
  # ...
  assigns = init_consolidated_view_assigns(user, team)
  # ...

defp init_consolidated_view_assigns(user, team):
  # This is called on every page load
  case ConsolidatedView.enable(team) do
    {:ok, view} ->
      # If successful, show the consolidated view card
      %{consolidated_view: view, ...}
    {:error, reason} ->
      # If not, show a CTA card with the reason
      %{no_consolidated_view_reason: reason, ...}
  end

After:

# in lib/plausible_web/live/sites.ex

def mount(..., socket):
  # ...
  assigns = init_consolidated_view_assigns(user, team)
  # ...

defp init_consolidated_view_assigns(user, team):
  # On page load, only check for eligibility, don't create
  if ConsolidatedView.get(team) do
    # Show existing view
  else if ConsolidatedView.is_eligible_for_creation?(team) do
    # Show CTA with a "Create View" button
  else
    # Show CTA with "Upgrade" button
  end

def handle_event("create-consolidated-view", _, socket):
  # Create the view only on explicit user action
  ConsolidatedView.enable(socket.assigns.current_team)
  # ... update socket

Suggestion importance[1-10]: 9

__

Why: This is an excellent architectural suggestion that correctly identifies a major design issue: performing a potential write operation (ConsolidatedView.enable) on every page load (mount). Adopting this change would improve performance, adhere to RESTful principles (separating GET from state-changing actions), and significantly simplify the logic in lib/plausible_web/live/sites.ex.

High
Possible issue
Avoid potential crashes from pattern matching

Refactor cta_dismissed?/2, dismiss_cta/2, and restore_cta/2 to use a with
statement to safely handle potential errors from get_team_membership and avoid
MatchError exceptions.

extra/lib/plausible/consolidated_view.ex [24-49]

 @spec cta_dismissed?(User.t(), Team.t()) :: boolean()
 def cta_dismissed?(%User{} = user, %Team{} = team) do
-  {:ok, team_membership} = Teams.Memberships.get_team_membership(team, user)
-  Teams.Memberships.get_preference(team_membership, :consolidated_view_cta_dismissed)
+  with {:ok, team_membership} <- Teams.Memberships.get_team_membership(team, user) do
+    Teams.Memberships.get_preference(team_membership, :consolidated_view_cta_dismissed)
+  else
+    _ -> false
+  end
 end
 
 @spec dismiss_cta(User.t(), Team.t()) :: :ok
 def dismiss_cta(%User{} = user, %Team{} = team) do
-  {:ok, team_membership} = Teams.Memberships.get_team_membership(team, user)
-  Teams.Memberships.set_preference(team_membership, :consolidated_view_cta_dismissed, true)
+  with {:ok, team_membership} <- Teams.Memberships.get_team_membership(team, user) do
+    Teams.Memberships.set_preference(team_membership, :consolidated_view_cta_dismissed, true)
+  end
 
   :ok
 end
 
 @spec restore_cta(User.t(), Team.t()) :: :ok
 def restore_cta(%User{} = user, %Team{} = team) do
-  {:ok, team_membership} = Teams.Memberships.get_team_membership(team, user)
-
-  Teams.Memberships.set_preference(
-    team_membership,
-    :consolidated_view_cta_dismissed,
-    false
-  )
+  with {:ok, team_membership} <- Teams.Memberships.get_team_membership(team, user) do
+    Teams.Memberships.set_preference(
+      team_membership,
+      :consolidated_view_cta_dismissed,
+      false
+    )
+  end
 
   :ok
 end
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that pattern matching on the result of Teams.Memberships.get_team_membership/2 can cause a MatchError, and proposes a robust solution using a with statement to prevent a potential crash.

Medium
General
Inform user about redirection reason

Add a flash message before redirecting a user who has lost access to a
consolidated view to inform them of the reason for the redirection.

lib/plausible_web/controllers/stats_controller.ex [76-80]

 cond do
   consolidated_view? and not consolidated_view_available? and site_role != :super_admin ->
-    redirect(conn, to: Routes.site_path(conn, :index))
+    conn
+    |> put_flash(:error, "You no longer have access to this consolidated view. Please upgrade your plan to restore access.")
+    |> redirect(to: Routes.site_path(conn, :index))
 
   (stats_start_date && can_see_stats?) || (can_see_stats? && skip_to_dashboard?) ->
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: This suggestion improves user experience by adding a flash message to explain a redirect, which is helpful but not a critical bug fix.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants