Skip to content

fix: use system time format in channel details - #1123

Open
guzino wants to merge 12 commits into
synonymdev:masterfrom
guzino:fix/channel-details-time-format
Open

guzino wants to merge 12 commits into
synonymdev:masterfrom
guzino:fix/channel-details-time-format

Conversation

@guzino

@guzino guzino commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Fixes #1110
Related to #1275

This PR routes the Lightning connection timestamps through the shared uiDateText entry point, so they follow the device's 12/24-hour setting.

Description

Channel details built its own formatter from DatePattern.CHANNEL_DETAILS, the hardcoded 24-hour pattern MMM d, yyyy, HH:mm. A connection created at 8:58 PM therefore read 20:58 on a device set to a 12-hour clock, while the activity list one screen away read 8:58 PM. #1111 added uiDateText and left this screen for a follow-up.

  • Fixes Created on, Order expiry, Opened on and Closed on ignoring the 12-hour setting by formatting them with uiDateText, which reads LocalIs24HourFormat and updates while the screen is open.
  • Adds a short-month date-time style that reuses DatePattern.DATE_FORMAT for the MMM d, yyyy date, so the date shape this screen shares with iOS stays the same.
  • Adds ISO-8601 parsing for the timestamps Blocktank returns, falling back to the raw string when parsing fails and rejecting pre-1970 instants so they cannot wrap to a huge unsigned value.
  • Removes DatePattern.CHANNEL_DETAILS and the screen's own date formatting helpers, now unused.

Out of Scope

  • Channel details: the , date/time separator is kept; Figma and iOS use - (Nov 13, 2022 - 14:38). Aligning it is a follow-up.
  • iOS parity: LightningConnectionDetailView.swift also hardcodes HH:mm, so iOS has the same 12-hour bug. Needs a matching bitkit-ios fix.
  • ui/screens/subscriptions/SubscriptionsScreen.kt: the first-period end date still uses a literal MMM d, yyyy, HH:mm and ignores the 12/24-hour setting. Not part of Channel details time uses different time format to system #1110.
  • Clock format changed with adb shell settings put system time_12_24 does not refresh an open screen, because the app listens for ACTION_TIME_CHANGED, which only the system Settings app broadcasts. Existing behaviour, unchanged.

Design

Preview

Device set to 12-hour time. Connection 6 is the Blocktank order connection from the earlier captures, since closed, so it still shows Created on and Opened on:

Figma Before After
Figma Connection details Before, 12-hour: Created on shows 14:44 After, 12-hour: Created on shows 2:44 PM
Before, 12-hour: Opened on shows 14:44 After, 12-hour: Opened on shows 2:44 PM

Device set to 24-hour time, after the fix:

Created on Opened on
After, 24-hour: Created on shows 14:44 After, 24-hour: Opened on shows 14:44

Flow on the fix build: 12-hour, then system Settings to 24-hour and back to the still-open screen, which updates live, then system Settings to Automatic and back, AM/PM again.

channel-date-format-flow.mp4

Earlier comparison captures from the original PR:

compare-12h compare-clock-formats

QA Notes

Manual Tests

  • 1. Device on 12-hour time → Settings → Advanced → Lightning Connections → a connection opened with Transfer to Spending: Created on and Opened on read like Sep 15, 2026, 2:44 PM, matching the status bar. On a CJIT connection only Created on is shown, since Opened on comes from the on-chain transfer that funded the channel.
  • 2a. System Settings → Date & time → 24-hour format → back to the app: the open Channel Detail updates to Sep 15, 2026, 14:44 without reopening.
    • 2b. Switch back to 12-hour or Automatic (en-US): the rows return to AM/PM live.
  • 3. Closed connection → Channel Detail: Closed on follows the same setting; on an unopened order, Order expiry does too.
  • 4. regression: Connection whose order timestamp is malformed: the row shows the raw value instead of being empty.

A new journey covers steps 1 and 2 and was run on a Pixel 9 emulator. It is committed here at journeys/lightning-connections/channel-details-time-format.xml, with its rows added to journeys/README.md (suites table, and the cross-platform table as not ported — iOS LightningConnectionDetailView.swift hardcodes MMM d, yyyy - HH:mm):

