IOS-6610 IOS-7064 Add a mechanism for automatic indexation of wallets #3814

amuraveinik · 2024-09-10T08:05:44Z

Открыл PR из ветки соседнего, подмерджил сюда develop.
Старый PR пока оставлю для референса. Если что, то потом изменения перенесу туда

До/после

…add-a-mechanism-for-automatic-indexation-of-wallets-3

…dexation-of-wallets-3

amuraveinik · 2024-09-13T03:42:16Z

Потрогал логику хелпера, сделал его stateless, по мотивам коммента
#3322 (comment)

amuraveinik · 2024-09-13T03:40:54Z

Tangem/App/Models/UserWallet/StoredUserWallet.swift

@@ -66,3 +66,9 @@ extension StoredUserWallet: Decodable {
        }
    }
 }
+
+protocol NameableWallet {
+    var name: String { get set }


Добавил этот протокол на случай если понадобится переименовывать не только StoredUserWallet, но и другие сущности

Да у нас других сущностей и нет, этот код вообще временный, для миграции, я бы не усложнял

В общем, тут есть еще проблема. Для такой реализации очень сложно написать тесты, потому что надо инициализировать StoredUserWallet чисто чтобы потом из него получить имя. Основная сложность в том что в Card нет публичного инита, а сам он лежит в TangemSDK, и ради тестов добавлять туда инит - вот это точно оверкилл

StoredUserWallet( userWalletId: <#T##Data#>, name: <#T##String#>, card: CardDTO( card: <#T##Card#> ), associatedCardIds: <#T##Set<String>#>, walletData: <#T##DefaultWalletData#>, artwork: <#T##ArtworkInfo?#> )

Я поломал вчера голову как это лучше сделать, но кажется что удовлетворимых варианта только два:

Тот что сейчас в этом PR-е, с NameableWallet

Либо переделать реализацию, чтобы хелпер принимал массив строк. Так сделал Андрей Чукавин в изначальном PR-е, но там был коммент от тебя чтобы принимать модельки кошельков сразу

ок, оставляй так, не крит

amuraveinik · 2024-09-13T03:43:09Z

Тест кейсы и все остальное оставил из предыдущего PR

tureck1y · 2024-09-13T15:48:14Z

Tangem/App/Models/UserWallet/StoredUserWallet.swift

@@ -66,3 +66,9 @@ extension StoredUserWallet: Decodable {
        }
    }
 }
