From 293ffd647e2e43ddfcf9578e7189cb8d38791108 Mon Sep 17 00:00:00 2001 From: Dario Gabriel Lipicar Date: Fri, 29 Nov 2024 12:10:27 -0300 Subject: [PATCH] fix(@desktop/wallet): use unique activity entry key --- .../main/wallet_section/activity/entry.nim | 5 +---- .../main/wallet_section/activity/model.nim | 19 +------------------ src/backend/activity.nim | 9 +++++++++ test/nim/activity_tests.nim | 6 +++--- 4 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/app/modules/main/wallet_section/activity/entry.nim b/src/app/modules/main/wallet_section/activity/entry.nim index 3a6b5afdd9..c897a15dfe 100644 --- a/src/app/modules/main/wallet_section/activity/entry.nim +++ b/src/app/modules/main/wallet_section/activity/entry.nim @@ -138,11 +138,8 @@ QtObject: transactions:{$self.transactions}, )""" - # TODO: is this the right way to pass transaction identity? Why not use the instance? proc getId*(self: ActivityEntry): string {.slot.} = - if self.isMultiTransaction(): - return $(self.metadata.getMultiTransactionId().get()) - return $(self.metadata.getTransactionIdentity().get().hash) + return self.metadata.getKey() QtProperty[string] id: read = getId diff --git a/src/app/modules/main/wallet_section/activity/model.nim b/src/app/modules/main/wallet_section/activity/model.nim index 22c2ca5c8e..7e4d8056af 100644 --- a/src/app/modules/main/wallet_section/activity/model.nim +++ b/src/app/modules/main/wallet_section/activity/model.nim @@ -101,17 +101,6 @@ QtObject: self.countChanged() self.setHasMore(hasMore) - proc sameIdentity(e: entry.ActivityEntry, d: backend.Data): bool = - let m = e.getMetadata() - if m.getPayloadType() != d.payloadType: - return false - - if m.getPayloadType() == MultiTransaction: - let dID = d.id.get() - return dID > 0 and m.getMultiTransactionId().get(0) == dID - - return m.getTransactionIdentity().isSome() and d.transaction.isSome() and m.getTransactionIdentity().get() == d.transaction.get() - proc addNewEntries*(self: Model, newEntries: seq[entry.ActivityEntry], insertPositions: seq[int]) = let parentModelIndex = newQModelIndex() defer: parentModelIndex.delete @@ -128,7 +117,7 @@ QtObject: proc updateEntries*(self: Model, updates: seq[backend.Data]) = for i in countdown(self.entries.high, 0): for j in countdown(updates.high, 0): - if sameIdentity(self.entries[i], updates[j]): + if self.entries[i].getId() == updates[j].key: if updates[j].nftName.isSome(): self.entries[i].setNftName(updates[j].nftName.get()) if updates[j].nftUrl.isSome(): @@ -143,12 +132,6 @@ QtObject: QtProperty[bool] hasMore: read = getHasMore notify = hasMoreChanged - - proc getIndex*(self: Model, txHash: string): int {.slot.} = - for i, e in self.entries: - if e.getId() == txHash: - return i - return -1 proc refreshItemsContainingAddress*(self: Model, address: string) = for i in 0..self.entries.high: diff --git a/src/backend/activity.nim b/src/backend/activity.nim index ac317bcf05..4ee0ddc829 100644 --- a/src/backend/activity.nim +++ b/src/backend/activity.nim @@ -295,6 +295,7 @@ type ActivityEntry* = object # Identification payloadType: PayloadType + key: string transaction: Option[TransactionIdentity] id: int @@ -327,6 +328,7 @@ type # Mirrors status-go/services/wallet/activity/activity.go EntryData Data* = object payloadType*: PayloadType + key*: string transaction*: Option[TransactionIdentity] id*: Option[int] @@ -389,6 +391,9 @@ type proc getPayloadType*(ae: ActivityEntry): PayloadType = return ae.payloadType +proc getKey*(ae: ActivityEntry): string = + return ae.key + proc getTransactionIdentity*(ae: ActivityEntry): Option[TransactionIdentity] = if ae.payloadType == PayloadType.MultiTransaction: return none(TransactionIdentity) @@ -408,6 +413,7 @@ proc toJson*(ae: ActivityEntry): JsonNode {.inline.} = return %*(ae) proc fromJson*(e: JsonNode, T: typedesc[Data]): Data {.inline.} = + const keyField = "key" const transactionField = "transaction" const idField = "id" const transactionsField = "transactions" @@ -433,6 +439,7 @@ proc fromJson*(e: JsonNode, T: typedesc[Data]): Data {.inline.} = const isNewField = "isNew" result = T( payloadType: fromJson(e["payloadType"], PayloadType), + key: e[keyField].getStr(), transaction: if e.hasKey(transactionField): fromJson(e[transactionField], Option[TransactionIdentity]) else: @@ -503,6 +510,7 @@ proc fromJson*(e: JsonNode, T: typedesc[ActivityEntry]): ActivityEntry {.inline. let zeroValue: UInt256 = "0x0".parse(UInt256, 16) result = T( payloadType: data.payloadType, + key: data.key, transaction: data.transaction, id: if data.id.isSome: data.id.get() else: 0, transactions: data.transactions, @@ -531,6 +539,7 @@ proc `$`*(self: ActivityEntry): string = else: "none(TransactionIdentity)" return fmt"""ActivityEntry( payloadType:{$self.payloadType}, + key:{$self.key}, transaction:{transactionStr}, id:{self.id}, transactions:{self.transactions}, diff --git a/test/nim/activity_tests.nim b/test/nim/activity_tests.nim index 7dad273a00..144e1ec452 100644 --- a/test/nim/activity_tests.nim +++ b/test/nim/activity_tests.nim @@ -2,10 +2,10 @@ import unittest, json, options import backend/activity -const testOneNewJsonData = "{\"new\": [{\"pos\": 3, \"entry\": {\"payloadType\": 1, \"id\": 12, \"activityType\": 1, \"activityStatus\": 2, \"timestamp\": 1234567890, \"isNew\": true}}]}" -const testOneNewJsonDataMissingIsNew = "{\"new\": [{\"pos\": 3, \"entry\": {\"payloadType\": 1, \"id\": 12, \"activityType\": 1, \"activityStatus\": 2, \"timestamp\": 1234567890}}]}" +const testOneNewJsonData = "{\"new\": [{\"pos\": 3, \"entry\": {\"payloadType\": 1, \"key\": \"12ABCD\", \"id\": 12, \"activityType\": 1, \"activityStatus\": 2, \"timestamp\": 1234567890, \"isNew\": true}}]}" +const testOneNewJsonDataMissingIsNew = "{\"new\": [{\"pos\": 3, \"entry\": {\"payloadType\": 1, \"key\": \"12ABCD\", \"id\": 12, \"activityType\": 1, \"activityStatus\": 2, \"timestamp\": 1234567890}}]}" const oneRemovedJsonTestData = "{\"removed\":[{\"chainId\": 7, \"hash\": \"0x5\", \"address\": \"0x6\"}]}" -const testAllSetJsonData = "{\"hasNewOnTop\": true, \"new\": [{\"pos\": 3, \"entry\": {\"payloadType\": 1, \"id\": 12, \"activityType\": 1, \"activityStatus\": 2, \"timestamp\": 1234567890, \"isNew\": true}}], \"removed\":[{\"chainId\": 7, \"hash\": \"0x5\", \"address\": \"0x6\"}]}" +const testAllSetJsonData = "{\"hasNewOnTop\": true, \"new\": [{\"pos\": 3, \"entry\": {\"payloadType\": 1, \"key\": \"12ABCD\", \"id\": 12, \"activityType\": 1, \"activityStatus\": 2, \"timestamp\": 1234567890, \"isNew\": true}}], \"removed\":[{\"chainId\": 7, \"hash\": \"0x5\", \"address\": \"0x6\"}]}" suite "activity filter API json parsing":