channel-details-time-format.xml
diff --git a/journeys/lightning-connections/channel-details-time-format.xml b/journeys/lightning-connections/channel-details-time-format.xml
new file mode 100644
index 000000000..ea7493b69
--- /dev/null
+++ b/journeys/lightning-connections/channel-details-time-format.xml
@@ -0,0 +1,33 @@
+<journey name="channel details time format">
+  <description>
+    Proves the Connection details screen follows the device 12/24-hour setting for its dates, and
+    updates while the app stays open. Precondition: onboarded dev wallet, English locale, and at least
+    one open Lightning connection opened with Transfer to Spending, paid from savings. "Created on"
+    comes from the Blocktank order, so any bought connection shows it, but "Opened on" is read from the
+    on-chain transfer that funded the channel, so a CJIT connection — paid over Lightning — never shows
+    it; pick a transfer-funded connection or the "Opened on" steps cannot pass. Before starting, record
+    `adb shell settings get system time_12_24` (null means Automatic) and restore it at the end. Change
+    the format through the system Settings UI, not `settings put`: the app listens for
+    ACTION_TIME_CHANGED, which only the Settings app broadcasts. "Order Expiry" shows only while the
+    order is unopened and "Closed on" only for a closed connection; check them the same way when present.
+  </description>
+  <actions>
+    <action>Run `adb shell am start -a android.settings.DATE_SETTINGS`, tap "12-hour / 24-hour format" and select "12-hour format"</action>
+    <action>Run `adb shell am start -n to.bitkit.dev/to.bitkit.ui.MainActivity`</action>
+    <action>Tap the menu icon (testTag "HeaderMenu")</action>
+    <action>Tap "Settings" (testTag "DrawerSettings")</action>
+    <action>Tap the "Advanced" tab (testTag "Tab-advanced")</action>
+    <action>Tap "Lightning Connections" (testTag "Channels")</action>
+    <action>Tap the connection opened with Transfer to Spending (testTag "Channel"; all rows share the tag, so with several connections take the one funded from savings)</action>
+    <action>Verify that the "Created on" value reads like "Sep 15, 2026, 2:44 PM", ending in AM or PM with no leading zero on the hour</action>
+    <action>Scroll down until "Opened on" is visible</action>
+    <action>Verify that the "Opened on" value ends in AM or PM</action>
+    <action>Run `adb shell am start -a android.settings.DATE_SETTINGS`, tap "12-hour / 24-hour format" and select "24-hour format"</action>
+    <action>Run `adb shell am start -n to.bitkit.dev/to.bitkit.ui.MainActivity`</action>
+    <action>Verify that the Connection details screen is still visible and the "Opened on" value reads like "Sep 15, 2026, 14:44", with no AM or PM</action>
+    <action>Scroll up until "Created on" is visible</action>
+    <action>Verify that the "Created on" value uses the 24-hour clock with no AM or PM</action>
+    <action>Run `adb shell am start -a android.settings.DATE_SETTINGS`, tap "12-hour / 24-hour format" and select the option recorded before starting ("Automatic" when it was null)</action>
+    <action>Run `adb shell am start -n to.bitkit.dev/to.bitkit.ui.MainActivity`</action>
+  </actions>
+</journey>

Automated Checks

  • Unit tests added in DateTimeExtTest.kt: ISO-8601 parsing over the plain and millisecond forms, unparseable and empty input, pre-1970 rejection, epoch start, the short-month style in both clock formats, and its month name under Locale.GERMANY.
  • Local: just compile, just test, just lint pass; detekt reports no findings in the changed files.

@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge.

Summary

Routes Lightning connection timestamps through the shared date formatter so channel details follow the device clock preference.

  • Adds non-negative ISO-8601-to-epoch parsing with raw-string fallback for invalid timestamps.
  • Adds a short-month date-and-time style supporting both 12-hour and 24-hour output.
  • Updates created, expiry, opened, and closed timestamps and adds unit coverage for parsing and formatting.

Reviews (3) · Last reviewed commit: "fix: reuse date format and reject pre-19..."

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please, fix the conflict

@jvsena42
jvsena42 marked this pull request as draft August 10, 2026 14:58
…-time-format