+
+protocol NameableWallet {
+    var name: String { get set }


Да у нас других сущностей и нет, этот код вообще временный, для миграции, я бы не усложнял

tureck1y · 2024-09-13T15:49:38Z

Tangem/Modules/Main/MainViewModel.swift

@@ -186,10 +188,21 @@ final class MainViewModel: ObservableObject {

        guard let userWalletModel = userWalletRepository.selectedModel else { return }

+        let otherWalletNames = userWalletRepository.models.compactMap { model -> String? in


Надо будет потом эту логику реюнуть постараться с экраном настроек

tureck1y · 2024-09-13T15:51:11Z

Tangem/App/Services/UserWalletRepository/CommonUserWalletRepository.swift

+            return
+        }
+
+        if let migratedWallets = UserWalletNameIndexationHelper.migratedWallets(wallets) {


если migratedWallets не используется, то и возвращать думаю не нужно

Ошибся немного, надо именно вернувшиеся сохранять, поправил

tureck1y · 2024-09-13T15:53:36Z

TangemTests/UserWalletNameIndexationTests.swift

+                existingNamesTestCases: [
+                    ("Wallet 2", /*     -> */ "Wallet 2"),
+                    ("Wallet", /*       -> */ "Wallet"),
+                    ("Wallet 2", /*     -> */ "Wallet 2"),


а почему тут Wallet 2, хотя на первой строке тоже Wallet 2?

А вы это обсуждали уже в старом PRе, я тут просто оставил как есть

#3322 (comment)

fedorov-d · 2024-09-13T16:14:58Z

Tangem/App/Services/UserWalletNameIndexationHelper/UserWalletNameIndexationHelper.swift

+    func nextIndex(for nameTemplate: String) -> Int {
+        let indexes = self[nameTemplate, default: []]
+
+        for i in 1 ... 100 {


почему такой range?

Ты прав, тут на самом деле надо сделать верхней границей indices.count

Логика следующая:

Если массив индексов пустой, то значит можно использовать первый (index = 1)

Если не пустой, то мы проходимся в цикле от 1 до размера массива и ищем дырку со свободным индексом. Если такая дырка есть, то значит она находится точно ниже верхней грани цикла, то есть меньше размера массива. Пример: для массива [1, 3, 4] дырка - это 2, а 2 < indices.count

Если такой дырки нет, то значит у нас индексы идут сплошняком [1, 2, 3], и тогда следующий индекс = размер + 1

fedorov-d · 2024-09-13T16:17:16Z

Tangem/App/Services/UserWalletRepository/CommonUserWalletRepository.swift

@@ -520,6 +526,18 @@ class CommonUserWalletRepository: UserWalletRepository {
        initializeServices(for: selectedModel)
    }

+    private func migrateNamesIfNeeded(_ wallets: inout [StoredUserWallet]) {
+        if AppSettings.shared.didMigrateUserWalletNames {


тут guard по смыслу больше подходит

amuraveinik · 2024-09-25T08:51:02Z

Tangem/Modules/Main/MainViewModel.swift

+        }
+
+        walletNameFieldValidator = AlertFieldValidator { input in
+            !(otherWalletNames.contains(input) || input.isEmpty)


Это изменение в т.ч. и для задачи IOS-7064

Почему правило именно такое - обсудили здесь

megakoko and others added 8 commits May 21, 2024 20:25

IOS-6610 Indexation helper

91467de

IOS-6610 Merge remote-tracking branch 'origin/develop' into IOS-6610-…

6034d4a

…add-a-mechanism-for-automatic-indexation-of-wallets-3

IOS-6610 Fixed conflicts

988136f

IOS-6610 Merge remote-tracking branch 'origin/develop' into IOS-6610-…

c14f65f

…add-a-mechanism-for-automatic-indexation-of-wallets-3

IOS-6610 Refactoring

dc12931

IOS-6610 Merge remote-tracking branch 'origin/develop' into IOS-6610-…

ae2db03

…add-a-mechanism-for-automatic-indexation-of-wallets-3

IOS-6610 Refactored test cases

8bded77

Merge branch 'develop' into IOS-6610-add-a-mechanism-for-automatic-in…

d720ffd

…dexation-of-wallets-3

amuraveinik requested review from tureck1y and a team as code owners September 10, 2024 08:05

amuraveinik requested a review from fedorov-d September 10, 2024 08:05

amuraveinik marked this pull request as draft September 10, 2024 08:05

amuraveinik added 3 commits September 10, 2024 11:33

IOS-6610 Fix warnings

5d429f0

Merge branch 'develop' into feature/IOS-6610_wallet_indexation

81067b0

IOS-6610 Make helper stateless

597c119

amuraveinik commented Sep 13, 2024

View reviewed changes

amuraveinik marked this pull request as ready for review September 13, 2024 03:43

tureck1y reviewed Sep 13, 2024

View reviewed changes

tureck1y changed the title ~~IOS-6610 Add a mechanism for automatic indexation of wallets❌~~ IOS-6610 Add a mechanism for automatic indexation of wallets Sep 13, 2024

fedorov-d reviewed Sep 13, 2024

View reviewed changes

amuraveinik added 4 commits September 24, 2024 07:22

Merge branch 'develop' into feature/IOS-6610_wallet_indexation

f610983

IOS-6610 Fix merge issues

e59f644

IOS-6610 CR fixes

1d7f745

IOS-6610 CR fixes

81851c3

amuraveinik requested review from tureck1y and fedorov-d September 24, 2024 06:15

Merge branch 'develop' into feature/IOS-6610_wallet_indexation

c508560

tureck1y previously approved these changes Sep 24, 2024

View reviewed changes

fedorov-d previously approved these changes Sep 25, 2024

View reviewed changes

Merge branch 'develop' into feature/IOS-6610_wallet_indexation

f989c80

amuraveinik enabled auto-merge (squash) September 25, 2024 08:21

amuraveinik changed the title ~~IOS-6610 Add a mechanism for automatic indexation of wallets~~ IOS-6610 IOS-7064 Add a mechanism for automatic indexation of wallets Sep 25, 2024

amuraveinik marked this pull request as draft September 25, 2024 08:48

auto-merge was automatically disabled September 25, 2024 08:48
Pull request was converted to draft

IOS-7064 Add emptiness check

3a4ce34

amuraveinik dismissed stale reviews from fedorov-d and tureck1y via 3a4ce34 September 25, 2024 08:49

amuraveinik commented Sep 25, 2024

View reviewed changes

amuraveinik marked this pull request as ready for review September 25, 2024 08:51

amuraveinik requested review from tureck1y and fedorov-d September 25, 2024 08:51

fedorov-d approved these changes Sep 25, 2024

View reviewed changes

amuraveinik added 2 commits September 25, 2024 16:12

Merge branch 'develop' into feature/IOS-6610_wallet_indexation

ee0a3a2

Merge branch 'develop' into feature/IOS-6610_wallet_indexation

cf4513c

tureck1y approved these changes Sep 26, 2024

View reviewed changes

tureck1y merged commit c00779b into develop Sep 26, 2024
3 checks passed

tureck1y deleted the feature/IOS-6610_wallet_indexation branch September 26, 2024 13:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IOS-6610 IOS-7064 Add a mechanism for automatic indexation of wallets #3814

IOS-6610 IOS-7064 Add a mechanism for automatic indexation of wallets #3814

amuraveinik commented Sep 10, 2024 •

edited by jira bot

Loading

amuraveinik commented Sep 13, 2024

amuraveinik Sep 13, 2024

tureck1y Sep 13, 2024

amuraveinik Sep 24, 2024 •

edited

Loading

tureck1y Sep 24, 2024

amuraveinik commented Sep 13, 2024

tureck1y Sep 13, 2024

tureck1y Sep 13, 2024

tureck1y Sep 13, 2024

amuraveinik Sep 24, 2024 •

edited

Loading

tureck1y Sep 13, 2024

amuraveinik Sep 24, 2024

fedorov-d Sep 13, 2024

amuraveinik Sep 24, 2024

fedorov-d Sep 13, 2024

amuraveinik Sep 25, 2024

		@@ -186,10 +188,21 @@ final class MainViewModel: ObservableObject {

		guard let userWalletModel = userWalletRepository.selectedModel else { return }

		let otherWalletNames = userWalletRepository.models.compactMap { model -> String? in

IOS-6610 IOS-7064 Add a mechanism for automatic indexation of wallets #3814

IOS-6610 IOS-7064 Add a mechanism for automatic indexation of wallets #3814

Conversation

amuraveinik commented Sep 10, 2024 • edited by jira bot Loading

amuraveinik commented Sep 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amuraveinik Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amuraveinik commented Sep 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amuraveinik Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amuraveinik commented Sep 10, 2024 •

edited by jira bot

Loading

amuraveinik Sep 24, 2024 •

edited

Loading

amuraveinik Sep 24, 2024 •

edited

Loading