Skip to content

Conversation

@MartinSchoeler
Copy link
Member

@MartinSchoeler MartinSchoeler commented Sep 18, 2025

Proposed changes (including videos or screenshots)

Turn the timestamp parser into a full fledged feature

Issue(s)

CORE-1349

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • New Features

    • Message timestamps are now parsed and rendered by default, promoting the Timestamp Parser to stable. Timestamps display reliably in your local timezone.
  • Chores

    • Removed the preview toggle for timestamp parsing from feature previews and settings.
    • Cleaned up related translation strings across locales.
    • Prepared version bumps for multiple packages.

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Sep 18, 2025

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Sep 18, 2025

🦋 Changeset detected

Latest commit: 3c8cfca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 39 packages
Name Type
@rocket.chat/meteor Major
@rocket.chat/gazzodown Major
@rocket.chat/i18n Major
@rocket.chat/ui-client Major
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/livechat Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-contexts Major
@rocket.chat/web-ui-registration Major
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/ui-voip Major
@rocket.chat/uikit-playground Patch
@rocket.chat/ui-avatar Major
@rocket.chat/ui-video-conf Major
@rocket.chat/queue-worker Patch
@rocket.chat/core-typings Major
@rocket.chat/rest-typings Major
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/ddp-client Patch
@rocket.chat/freeswitch Patch
@rocket.chat/http-router Patch
@rocket.chat/model-typings Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/presence-service Patch
@rocket.chat/license Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/models Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 18, 2025

Walkthrough

Removes the “enable-timestamp-message-parser” feature flag and related preview/i18n entries, making timestamp parsing always active. Updates Gazzodown to render Timestamp unconditionally, adjusts context shape, and cleans up client usage. Adds a changeset declaring major bumps for multiple packages and notes promotion of Timestamp Parser to stable.

Changes

Cohort / File(s) Summary
Versioning / Changeset
./.changeset/empty-buses-walk.md
Adds changeset with major version bumps for four packages; notes promotion of Timestamp Parser to stable.
Client: remove feature flag usage
apps/meteor/client/components/GazzodownText.tsx
Drops useFeaturePreview import and enableTimestamp usage; stops passing the flag via context provider.
Gazzodown: context and rendering
packages/gazzodown/src/MarkupInteractionContext.ts, packages/gazzodown/src/elements/Timestamp/index.tsx, packages/gazzodown/src/Markup.stories.tsx
Removes enableTimestamp from context type; TimestampWrapper now always renders Timestamp within ErrorBoundary; Storybook decorator no longer sets enableTimestamp.
i18n: remove feature preview keys
packages/i18n/src/locales/en.i18n.json, .../nb.i18n.json, .../nn.i18n.json, .../pt-BR.i18n.json, .../sv.i18n.json
Deletes Enable_timestamp and Enable_timestamp_description keys across locales.
UI client: feature preview list
packages/ui-client/src/hooks/useFeaturePreviewList.ts
Removes 'enable-timestamp-message-parser' from FeaturesAvailable union and from defaultFeaturesPreview.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Client as Client UI
  participant GZD as Gazzodown
  participant TW as TimestampWrapper
  participant EB as ErrorBoundary
  participant TS as Timestamp

  User->>Client: View message
  Client->>GZD: Render markup
  GZD->>TW: Render timestamp node
  TW->>EB: Wrap render
  EB->>TS: Render timestamp component
  TS-->>EB: Formatted time
  EB-->>GZD: Result (or UTC fallback on error)
  GZD-->>Client: Rendered content
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • dougfabris
  • aleksandernsilva

Poem

Hop hop! The flags are gone—so neat!
Ticks turn timestamps into treats.
No preview lists, no toggle fuss,
Just steady time for all of us.
I thump my paws in stable cheer,
Parsers promoted—right on here! ⏱️🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Linked Issues Check ❓ Inconclusive The PR appears to satisfy making the timestamp parser active by default and removing it from feature preview lists: the feature was removed from FeaturesAvailable and defaultFeaturesPreview, useFeaturePreview calls were deleted in GazzodownText and stories, the Timestamp wrapper now always renders the Timestamp component, and i18n keys for the preview toggle were removed. However, the provided summaries do not clearly show that all duplicated timestamp-parsing implementations across the codebase were located and removed, so I cannot conclusively verify the "remove duplicated code" objective from the supplied information. Please provide a brief list or code-search results showing where duplicated timestamp-parsing code was removed (or confirm no other copies existed) and confirm that admin/user feature-preview UIs were updated everywhere they are rendered so the reviewer can validate the final objective.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat: Move Timestamp parser out of Feature Preview" is concise and accurately summarizes the PR's primary change of promoting the timestamp parser out of feature preview; the diffs (removal of the feature-preview flag usage, removal from feature lists, deletion of i18n preview keys, and updates to the timestamp wrapper and context) align with that description and make the intent clear to reviewers.
Out of Scope Changes Check ✅ Passed All changed files and the added changeset are consistent with promoting the timestamp parser to stable: they remove the feature-preview flag, update the Markup interaction context and timestamp rendering, remove related i18n keys, and bump package versions; I see no evidence in the summaries of unrelated or out-of-scope functionality being modified.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/timestamp-parser