Co-authored-by: Cursor <cursoragent@cursor.com>
@guzino

guzino commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Merged master in 9301c217c. DateTimeExtTest keeps both the channel-details tests and the locale tests from #1124. just compile, just test, and just lint pass locally; no new detekt findings in the touched files.

@guzino
guzino marked this pull request as ready for review August 21, 2026 11:44
@jvsena42
jvsena42 self-requested a review August 26, 2026 14:39
@jvsena42
jvsena42 enabled auto-merge August 27, 2026 13:59
Comment thread app/src/main/java/to/bitkit/ext/DateTime.kt Outdated
Comment thread app/src/main/java/to/bitkit/ext/DateTime.kt Outdated
@jvsena42
jvsena42 marked this pull request as draft September 16, 2026 17:35
auto-merge was automatically disabled September 16, 2026 17:35

Pull request was converted to draft

@jvsena42
jvsena42 marked this pull request as ready for review September 16, 2026 18:53
jvsena42
jvsena42 previously approved these changes Sep 18, 2026

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved — no HIGH/MEDIUM findings at head e201c7e71.

Checked: The earlier change requests are addressed at head e201c7e: the branch merges cleanly into current master (GitHub reports MERGEABLE, git merge-tree shows no conflicts), DATE_TIME_YEAR_SHORT now reuses DatePattern.DATE_FORMAT instead of duplicating the pattern, and toEpochSecondsOrNull drops pre-1970 instants before toULong(), so they fall back to the raw string. All four rows (Created on, Order expiry, Opened on, Closed on) go through uiDateText, which reads LocalIs24HourFormat. That local is provided in ContentView.kt:454 by rememberIs24HourFormat(), so the screen updates on ACTION_TIME_CHANGED without reopening. channelDateText keeps the old fallback to the raw string when parsing fails. The Opened on value comes from the same activity timestamp/confirmTimestamp that the activity list already formats with uiDateText, so dropping the old runCatching there adds no new crash path. DatePattern.CHANNEL_DETAILS has no remaining references, and the new tests check both clock formats and the locale-dependent month name.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ovi-reviewer[bot]

This comment was marked as resolved.

jvsena42
jvsena42 previously approved these changes Sep 18, 2026

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved — no blocking HIGH/MEDIUM findings at head e67916b74.

Inline: LOW — A CJIT connection meets the precondition but never shows "Opened on"

Checked: Follow-up on 6f4adcf (new journey): Reviewed the one new own commit 6f4adcf (journey lightning-connections/channel-details-time-format.xml plus README rows). Every testTag it names exists at head: HeaderMenu (HomeScreen.kt:1402), DrawerSettings (DrawerMenu.kt:311), Tab-advanced (CustomTabRowWithSpacing.kt:62 from SettingsTab.Advanced), Channels (SettingsScreen.kt:596) and Channel (LightningConnectionsScreen.kt:345), in that navigation order. The journey changes the format through the Settings app rather than settings put, which is correct: rememberIs24HourFormat (DateText.kt:40) only re-reads DateFormat.is24HourFormat on ACTION_TIME_CHANGED, which settings put system time_12_24 never broadcasts, and since the local is provided once at ContentView.kt:460 a settings put change would stay stale even after reopening the screen. Relaunching with am start -n brings back the existing singleTask MainActivity, so the "still on Connection details" check holds. The expected values match DATE_TIME_YEAR_SHORT = MMM d, yyyy, h:mm a / MMM d, yyyy, HH:mm, and with the fix reverted master's hardcoded MMM d, yyyy, HH:mm would fail the first AM/PM assertion. The iOS skip is recorded in the cross-platform table.

Comment thread journeys/lightning-connections/channel-details-time-format.xml Outdated
ovi-reviewer[bot]

This comment was marked as resolved.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ovitrif

This comment was marked as resolved.

@ovitrif
ovitrif enabled auto-merge September 18, 2026 16:39
@ovitrif

ovitrif commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

@jvsena42 conflicts

@jvsena42

Copy link
Copy Markdown
Member

In hand — a merge of current master is running on this branch right now (this one pushes to the fork over SSH, since it is a cross-repo PR).

