Skip to content

Fix Incorrect Asset Date When Viewing Restored Data Offline #3338

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented May 23, 2025

Problem

When the app is in offline mode and users view restored assets, the displayed dates are incorrect. This issue only occurs in offline mode and creates confusion for users who need accurate timestamps.

Root Cause

The uploadedAtOrTimestamp getter in the Proof class was using string length to determine if a timestamp was in milliseconds or seconds format:

// Old implementation
const proofTimestampInMilliseconds =
  this.timestamp.toString().length === LENGTH_IN_MILLISECONDS
    ? this.timestamp
    : this.timestamp * MILLISECONDS_PER_SECOND;

This approach is unreliable when handling timestamps from restored data, especially when offline.

Solution

Implemented a more robust threshold-based approach to detect if a timestamp is in milliseconds or seconds:

// New implementation
const MILLISECONDS_THRESHOLD = 10000000000; // 10^10, timestamps after March 2001
const proofTimestampInMilliseconds =
  this.timestamp > MILLISECONDS_THRESHOLD
    ? this.timestamp
    : this.timestamp * MILLISECONDS_PER_SECOND;

This approach is more reliable because:

  1. It checks the numerical value instead of string length
  2. The threshold (10^10) correctly differentiates between seconds and milliseconds for any date after March 2001
  3. It works consistently regardless of network status

Changes

  1. Replaced string length check with threshold-based detection in uploadedAtOrTimestamp getter
  2. Updated documentation to reflect the new approach
  3. Added entry to CHANGELOG.md

Fixes #3336.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • npm.pqina.nl
    • Triggering command: npm install (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@Copilot Copilot AI changed the title [WIP] Incorrect Asset Date When Viewing Restored Data Offline Fix Incorrect Asset Date When Viewing Restored Data Offline May 23, 2025
@Copilot Copilot AI requested a review from numbers-official May 23, 2025 06:58
Copilot finished work on behalf of numbers-official May 23, 2025 06:58
@numbers-official numbers-official requested a review from Copilot May 23, 2025 07:01
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue with incorrect asset date display in offline mode by replacing the unreliable string length check with a threshold-based approach in the timestamp getter within the Proof class.

  • Replaces a string-length check with numerical threshold logic.
  • Updates inline documentation and changelog to reflect the new approach.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/app/shared/repositories/proof/proof.ts Updates the logic in the timestamp getter to use a threshold-based approach and revises documentation comments.
CHANGELOG.md Adds an entry referencing the fix for incorrect asset date display (fixes #3336).

@@ -61,11 +61,12 @@ export class Proof {
*/
get uploadedAtOrTimestamp() {
const MILLISECONDS_PER_SECOND = 1000;
const LENGTH_IN_MILLISECONDS = 13;
const MILLISECONDS_THRESHOLD = 10000000000; // 10^10, timestamps after March 2001
Copy link
Preview

Copilot AI May 23, 2025

Choose a reason for hiding this comment

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

If this threshold logic is or may be reused elsewhere, consider extracting MILLISECONDS_THRESHOLD into a shared constants file for improved maintainability.

Copilot uses AI. Check for mistakes.

@olgahaha
Copy link
Contributor

This PR doesn’t address the root issue and should be revised.

@numbers-official numbers-official requested review from olgahaha and removed request for numbers-official May 23, 2025 07:07
Copy link
Contributor Author

Copilot AI commented May 23, 2025

This PR doesn’t address the root issue and should be revised.

I've revised the PR to address the root issue. The previous approach only fixed the timestamp conversion in the uploadedAtOrTimestamp getter, but not in the timestamp getter itself, which is used directly in other parts of the code like the information page.

The new solution modifies the timestamp getter to ensure all timestamps are consistently in milliseconds format, regardless of whether they come from restored data or not. This ensures correct date display in all cases, including offline mode. The fix is applied in commit 3b81985.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • npm.pqina.nl
    • Triggering command: npm install (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot finished work on behalf of olgahaha May 23, 2025 07:16
@numbers-official numbers-official assigned olgahaha and unassigned Copilot May 23, 2025
@numbers-official numbers-official requested review from numbers-official and removed request for olgahaha May 23, 2025 07:19
@olgahaha
Copy link
Contributor

Fix Codacy Static Code Analysis

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.

Incorrect Asset Date When Viewing Restored Data Offline
3 participants