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: remove unused attempt for account names counter #600

Merged
merged 6 commits into from
Dec 12, 2023

Conversation

janmichek
Copy link
Collaborator

Description

Initially I grabbed the issue #553
Found out that

  1. Such an endpoint does not exist (requested here Add counter for account (active) names ae_mdw#1629)
  2. There was a wrong attempt to get names count, that was not even implemented (dead code) SO at least I removed the dead code

Demo

Without any visual change. Should work the same

Checklist:

  • I have read and followed the Contributing Guide
  • remove attempt to get names count manually

@janmichek janmichek self-assigned this Nov 22, 2023
Copy link

Copy link
Collaborator

@Liubov-crypto Liubov-crypto left a comment

Choose a reason for hiding this comment

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

As I understood the name counter isn't implemented yet on middleware's side. Besides that LGTM.

But I have a question: I noticed that on other pr name counter is working correctly and showing number of names. @janmichek Correct me if I'm wrong:

2023-11-22.6.48.42.mov

@janmichek
Copy link
Collaborator Author

@Liubov-crypto
Good catch! There was a bug in app.
The count of names was taken from paginated response so it was always maximum of 10.

firefox_qmAx5nx3Bq.mp4

I removed that field for now, until we resolve it and will be able to show correct number.
Followup here #553

@Liubov-crypto
Copy link
Collaborator

LGTM. AENS Names counter has been removed.

Copy link
Collaborator

@michele-franchi michele-franchi left a comment

Choose a reason for hiding this comment

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

Makes sense, it was counting first page only.

@janmichek janmichek merged commit 365b494 into develop Dec 12, 2023
2 checks passed
@janmichek janmichek deleted the Add-names-counter-in-the-account-details-page branch December 12, 2023 09:58
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.

3 participants