Comment @coderabbitai help to get the list of available commands and usage tips.

@MartinSchoeler MartinSchoeler added this to the 8.0.0 milestone Sep 18, 2025
@codecov
Copy link

codecov bot commented Sep 18, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.38%. Comparing base (3c42f0d) to head (3c8cfca).
⚠️ Report is 99 commits behind head on release-8.0.0.

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                @@
##           release-8.0.0   #36976      +/-   ##
=================================================
- Coverage          66.43%   66.38%   -0.06%     
=================================================
  Files               3276     3312      +36     
  Lines             109596   111663    +2067     
  Branches           20860    20941      +81     
=================================================
+ Hits               72814    74122    +1308     
- Misses             34116    34863     +747     
- Partials            2666     2678      +12     
Flag Coverage Δ
e2e 58.24% <ø> (+0.39%) ⬆️
unit 70.88% <0.00%> (-0.68%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MartinSchoeler MartinSchoeler marked this pull request as ready for review September 18, 2025 16:15
@MartinSchoeler MartinSchoeler requested a review from a team as a code owner September 18, 2025 16:15
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (8)
.changeset/empty-buses-walk.md (1)

1-8: Add explicit BREAKING notes and migration guidance to the changeset.

Major bumps are appropriate. Please expand the body with concrete breaking changes and migration steps (context/type removals, flag removal, i18n key deletions) to help integrators.

Apply this diff to append actionable notes:

 Promotes Timestamp Parser from preview state to stable
+
+BREAKING:
+- @rocket.chat/gazzodown: MarkupInteractionContextValue no longer includes `enableTimestamp`; Timestamp always renders.
+- @rocket.chat/ui-client: Feature flag `enable-timestamp-message-parser` removed from `FeaturesAvailable` and from defaults.
+- @rocket.chat/i18n: Removed keys `Enable_timestamp` and `Enable_timestamp_description`.
+
+MIGRATION:
+- Remove any usage of `enableTimestamp` from Markup providers/consumers.
+- Drop persisted `enable-timestamp-message-parser` entries from user/admin settings payloads if you store custom lists.
+- Replace any i18n references to the removed keys.
packages/gazzodown/src/Markup.stories.tsx (1)

81-107: Add a locale showcase to validate RelativeTime i18n.

Consider an extra story passing language in the context (e.g., 'pt-BR', 'sv') to visually verify formatting.

packages/gazzodown/src/elements/Timestamp/index.tsx (6)

38-42: Always-on rendering: good; ensure radix for parseInt.

Guard against accidental octal/hex by specifying base 10.

-	<ErrorBoundary fallback={<>{new Date(parseInt(children.value.timestamp) * 1000).toUTCString()}</>}>
-		<Timestamp format={children.value.format} value={new Date(parseInt(children.value.timestamp) * 1000)} />
+	<ErrorBoundary fallback={<>{new Date(parseInt(children.value.timestamp, 10) * 1000).toUTCString()}</>}>
+		<Timestamp format={children.value.format} value={new Date(parseInt(children.value.timestamp, 10) * 1000)} />

79-91: Language is ignored after the first tick; fix effect to use context language.

The interval callback hardcodes 'en' and language isn’t in deps, so changing language won’t update the text.

-	const { language } = useContext(MarkupInteractionContext);
+	const { language } = useContext(MarkupInteractionContext);
 	const [text, setTime] = useState(() => intlFormatDistance(time, Date.now(), { locale: language ?? 'en' }));
 	const [timeToRefresh, setTimeToRefresh] = useState(() => getTimeToRefresh(time));
 	useEffect(() => {
 		const interval = setInterval(() => {
-			setTime(intlFormatDistance(value.getTime(), Date.now(), { locale: 'en' }));
+			setTime(intlFormatDistance(value.getTime(), Date.now(), { locale: language ?? 'en' }));
 			setTimeToRefresh(getTimeToRefresh(time));
-		}, timeToRefresh);
+		}, timeToRefresh);
 		return () => clearInterval(interval);
-	}, [time, timeToRefresh, value]);
+	}, [time, timeToRefresh, value, language]);

