Fix/accounts tab search does not work correctly with special chars

[mempoolboy]

Unlabeled accounts are represented by default labels created as translation strings. As these are derived from account data based on user settings and require an intl provider, they are not saved in the store and therefore are not included in the search).

Description

Include default labels in subject of account search, consolidate and refactor related code.

Related Issue

Resolve #11383

Screenshots:

[MiroslavProchazka]

Hello @mempoolboy ,

Thank you for your contribution!
I've asked my colleagues to do a code review. Once this will be done, I will create a separate PR from your source code in our repository to run the pipelines. Then we can merge this.

Thanks

[Lemonexe]

I love this improvement, we were missing it, thank you! 🎉
Codewise looking good, I have only small codestyle nits 🙂
Tested locally rebased on develop, and works as expected ✔️

Note: there is conflict in one file, but it's trivial, so no need to re-review after solving it.

[Lemonexe]

Nit: I know this line was copied as is from the old function, but we could make it neater 🙂
I'd extract index+1 as a variable, could be named for example displayedAccountNumber or something, then comment is not needed. Code that explains itself is preferred over commentary, but it's a tiny thing 🙂

[Lemonexe]

thanks for renaming the returned callback, the old name didn't make much sense 👍

[Lemonexe]

Nit: I think GetDefaultAccountLabelParams would be more suitable, because this isn't type of the function, just its params.

[Lemonexe]

Nit: this is imho more semantic - although it's longer, it makes it clear we're not checking for truthiness, but if the value exists on the record at all. Though this code is also fine 👌

Suggested change

	accountLabels[key] || getDefaultAccountLabel({ accountType, symbol, index });
	accountLabels.hasOwnProperty(key) ? accountLabels[key] : getDefaultAccountLabel({ accountType, symbol, index });

[mempoolboy]

I really like this solution. 🤩

[Lemonexe]

💭 I'd add a test case for this at accountUtils.test.ts#L217.
Although the accountSearchFn function is itself unchanged, its usage has slightly changed. There is now no test case to match substring with supplied label, only whole string.
This will test it, and at the same time it'll help describe what is the fn typically used for 🙂

expect(accountSearchFn(btcAcc, '#1', undefined, 'Bitcoin #1')).toBe(true);

[mempoolboy]

I love this improvement, we were missing it, thank you! 🎉 Codewise looking good, I have only small codestyle nits 🙂 Tested locally rebased on develop, and works as expected ✔️

Note: there is conflict in one file, but it's trivial, so no need to re-review after solving it.

Thanks for the thorough CR, it's much needed and appreciated. 😃

I've rebased the branch, resolved the conflict, addressed the issues and added the test scenario (well, I've rebased the branch and resolved the conflict 😄).