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

fix: account name checks FE-724 #1570

Merged
merged 7 commits into from
Oct 11, 2024

Conversation

nelitow
Copy link
Contributor

@nelitow nelitow commented Oct 9, 2024

Fixes:

  • User not being able to change the account name to case-sensitive variations of the same word.
    • Example: Changing a case typo, like Fuel wAllet -> Fuel Wallet was not possible
  • User not being able to change or create accounts that are substring of eachother.
    • Example: Changing Fuel Wallet -> Wallet was not possible

Considerations:

  • The filterByName function sound like it could be used for a search filter, in which it should be case insensitive, but that is not the case at the moment, it is only used for account name editing and creation.
  • Users can now create multiple accounts with similar names, which can cause confusion, changing only a letter case, so:
    • Account 1 and account 1 can now coexist as account names

UPDATE b611c8b
Addressing an issue that arose as the function was used for account searching inside the wallet by another method, so now the account addition and editing don't rely on the same function. That was caught by e2e tests.

helciofranco
helciofranco previously approved these changes Oct 9, 2024
Copy link
Member

@helciofranco helciofranco left a comment

Choose a reason for hiding this comment

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

LGTM, your consideration about name conflicts is fine.

for reference, check rabby wallet, they allow even same exact name:

Rabby Wallet
Screenshot 2024-10-09 at 10 35 43

@helciofranco
Copy link
Member

it's missing to add a changeset @nelitow

pnpm changeset

helciofranco
helciofranco previously approved these changes Oct 9, 2024
Copy link
Contributor

@LuizAsFight LuizAsFight left a comment

Choose a reason for hiding this comment

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

this broke the test that validates connected screen flow.

the comparison before was done with includes and now it's done with === what was the reason for changing this? the issue was to fix the lowerCase only

@LuizAsFight LuizAsFight merged commit 8e42400 into master Oct 11, 2024
14 checks passed
@LuizAsFight LuizAsFight deleted the nj/fix/lowercase-acc-name-comparison branch October 11, 2024 03:39
LuizAsFight pushed a commit that referenced this pull request Oct 11, 2024
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to master, this PR
will be updated.


# Releases
## @fuels/[email protected]



## [email protected]

### Minor Changes

- [#1572](#1572)
[`97185130`](9718513)
Thanks [@helciofranco](https://github.com/helciofranco)! - Reduce
transaction history loading time by optimizing the complexity of the
page info query.

- [#1571](#1571)
[`112e002c`](112e002)
Thanks [@helciofranco](https://github.com/helciofranco)! - Format tiny,
large, and regular amounts, applying 6 decimal places of precision.

### Patch Changes

- [#1570](#1570)
[`8e42400c`](8e42400)
Thanks [@nelitow](https://github.com/nelitow)! - Improve account name
colision verification

-   Updated dependencies \[]:
    -   @fuel-wallet/[email protected]

## @fuel-wallet/[email protected]

### Minor Changes

- [#1571](#1571)
[`112e002c`](112e002)
Thanks [@helciofranco](https://github.com/helciofranco)! - Format tiny,
large, and regular amounts, applying 6 decimal places of precision.

## @fuel-wallet/[email protected]

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Account name case sensitivity
3 participants