Skip to content

Commit

Permalink
Resolve segment owner names and dates in site timezone in the BE
Browse files Browse the repository at this point in the history
  • Loading branch information
apata committed Feb 26, 2025
1 parent 6d3e6a7 commit 30a99fc
Show file tree
Hide file tree
Showing 13 changed files with 128 additions and 122 deletions.
8 changes: 8 additions & 0 deletions assets/js/dashboard/filtering/segments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,18 @@ export type SavedSegment = {
name: string
type: SegmentType
owner_id: number
owner_name: string
/** 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<
SavedSegment,
'id' | 'type' | 'name' | 'inserted_at' | 'updated_at'
>

export type SegmentDataFromApi = {
filters: unknown[]
labels: Record<string, string>
Expand Down
34 changes: 6 additions & 28 deletions assets/js/dashboard/segments/segment-authorship.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,51 +2,29 @@

import React from 'react'
import { SavedSegment } from '../filtering/segments'
import { PlausibleSite, useSiteContext } from '../site-context'
import { dateForSite, formatDayShort } from '../util/date'

const getAuthorLabel = (
site: Pick<PlausibleSite, 'members'>,
owner_id: number
) => {
if (!site.members) {
return ''
}

if (!owner_id || !site.members[owner_id]) {
return '(Removed User)'
}

// if (owner_id === user.id) {
// return 'You'
// }

return site.members[owner_id]
}
import { formatDayShort, parseNaiveDate } from '../util/date'

export const SegmentAuthorship = ({
className,
owner_id,
owner_name,
inserted_at,
updated_at
}: Pick<SavedSegment, 'owner_id' | 'inserted_at' | 'updated_at'> & {
}: Pick<SavedSegment, 'owner_name' | 'inserted_at' | 'updated_at'> & {
className?: string
}) => {
const site = useSiteContext()

const authorLabel = getAuthorLabel(site, owner_id)
const authorLabel = owner_name

const showUpdatedAt = updated_at !== inserted_at

return (
<div className={className}>
<div>
{`Created at ${formatDayShort(dateForSite(inserted_at, site))}`}
{`Created at ${formatDayShort(parseNaiveDate(inserted_at))}`}
{!showUpdatedAt && !!authorLabel && ` by ${authorLabel}`}
</div>
{showUpdatedAt && (
<div>
{`Last updated at ${formatDayShort(dateForSite(updated_at, site))}`}
{`Last updated at ${formatDayShort(parseNaiveDate(updated_at))}`}
{!!authorLabel && ` by ${authorLabel}`}
</div>
)}
Expand Down
4 changes: 1 addition & 3 deletions assets/js/dashboard/site-context.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ describe('parseSiteFromDataset', () => {
data-is-dbip="false"
data-current-user-role="owner"
data-current-user-id="1"
data-members='{"1":"Test User"}'
data-flags="{}"
data-valid-intervals-by-period='{"12mo":["day","week","month"],"30d":["day","week"],"6mo":["day","week","month"],"7d":["hour","day"],"all":["week","month"],"custom":["day","week","month"],"day":["minute","hour"],"month":["day","week"],"realtime":["minute"],"year":["day","week","month"]}'
{...attrs}
Expand Down Expand Up @@ -67,8 +66,7 @@ describe('parseSiteFromDataset', () => {
realtime: ['minute'],
year: ['day', 'week', 'month']
},
shared: false,
members: { '1': 'Test User' }
shared: false
}

it('parses from dom string map correctly', () => {
Expand Down
6 changes: 2 additions & 4 deletions assets/js/dashboard/site-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ export function parseSiteFromDataset(dataset: DOMStringMap): PlausibleSite {
isDbip: dataset.isDbip === 'true',
flags: JSON.parse(dataset.flags!),
validIntervalsByPeriod: JSON.parse(dataset.validIntervalsByPeriod!),
shared: !!dataset.sharedLinkAuth,
members: JSON.parse(dataset.members!)
shared: !!dataset.sharedLinkAuth
}
}

Expand Down Expand Up @@ -57,8 +56,7 @@ const siteContextDefaultValue = {
isDbip: false,
flags: {} as FeatureFlags,
validIntervalsByPeriod: {} as Record<string, Array<string>>,
shared: false,
members: null as null | Record<number, string>
shared: false
}

export type PlausibleSite = typeof siteContextDefaultValue
Expand Down
9 changes: 9 additions & 0 deletions assets/js/dashboard/util/date.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
formatDayShort,
formatISO,
nowForSite,
parseNaiveDate,
shiftMonths,
yesterday
} from './date'
Expand Down Expand Up @@ -119,3 +120,11 @@ describe('formatting UTC dates from database', () => {
).toEqual('2 Jan')
})
})

describe('formatting site-timezoned datetimes from database works flawlessly', () => {
it('is able to enrich UTC date string with site timezone, formatting the value correctly', () => {
expect(formatDayShort(parseNaiveDate('2025-01-01 14:00:00'))).toEqual(
'1 Jan'
)
})
})
3 changes: 1 addition & 2 deletions assets/test-utils/app-context-providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ export const TestContextProviders = ({
isDbip: false,
flags: {},
validIntervalsByPeriod: {},
shared: false,
members: { 1: 'Test User' }
shared: false
}

const site = { ...defaultSite, ...siteOptions }
Expand Down
33 changes: 22 additions & 11 deletions lib/plausible/segments/segment.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,6 @@ defmodule Plausible.Segments.Segment do

@type t() :: %__MODULE__{}

@derive {Jason.Encoder,
only: [
:id,
:name,
:type,
:segment_data,
:owner_id,
:inserted_at,
:updated_at
]}

schema "segments" do
field :name, :string
field :type, Ecto.Enum, values: @segment_types
Expand Down Expand Up @@ -204,3 +193,25 @@ defmodule Plausible.Segments.Segment do
end
end
end

defimpl Jason.Encoder, for: Plausible.Segments.Segment do
def encode(%Plausible.Segments.Segment{} = segment, opts) do
%{
id: segment.id,
name: segment.name,
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),
inserted_at:
segment.inserted_at
|> Plausible.Timezones.to_datetime_in_timezone(segment.site.timezone)
|> Calendar.strftime("%Y-%m-%d %H:%M:%S"),
updated_at:
segment.updated_at
|> Plausible.Timezones.to_datetime_in_timezone(segment.site.timezone)
|> Calendar.strftime("%Y-%m-%d %H:%M:%S")
}
|> Jason.Encode.map(opts)
end
end
17 changes: 14 additions & 3 deletions lib/plausible/segments/segments.ex
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ defmodule Plausible.Segments do
),
:ok <-
Segment.validate_segment_data(site, params["segment_data"], true) do
{:ok, Repo.insert!(changeset)}
{:ok, Repo.insert!(changeset) |> Repo.preload(:owner)}
else
%{valid?: false, errors: errors} ->
{:error, {:invalid_segment, errors}}
Expand Down Expand Up @@ -123,6 +123,7 @@ defmodule Plausible.Segments do
{:ok,
Repo.update!(
changeset,
preload: :owner,
returning: true
)}
else
Expand Down Expand Up @@ -271,7 +272,8 @@ defmodule Plausible.Segments do
from(segment in Segment,
where: segment.site_id == ^site_id,
where: segment.id == ^segment_id,
where: segment.type == :site or segment.owner_id == ^user_id
where: segment.type == :site or segment.owner_id == ^user_id,
preload: [:owner]
)

Repo.one(query)
Expand Down Expand Up @@ -339,7 +341,8 @@ defmodule Plausible.Segments do
from(segment in Segment,
select: ^fields,
where: segment.site_id == ^site_id,
order_by: [desc: segment.updated_at, desc: segment.id]
order_by: [desc: segment.updated_at, desc: segment.id],
preload: [:owner]
)

query =
Expand Down Expand Up @@ -370,4 +373,12 @@ defmodule Plausible.Segments do

"#{field} #{formatted_message}"
end

@doc """
This function enriches the segment with site, without actually querying the database for the site again.
Needed for Plausible.Segments.Segment custom JSON serialization.
"""
def enrich_with_site(%Segment{} = segment, %Plausible.Site{} = site) do
Map.put(segment, :site, site)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ defmodule PlausibleWeb.Api.Internal.SegmentsController do
H.not_enough_permissions(conn, "Not enough permissions to get segments")

{:ok, segments} ->
json(conn, segments)
json(
conn,
Enum.map(segments, fn segment -> Segments.enrich_with_site(segment, site) end)
)
end
end

Expand Down Expand Up @@ -54,7 +57,7 @@ defmodule PlausibleWeb.Api.Internal.SegmentsController do
segment_not_found(conn, params["segment_id"])

{:ok, segment} ->
json(conn, segment)
json(conn, Segments.enrich_with_site(segment, site))
end
end

Expand All @@ -80,7 +83,7 @@ defmodule PlausibleWeb.Api.Internal.SegmentsController do
})

{:ok, segment} ->
json(conn, segment)
json(conn, Segments.enrich_with_site(segment, site))
end
end

Expand Down Expand Up @@ -114,7 +117,7 @@ defmodule PlausibleWeb.Api.Internal.SegmentsController do
})

{:ok, segment} ->
json(conn, segment)
json(conn, Segments.enrich_with_site(segment, site))
end
end

Expand All @@ -141,7 +144,7 @@ defmodule PlausibleWeb.Api.Internal.SegmentsController do
segment_not_found(conn, params["segment_id"])

{:ok, segment} ->
json(conn, segment)
json(conn, Segments.enrich_with_site(segment, site))
end
end

Expand Down
34 changes: 0 additions & 34 deletions lib/plausible_web/controllers/stats_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,6 @@ defmodule PlausibleWeb.StatsController do
title: title(conn, site),
demo: demo,
flags: flags,
members:
if(flags.saved_segments,
do: get_members(conn.assigns[:site_role], site),
else: nil
),
is_dbip: is_dbip(),
dogfood_page_path: dogfood_page_path,
load_dashboard_js: true
Expand Down Expand Up @@ -381,11 +376,6 @@ defmodule PlausibleWeb.StatsController do
background: conn.params["background"],
theme: conn.params["theme"],
flags: flags,
members:
if(flags.saved_segments,
do: get_members(conn.assigns[:site_role], shared_link.site),
else: nil
),
is_dbip: is_dbip(),
load_dashboard_js: true
)
Expand All @@ -411,30 +401,6 @@ defmodule PlausibleWeb.StatsController do
end)
|> Map.new()

defp get_members(site_role, %Plausible.Site{} = site)
when site_role in [:viewer, :editor, :owner, :admin, :super_admin] do
site =
site
|> Plausible.Repo.preload(
team: [team_memberships: [:user]],
guest_memberships: [team_membership: [:user]]
)

site.guest_memberships
|> Enum.map(fn i = %Plausible.Teams.GuestMembership{} ->
i.team_membership
end)
|> Enum.concat(site.team.team_memberships)
|> Enum.map(fn i = %Plausible.Teams.Membership{} ->
{i.user.id, i.user.name}
end)
|> Map.new()
end

defp get_members(_site_role, _site) do
nil
end

defp is_dbip() do
on_ee do
false
Expand Down
1 change: 0 additions & 1 deletion lib/plausible_web/templates/stats/stats.html.heex
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
data-current-user-id={
if user = @conn.assigns[:current_user], do: user.id, else: Jason.encode!(nil)
}
data-members={Jason.encode!(@members)}
data-flags={Jason.encode!(@flags)}
data-valid-intervals-by-period={
Plausible.Stats.Interval.valid_by_period(site: @site) |> Jason.encode!()
Expand Down
Loading

0 comments on commit 30a99fc

Please sign in to comment.