Fix SegmentAuthorship

assets/js/dashboard/filtering/segments.ts

@@ -14,18 +14,23 @@ export type SavedSegment = {
   id: number
   name: string
   type: SegmentType
-  owner_id: number
-  owner_name: string
+  /** null owner_id or owner_name signifies that the owner has been removed from the site */
+  owner_id: number | null
+  owner_name: string | null
   /** datetime in site timezone, example 2025-02-26 10:00:00 */
   inserted_at: string
   /** datetime in site timezone, example 2025-02-26 10:00:00 */
   updated_at: string
 }
 
-export type SavedPublicSiteSegment = Pick<
+export type SavedSegmentPublic = Pick<
   SavedSegment,
   'id' | 'type' | 'name' | 'inserted_at' | 'updated_at'
->
+> & {
+  /** null owner_id signifies that the owner can't be shown */
+  owner_id: null
+  owner_name: null
+}
 
 export type SegmentDataFromApi = {
   filters: unknown[]

assets/js/dashboard/nav-menu/segments/searchable-segments-section.tsx

@@ -8,6 +8,7 @@ import {
   getFilterSegmentsByNameInsensitive,
   handleSegmentResponse,
   isSegmentFilter,
+  SavedSegmentPublic,
   SavedSegment,
   SegmentData,
   SegmentDataFromApi
@@ -24,14 +25,15 @@ import { AppNavigationLink } from '../../navigation/use-app-navigate'
 import { MenuSeparator } from '../nav-menu-components'
 import { ErrorPanel } from '../../components/error-panel'
 import { get } from '../../api'
+import { Role, useUserContext } from '../../user-context'
 
-const useSegmentsListQuery = () => {
+function useSegmentsListQuery<T extends boolean>(_isPublicRequest: T) {
   const site = useSiteContext()
-  return useQuery({
+  return useQuery<T extends true ? SavedSegmentPublic[] : SavedSegment[]>({
     queryKey: ['segments'],
     placeholderData: (previousData) => previousData,
     queryFn: async () => {
-      const response: SavedSegment[] = await get(
+      const response = await get(
         `/api/${encodeURIComponent(site.domain)}/segments`
       )
       return response
@@ -56,8 +58,11 @@ export const SearchableSegmentsSection = ({
   const { query, expandedSegment } = useQueryContext()
   const segmentFilter = query.filters.find(isSegmentFilter)
   const appliedSegmentIds = (segmentFilter ? segmentFilter[2] : []) as number[]
+  const user = useUserContext()
+
+  const isPublicListQuery = !user.loggedIn || user.role === Role.public
+  const { data, ...listQuery } = useSegmentsListQuery(isPublicListQuery)
 
-  const { data, ...listQuery } = useSegmentsListQuery()
   const [searchValue, setSearch] = useState<string>()
   const [showAll, setShowAll] = useState(false)
 
@@ -114,7 +119,18 @@ export const SearchableSegmentsSection = ({
                       }
                     </div>
 
-                    <SegmentAuthorship {...s} className="font-normal text-xs" />
+                    <SegmentAuthorship
+                      className="font-normal text-xs"
+                      {...(isPublicListQuery
+                        ? {
+                            showOnlyPublicData: true,
+                            segment: s as SavedSegmentPublic
+                          }
+                        : {
+                            showOnlyPublicData: false,
+                            segment: s as SavedSegment
+                          })}
+                    />
                   </div>
                 }
               >
@@ -229,7 +245,10 @@ const SegmentLink = ({
   name,
   appliedSegmentIds,
   closeList
-}: SavedSegment & { appliedSegmentIds: number[]; closeList: () => void }) => {
+}: Pick<SavedSegment, 'id' | 'name'> & {
+  appliedSegmentIds: number[]
+  closeList: () => void
+}) => {
   const { query } = useQueryContext()
 
   const { prefetchSegment } = useSegmentPrefetch({ id: String(id) })

assets/js/dashboard/segments/segment-authorship.test.tsx

@@ -0,0 +1,98 @@
+/** @format */
+
+import React from 'react'
+import { render, screen } from '@testing-library/react'
+import { SegmentAuthorship } from './segment-authorship'
+import { SegmentType } from '../filtering/segments'
+
+describe('public (no "by <user>" shown)', () => {
+  const showOnlyPublicData = true
+
+  test('shows only insert date if its the same as updated at', async () => {
+    render(
+      <SegmentAuthorship
+        showOnlyPublicData={showOnlyPublicData}
+        segment={{
+          type: SegmentType.site,
+          name: 'APAC region',
+          id: 100,
+          owner_id: null,
+          owner_name: null,
+          inserted_at: '2025-02-01 14:00:00',
+          updated_at: '2025-02-01 14:00:00'
+        }}
+      />
+    )
+
+    expect(screen.getByText('Created at 1 Feb')).toBeVisible()
+    expect(screen.queryByText(/Last updated/)).toBeNull()
+    expect(screen.queryByText(/by /)).toBeNull()
+  })
+
+  test('shows both insert date and updated at', async () => {
+    render(
+      <SegmentAuthorship
+        showOnlyPublicData={showOnlyPublicData}
+        segment={{
+          type: SegmentType.site,
+          name: 'APAC region',
+          id: 100,
+          owner_id: null,
+          owner_name: null,
+          inserted_at: '2025-02-01 14:00:00',
+          updated_at: '2025-02-01 15:00:00'
+        }}
+      />
+    )
+
+    expect(screen.getByText('Created at 1 Feb')).toBeVisible()
+    expect(screen.getByText('Last updated at 1 Feb')).toBeVisible()
+    expect(screen.queryByText(/by /)).toBeNull()
+  })
+})
+
+describe('shown to a site member ("by <user>" shown)', () => {
+  const showOnlyPublicData = false
+
+  test('shows only insert date if its the same as updated at', async () => {
+    render(
+      <SegmentAuthorship
+        showOnlyPublicData={showOnlyPublicData}
+        segment={{
+          type: SegmentType.site,
+          name: 'APAC region',
+          id: 100,
+          owner_id: null,
+          owner_name: null,
+          inserted_at: '2025-02-01 14:00:00',
+          updated_at: '2025-02-01 14:00:00'
+        }}
+      />
+    )
+
+    expect(screen.getByText('Created at 1 Feb by (Removed User)')).toBeVisible()
+    expect(screen.queryByText(/Last updated/)).toBeNull()
+  })
+
+  test('shows both insert date and updated at', async () => {
+    render(
+      <SegmentAuthorship
+        showOnlyPublicData={showOnlyPublicData}
+        segment={{
+          type: SegmentType.site,
+          name: 'APAC region',
+          id: 100,
+          owner_id: 500,
+          owner_name: 'Jane Smith',
+          inserted_at: '2025-02-01 14:00:00',
+          updated_at: '2025-02-05 15:00:00'
+        }}
+      />
+    )
+
+    expect(screen.getByText('Created at 1 Feb')).toBeVisible()
+    expect(
+      screen.queryByText('Last updated at 5 Feb by Jane Smith')
+    ).toBeVisible()
+  })
+})

assets/js/dashboard/segments/segment-authorship.tsx

@@ -1,19 +1,25 @@
 /** @format */
 
 import React from 'react'
-import { SavedSegment } from '../filtering/segments'
+import { SavedSegmentPublic, SavedSegment } from '../filtering/segments'
 import { formatDayShort, parseNaiveDate } from '../util/date'
 
-export const SegmentAuthorship = ({
+type SegmentAuthorshipProps = { className?: string } & (
+  | { showOnlyPublicData: true; segment: SavedSegmentPublic }
+  | { showOnlyPublicData: false; segment: SavedSegment }
+)
+
+export function SegmentAuthorship({
   className,
-  owner_name,
-  inserted_at,
-  updated_at
-}: Pick<SavedSegment, 'owner_name' | 'inserted_at' | 'updated_at'> & {
-  className?: string
-}) => {
-  const authorLabel = owner_name
+  showOnlyPublicData,
+  segment
+}: SegmentAuthorshipProps) {
+  const authorLabel =
+    showOnlyPublicData === true
+      ? null
+      : (segment.owner_name ?? '(Removed User)')
 
+  const { updated_at, inserted_at } = segment
   const showUpdatedAt = updated_at !== inserted_at
 
   return (

assets/js/dashboard/segments/segment-modals.tsx

@@ -466,7 +466,11 @@ export const SegmentModal = ({ id }: { id: SavedSegment['id'] }) => {
           <>
             <FiltersInSegment segment_data={data.segment_data} />
 
-            <SegmentAuthorship {...data} className="mt-4 text-sm" />
+            <SegmentAuthorship
+              segment={data}
+              showOnlyPublicData={false}
+              className="mt-4 text-sm"
+            />
             <div className="mt-4">
               <ButtonsRow>
                 <AppNavigationLink

lib/plausible/segments/segment.ex

@@ -202,7 +202,7 @@ defimpl Jason.Encoder, for: Plausible.Segments.Segment do
       type: segment.type,
       segment_data: segment.segment_data,
       owner_id: segment.owner_id,
-      owner_name: if(is_nil(segment.owner_id), do: "(Removed User)", else: segment.owner.name),
+      owner_name: if(is_nil(segment.owner_id), do: nil, else: segment.owner.name),
       inserted_at:
         segment.inserted_at
         |> Plausible.Timezones.to_datetime_in_timezone(segment.site.timezone)

test/plausible_web/controllers/api/internal_controller/segments_controller_test.exs

@@ -41,10 +41,10 @@ defmodule PlausibleWeb.Api.Internal.SegmentsControllerTest do
                      "id" => s.id,
                      "name" => s.name,
                      "type" => Atom.to_string(s.type),
-                     "owner_id" => nil,
-                     "owner_name" => nil,
                      "inserted_at" => Calendar.strftime(s.inserted_at, "%Y-%m-%d %H:%M:%S"),
                      "updated_at" => Calendar.strftime(s.updated_at, "%Y-%m-%d %H:%M:%S"),
+                     "owner_id" => nil,
+                     "owner_name" => nil,
                      "segment_data" => nil
                    }
                  end)
@@ -125,7 +125,7 @@ defmodule PlausibleWeb.Api.Internal.SegmentsControllerTest do
             type: :site,
             name: "Another region"
           )
-          |> Map.put(:owner_name, "(Removed User)")
+          |> Map.put(:owner_name, nil)
 
         conn =
           get(conn, "/api/#{site.domain}/segments")