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

feat: ListUsersByPrefix and ListJsonUsersByPrefix #35

Merged
merged 3 commits into from
Mar 7, 2024
Merged

feat: ListUsersByPrefix and ListJsonUsersByPrefix #35

merged 3 commits into from
Mar 7, 2024

Conversation

jefft0
Copy link
Contributor

@jefft0 jefft0 commented Mar 7, 2024

The GnoSocial demo app needs to search by username. This PR has three commits:

  1. Add persistent variable gUserAddressByName. Add a centralized utility function getOrCreateUserPosts which creates a new UserPosts if needed and adds it to gUserPostsByAddress as well as updating gUserAddressByName. Change public functions like PostMessage to use getOrCreateUserPosts. Also, we can simplify Render for the main page by listing all usernames directly from gUserAddressByName.
  2. Add public function ListUsersByPrefix. (This uses a temporary local copy of ListKeysByPrefix which can be removed when the PR is merged to add this to p/demo/avlhelpers.) Also, add ListJsonUsersByPrefix which returns the same result a JSON array.
  3. Remove GetUsers and GetJsonUsers (which return all users and are not scalable). The app can use ListJsonUsersByPrefix instead.

@jefft0 jefft0 requested a review from D4ryl00 March 7, 2024 10:30
Copy link
Contributor

@D4ryl00 D4ryl00 left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +26 to +27
// (The caller usually has already called usernameOf to get the username, but if
// it is "" then this will get it.)
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't understand this comment :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function has an argument for the user address, but also an argument for the username. We want to save CPU cycles. In all the cases where code needs to call this function, it has already looked up the user name by user address.
https://github.com/gnolang/gnosocial/blob/1c86452df7b8f9400863386b98edeaa3c7c12aa9/realm/public.gno#L70
So it should not be necessary for this function to look it up again. The comment is justifying why this function has a username argument. For completeness, this function will look up the username by user address, but only if the caller doesn't provide the username.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think that the comment needs to be updated?

Copy link
Contributor

Choose a reason for hiding this comment

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

In the next PR ;)

@jefft0 jefft0 changed the title feat: add list users by prefix ListJsonUsersByPrefix feat: ListUsersByPrefix and ListJsonUsersByPrefix Mar 7, 2024
@jefft0 jefft0 merged commit 5150c73 into gnoverse:main Mar 7, 2024
@jefft0 jefft0 deleted the feat/add-ListUsersByPrefix branch March 7, 2024 16:38
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