95-115: Refresh cadence uses signed delta; use absolute difference to avoid 1s refresh forever.

Past timestamps yield negative deltas, hitting the “< 1 minute” branch indefinitely.

-const getTimeToRefresh = (time: number): number => {
-	const timeToRefresh = time - Date.now();
+const getTimeToRefresh = (time: number): number => {
+	const diff = Math.abs(Date.now() - time);
 	// less than 1 minute
-	if (timeToRefresh < 60000) {
+	if (diff < 60_000) {
 		return 1000;
 	}
 	// if the difference is in the minutes range, we should refresh the time in 1 minute / 2
-	if (timeToRefresh < 3600000) {
-		return 60000 / 2;
+	if (diff < 3_600_000) {
+		return 60_000 / 2;
 	}
 	// if the difference is in the hours range, we should refresh the time in 5 minutes
-	if (timeToRefresh < 86400000) {
-		return 60000 * 5;
+	if (diff < 86_400_000) {
+		return 60_000 * 5;
 	}
 	// refresh the time in 1 hour
-	return 3600000;
+	return 3_600_000;

63-73: Optional: Title localization.

title uses browser default. Consider toLocaleString(language) for consistency with inline text.


9-11: Nit: Rename prop type.

BoldSpanProps is misleading here; consider TimestampSpanProps.


32-34: Nit: Remove JSON.stringify from default branch.

Rendering a raw number JSON is odd; prefer ISO string.

-			return <time dateTime={value.toISOString()}> {JSON.stringify(value.getTime())}</time>;
+			return <time dateTime={value.toISOString()}>{value.toISOString()}</time>;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0b90f7a and 3c8cfca.

⛔ Files ignored due to path filters (1)
  • apps/meteor/public/images/featurePreview/timestamp.png is excluded by !**/*.png
📒 Files selected for processing (11)
  • .changeset/empty-buses-walk.md (1 hunks)
  • apps/meteor/client/components/GazzodownText.tsx (0 hunks)
  • packages/gazzodown/src/Markup.stories.tsx (1 hunks)
  • packages/gazzodown/src/MarkupInteractionContext.ts (0 hunks)
  • packages/gazzodown/src/elements/Timestamp/index.tsx (1 hunks)
  • packages/i18n/src/locales/en.i18n.json (0 hunks)
  • packages/i18n/src/locales/nb.i18n.json (0 hunks)
  • packages/i18n/src/locales/nn.i18n.json (0 hunks)
  • packages/i18n/src/locales/pt-BR.i18n.json (0 hunks)
  • packages/i18n/src/locales/sv.i18n.json (0 hunks)
  • packages/ui-client/src/hooks/useFeaturePreviewList.ts (1 hunks)
💤 Files with no reviewable changes (7)
  • packages/gazzodown/src/MarkupInteractionContext.ts
  • packages/i18n/src/locales/pt-BR.i18n.json
  • packages/i18n/src/locales/sv.i18n.json
  • apps/meteor/client/components/GazzodownText.tsx
  • packages/i18n/src/locales/nb.i18n.json
  • packages/i18n/src/locales/en.i18n.json
  • packages/i18n/src/locales/nn.i18n.json
🧰 Additional context used
🧬 Code graph analysis (1)
packages/gazzodown/src/Markup.stories.tsx (1)
packages/gazzodown/src/MarkupInteractionContext.ts (1)
  • MarkupInteractionContext (28-28)
🔇 Additional comments (3)
packages/gazzodown/src/Markup.stories.tsx (1)

18-18: LGTM: provider updated to the new context shape.

No gating needed anymore.

packages/ui-client/src/hooks/useFeaturePreviewList.ts (2)

3-3: LGTM: removed timestamp feature flag from the public union.

Type-level breakage is expected and correct for a removed feature.


20-62: Confirm no stale persisted entries or i18n keys remain downstream.
Repo search for enable-timestamp-message-parser / enableTimestamp and Enable_timestamp(_description)? returned no matches.
Manually verify and remove any stale translations (translation repo/service) and persisted user/DB feature settings.

@dougfabris dougfabris added the stat: QA assured Means it has been tested and approved by a company insider label Sep 18, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Sep 18, 2025
@MartinSchoeler MartinSchoeler added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Sep 18, 2025
@kodiakhq kodiakhq bot merged commit 4a60786 into release-8.0.0 Sep 18, 2025
71 of 73 checks passed
@kodiakhq kodiakhq bot deleted the feat/timestamp-parser branch September 18, 2025 18:53
gaolin1 pushed a commit to gaolin1/medsense.webchat that referenced this pull request Jan 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants