Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR plausible#5866

Type: Corrupted (contains bugs)

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, Bug fix


Description

  • Refactor consolidated view eligibility to use feature flags and billing checks instead of super admin role

  • Add team-specific user preferences for CTA dismissal state management

  • Implement CTA card with context-aware messaging for upgrade/setup requirements

  • Add dropdown menu for managing consolidated view creation and restoration

  • Enhance responsive design and fix z-index issues on sites page

  • Redirect ineligible users accessing consolidated views to /sites


Diagram Walkthrough

flowchart LR
  A["Feature Flag<br/>+ Billing Check"] -->|eligibility| B["ConsolidatedView<br/>ok_to_display?"]
  C["User Preferences<br/>Table"] -->|CTA state| D["CTA Dismiss/<br/>Restore"]
  B -->|eligible| E["Show CV Card"]
  B -->|ineligible| F["Show CTA Card"]
  D -->|dismissed| G["Dropdown Option<br/>to Restore"]
  F -->|context| H["Setup/Upgrade<br/>Messages"]
Loading

File Walkthrough

Relevant files
Enhancement
10 files
consolidated_view.ex
Refactor eligibility checks to use feature flags and billing
+61/-29 
user_preference.ex
New schema for team-specific user preferences                       
+28/-0   
memberships.ex
Add preference getter/setter functions for team memberships
+31/-0   
feature.ex
Register ConsolidatedView as a billing feature                     
+11/-1   
prima_dropdown.ex
New wrapper component for Prima dropdown functionality     
+47/-0   
sites.ex
Implement CTA card and dropdown menu for consolidated views
+204/-61
consolidated_views.ex
Display availability status and enhance delete confirmation
+10/-1   
stats.html.heex
Rename data attribute for consolidated view availability 
+1/-1     
dashboard.tsx
Update data attribute reference for consolidated view       
+1/-1     
live_socket.js
Import Dropdown hook from Prima library                                   
+2/-2     
Bug fix
2 files
stats_controller.ex
Add redirect for ineligible consolidated view access         
+10/-12 
generic.ex
Fix filter bar responsive design and width constraints     
+2/-2     
Configuration changes
5 files
plans_v3.json
Add consolidated_view feature to all business plans           
+17/-9   
plans_v4.json
Add consolidated_view feature to all business plans           
+17/-9   
plans_v5.json
Add consolidated_view feature to all business plans           
+17/-9   
seeds.exs
Enable consolidated_view feature flag in seeds                     
+2/-0     
test_helper.exs
Enable consolidated_view feature flag for tests                   
+1/-2     
Tests
5 files
consolidated_view_test.exs
Add tests for CTA state and eligibility checks                     
+100/-8 
sites_test.exs
Add comprehensive tests for CTA card behavior and dismissal
+204/-14
feature_test.exs
Add ConsolidatedView to billing feature tests                       
+3/-2     
plan_benefits_test.exs
Add ConsolidatedView to plan benefits assertions                 
+6/-3     
stats_controller_test.exs
Add test for redirect when ineligible for consolidated view
+20/-1   
Additional files
15 files
capabilities.ex +2/-1     
mix.exs +1/-1     
cache_test.exs +2/-1     
site_removal_test.exs +7/-4     
sites_test.exs +1/-0     
consolidated_view_sync_test.exs +0/-55   
consolidated_view_test.exs +3/-0     
query_parser_test.exs +7/-3     
query_test.exs +4/-2     
transfer_test.exs +3/-2     
external_sites_controller_sites_crud_api_test.exs +2/-1     
site_controller_test.exs +4/-1     
teams_test.exs +17/-39 
form_test.exs +1/-0     
capabilities_test.exs +10/-5   

aerosol and others added 30 commits November 10, 2025 13:40
 - add functions to manipulate user/team options (for CTA)
 - require at least two sites in order to create a consolidated view
 - require billing/plan compliance when computing eligibility
Co-authored-by: Sanne de Vries <[email protected]>
Co-authored-by: Uku Taht <[email protected]>
- require team-wise feature flag instead of super admin role
- redirect to /sites if the team isn't eligible any more
- enforce regular site in shared links controller
- Fix z-index issue with button dropdown
- Fix truncation issue with team name
- Improve responsiveness of site cards
- Fix focus state of site cards
- Improve responsiveness of consolidated view CTA
- Improve design of prima_dropdown component
- Replace `+` with plus icons in dropdown items
Will be included via main branch sync

This reverts commit 05bec55.
@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:
Unlogged Actions: New user actions like dismissing/restoring the consolidated view CTA and navigating
upgrade paths are introduced without any visible audit logging of who performed them and
when.

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:
Silent Failures: Functions like cta preference operations and ok_to_display?/enable pathways rely on
external calls but do not log or surface errors when dependencies fail or unexpected
states occur, which may hinder debugging.

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:
Event AuthZ: New LiveView events for dismissing/restoring CTAs rely on assigns but show no explicit
authorization checks or input validation, which may depend on external layers not visible
in this diff.

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

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
Possible issue
Require multiple sites for consolidation

Change the condition in has_sites_to_consolidate?/1 to require more than one
site (> 1) for creating a consolidated view, as it is not useful for a single
site.

extra/lib/plausible/consolidated_view.ex [211-213]

 defp has_sites_to_consolidate?(%Team{} = team) do
-  Teams.owned_sites_count(team) >= 1
+  Teams.owned_sites_count(team) > 1
 end
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This is a valid and important suggestion that improves the core logic of the feature. A consolidated view for a single site is redundant, so requiring at least two sites (> 1) makes the feature more logical and useful.

Medium
General
Add flash message on redirect

Add a flash message to inform the user why they are being redirected when a
consolidated view is no longer available.

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

 cond do
   consolidated_view? and not consolidated_view_available? ->
-    redirect(conn, to: Routes.site_path(conn, :index))
+    conn
+    |> put_flash(:info, "Consolidated view is no longer available for this team.")
+    |> 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]: 7

__

Why: This is a good suggestion for improving user experience. Adding a flash message clarifies why the user was redirected, preventing confusion when they are unexpectedly taken away from a page they tried to access.

Medium
  • 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.

4 participants