-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Refactor transaction ids to be UUIDs instead of random number #20904
Closed
OGPoyraz
wants to merge
8
commits into
develop
from
1102-use-uuids-for-transaction-ids-in-extension-transaction-controller
Closed
Refactor transaction ids to be UUIDs instead of random number #20904
OGPoyraz
wants to merge
8
commits into
develop
from
1102-use-uuids-for-transaction-ids-in-extension-transaction-controller
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
OGPoyraz
added
the
team-confirmations-system
DEPRECATED: please use "team-confirmations" label instead
label
Sep 18, 2023
OGPoyraz
force-pushed
the
1102-use-uuids-for-transaction-ids-in-extension-transaction-controller
branch
from
September 18, 2023 07:03
5b7a907
to
8c3c64d
Compare
OGPoyraz
force-pushed
the
1102-use-uuids-for-transaction-ids-in-extension-transaction-controller
branch
from
September 18, 2023 07:04
8c3c64d
to
1d50d68
Compare
OGPoyraz
force-pushed
the
1102-use-uuids-for-transaction-ids-in-extension-transaction-controller
branch
from
September 18, 2023 07:27
61f7cef
to
8bcc8ea
Compare
OGPoyraz
force-pushed
the
1102-use-uuids-for-transaction-ids-in-extension-transaction-controller
branch
2 times, most recently
from
September 18, 2023 08:06
36929c3
to
42aa6bb
Compare
OGPoyraz
force-pushed
the
1102-use-uuids-for-transaction-ids-in-extension-transaction-controller
branch
from
September 18, 2023 08:16
42aa6bb
to
e060a75
Compare
OGPoyraz
force-pushed
the
1102-use-uuids-for-transaction-ids-in-extension-transaction-controller
branch
from
September 18, 2023 09:08
c9561f2
to
340ebb8
Compare
OGPoyraz
force-pushed
the
1102-use-uuids-for-transaction-ids-in-extension-transaction-controller
branch
from
September 19, 2023 09:47
a7ff201
to
c8b485c
Compare
Builds ready [c8b485c]
Page Load Metrics (1743 ± 98 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #20904 +/- ##
===========================================
+ Coverage 68.17% 68.17% +0.01%
===========================================
Files 998 999 +1
Lines 39966 39982 +16
Branches 10686 10689 +3
===========================================
+ Hits 27243 27256 +13
- Misses 12723 12726 +3
☔ View full report in Codecov by Sentry. |
OGPoyraz
deleted the
1102-use-uuids-for-transaction-ids-in-extension-transaction-controller
branch
October 18, 2023 22:48
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation
Core
TransactionController
creates UUID strings as anid
for transactions. On the otherside extension creates random number while creating transactions usingcreateRandomId
.While we are on a mission for
TransactionController
alignment for both clients, for the sake of standardisation the use of UUIDs is preferred.So this PR aims to make transaction ids to be UUID instead of random number. And also creates a migration for persisted transactions to migrate over UUIDs.
Screenshots/Screencaps
No functional change.
Manual Testing Steps
This PRs manual testing could be tricky since it's using a migration and needs a history of transactions persisted in previous commit. So testing could be done in these steps:
develop
1102-use-uuids-for-transaction-ids-in-extension-transaction-controller
Apart from the case above, we need to be able to navigate between transactions on multiple transaction notification:
Pre-merge author checklist
Pre-merge reviewer checklist
If further QA is required (e.g. new feature, complex testing steps, large refactor), add the
Extension QA Board
label.In this case, a QA Engineer approval will be be required.