Same cause as the rest of the batch: every PR here commits a journey and adds a row to journeys/README.md, so each merge to master re-conflicts the others. #1279, #1284 and #1306 landed in the last hour. Background and the option to stop the churn: #1275 (comment)

@ovitrif

ovitrif commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

@talosmachina any idea how we can avoid this class of issues:
#1123 (comment)

I want something basic so we avoid unwanted conflicts of this kind, something simple not elaborate like the changelog fragments solution we have for changelogs.

@jvsena42

Copy link
Copy Markdown
Member

From the receiving end of about thirty of these today, the cause is narrow: journeys/README.md holds two hand-maintained indexes — the suites table and the "not ported" table — and every PR that adds a journey appends a row to both. Two PRs adding rows near the same alphabetical position collide, and merging any one of them re-conflicts the rest. Nothing else in the corpus conflicts; the journey files themselves never do, because each PR adds its own file.

Simplest fix, no tooling: move that metadata into the journey it describes. Each journey XML already has a <description>; a <ported> line (or an attribute) inside it carries the iOS status, and the suite list stops needing a row per journey. journeys/README.md keeps the format docs and the conventions, which change rarely, so it stops being a merge point. The information stays next to the thing it describes, which is also where a reader looks for it.

If a listing is still wanted, it can be generated rather than typed: scripts/journeys_index.py in #1250 already walks the corpus, so the same walk can print the table on demand or in CI, with nobody editing it by hand.

Middle option if the tables should stay: one README per suite (journeys/<suite>/README.md), which several suites already have. Then a new journey touches only its own suite file, and two PRs conflict only when they add to the same suite.

For this batch specifically, the offer stands to strip the README rows out of the open PRs and land them in one follow-up, which ends the re-merges immediately: #1275 (comment)

jvsena42
jvsena42 previously approved these changes Sep 18, 2026

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved — no blocking HIGH/MEDIUM findings at head 1a3f76e98.

Checked: Follow-up on 79bb2c8: the journey LOW is fixed. The precondition now requires a connection opened with Transfer to Spending from savings and explains why a CJIT connection never shows "Opened on" (it is read from the on-chain funding transfer), and the tap step picks that connection when several rows share the Channel tag.

ovitrif

This comment was marked as resolved.

@ovitrif

ovitrif commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Simplest fix, no tooling: move that metadata into the journey it describes.

jvsena42 Agreed on the diagnosis, and the suites table is now gone from both PRs — the count column was stale the moment any PR added a journey, and the Notes column had already moved: its setup information is the ## Capabilities table these PRs add, sourced from the suite READMEs. What was left was a directory listing that ls journeys/ answers correctly and a table answers eventually.

Nothing was dropped silently. On iOS the per-suite divergence notes ("adapted", "iOS-only") moved into ## Porting from Android, and the subscriptions Discover exclusion into ## Not ported, which is where a reader looks for porting status anyway.

Two corrections to the plan as written:

  • scripts/journeys_index.py is no longer part of chore: use journeys as prs qa contract #1250. The PR was rewritten and now ships no tooling at all, so the generated-listing option needs that script to come back in its own PR, deliberately. These PRs list drift tooling under Out of Scope on purpose: a journey that fails at review is the drift signal we want, and a stale index is the one we do not.
  • ## Not ported is untouched here. It is one prose block per gap, not a row per journey, so it is not the merge point the suites table was.

Worth noting the change argued for itself while landing: merging master into #1250 conflicted in exactly that table, for the third time in one sitting.

Per-suite READMEs and stripping the README rows from the other open PRs are both worth doing and are independent of these two — happy to take the offer on the batch.

@jvsena42
jvsena42 dismissed stale reviews from ovitrif and themself via c16e020 September 18, 2026 18:24

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ✅ Approve


Reaudit: diff 1 file.
Counterpart synonymdev/bitkit-ios#710: not compared.

Findings:
N/A

Audit:
Already done in comment.

Coverage:
QA: journeys and manual tests await all reviewers to approve, author can run it now via comment: @ovi-reviewer test


Reviewed by gpt-5.6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)

@jvsena42
jvsena42 requested a review from ovitrif September 18, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Channel details time uses different time format to system

3 participants