Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve segment owner names and dates in site timezone in the BE #5119

Merged
merged 6 commits into from
Feb 28, 2025

Conversation

apata
Copy link
Contributor

@apata apata commented Feb 26, 2025

Changes

  • Removes workaround with segment owner names and dates. Keeps the date util just in case.
  • Indicates clearly the different segments index payloads (one for public dashboards and one for site members).
  • Adds tests about claiming ownership of dangling segment.
  • Centralizes segment type label

Tests

  • Automated tests have been added

Changelog

  • This PR does not make a user-facing change

Documentation

  • This change does not need a documentation update

Dark mode

  • This PR does not change the UI

@apata apata force-pushed the saved-segments/resolve-owner-be branch from 863994f to 30a99fc Compare February 26, 2025 14:54
@apata apata added the preview label Feb 26, 2025
Copy link

Preview environment👷🏼‍♀️🏗️
PR-5119

@apata apata force-pushed the saved-segments/resolve-owner-be branch from dc61813 to 06e782f Compare February 27, 2025 09:56
@apata apata requested a review from ukutaht February 27, 2025 10:56
@@ -14,11 +14,24 @@ export type SavedSegment = {
id: number
name: string
type: SegmentType
owner_id: number
/** null owner_id or owner_name signifies that the owner has been removed from the site */
owner_id: number | null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have much experience with typescript types but would something like this be possible?

owner: {id: number, name: string} | null

?

This would disallow the following invalid state at the type level:

{
  owner_id: 123,
  owner_name: null
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, that state is invalid and I've updated the type definitions to reflect that: 26b136a

@apata apata added this pull request to the merge queue Feb 28, 2025
Merged via the queue into master with commit 5ae2fd8 Feb 28, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants