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

feat: add lastUpdatedAt to NetworkConfiguration #4652

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Prithpal-Sooriya
Copy link
Contributor

Explanation

This allows tracking of when the network configuration was updated, which is used to enable accurate network syncing

References

Changelog

@metamask/network-controller

  • ADDED: optional lastUpdatedAt to the NetworkConfiguration state. Used for performing network syncing.
  • ADDED: logic to update the lastUpdatedAt when a network is added or updated.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@Prithpal-Sooriya Prithpal-Sooriya added the team-notifications Notification Team changes. https://github.com/orgs/MetaMask/teams/notifications label Sep 3, 2024
Comment on lines +144 to +149
jest
.spyOn(
LastUpdatedAtNetworkConfigurationModule,
'lastUpdatedNetworkConfiguration',
)
.mockImplementation((n) => n);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need some thoughts on this.

The addition of the lastUpdatedAt field breaks many tests, especially when adding or updating a network. This is fine, however it also breaks tests that are marked as "if nothing is being changes > makes no updates to state".

We may need to rethink where we set the lastUpdatedAt field.

this allows tracking of when the network configuration was updated, which is used to enable accurate network syncing
@Prithpal-Sooriya Prithpal-Sooriya force-pushed the NOTIFY-1031/add-network-sync-fields branch from 8d92989 to 7b57cb3 Compare September 3, 2024 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-notifications Notification Team changes. https://github.com/orgs/MetaMask/teams/notifications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant