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

bug: empty username picked when first name and last name have more than 10 chars #50

Closed
nekomeowww opened this issue May 6, 2023 · 9 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@nekomeowww
Copy link
Owner

nekomeowww commented May 6, 2023

fix pkg/tgbot/utils.go

@nekomeowww nekomeowww added bug Something isn't working good first issue Good for newcomers labels May 6, 2023
@rafiramadhana
Copy link
Contributor

rafiramadhana commented May 6, 2023

@nekomeowww Is this fix related to FullNameFromFirstAndLastName?

func FullNameFromFirstAndLastName(firstName, lastName string) string {

In addition, would you mind to provide an example of input (i.e. the first name and last name with >10 chars) related to this case?

Lastly, what are the expectation? Trimming the output?

Thanks!

@nekomeowww
Copy link
Owner Author

nekomeowww commented May 6, 2023

Hey, welcome to this repo and contribute!

@nekomeowww Is this fix related to FullNameFromFirstAndLastName?

func FullNameFromFirstAndLastName(firstName, lastName string) string {

I checked the source code and find out I miss stated the package that needs to be fixed. The correct one should be:

func formatFullNameAndUsername(fullName, username string) string {

In addition, would you mind to provide an example of input (i.e. the first name and last name with >10 chars) related to this case?

The related edge cases should be the following:

Case One

Over 10 chars of first_name but empty last name and empty username.

{
  "first_name": "blah blah | SomeSubName :> | 😄",
  "last_name": "",
  "username": ""
}

Case Two

Over 10 chars of first_name but with only empty last name.

{
  "first_name": "blah blah | SomeSubName :> | 😄",
  "last_name": "",
  "username": "example_username"
}

This function is used to match the very long name I have seen in Telegram initially, some of the people would use the blah blah | SomeSubName :> | 😄 as their first name (or with last name combined), usually they would have a shorter username that would save more tokens (which is the case 2).
However, I encountered one people with simple Example Name first name but without username being set, so the function would simple return a non-exist username and break the context.

Lastly, what are the expectation? Trimming the output?

The answer to this question is: probably yes?

You might have to come up a idea about how to trim the very long first name in case 1 while still covering case 2.

@rafiramadhana
Copy link
Contributor

@nekomeowww what if full name (first name + last name) is over 10 chars and username is longer than full name?

something like:

first_name: aaaaa
last_name: bbbbbc
username: aaaaabbbbbccccc

are we still returning the username?

usually they would have a shorter username that would save more tokens

because it looks like you prefer to return shorter string here

@nekomeowww
Copy link
Owner Author

@nekomeowww what if full name (first name + last name) is over 10 chars and username is longer than full name?

something like:

first_name: aaaaa
last_name: bbbbbc
username: aaaaabbbbbccccc

are we still returning the username?

usually they would have a shorter username that would save more tokens

because it looks like you prefer to return shorter string here

Yes, it is better to have the less one. Therefore it is not strictly to say we have to return username or fullname, it is ok to return the shorter one directly. If the returned one is still to long (such as 20 runes), we might have to trim the string off.

@rafiramadhana
Copy link
Contributor

Therefore it is not strictly to say we have to return username or fullname

OK. I think it is clear enough.

@rafiramadhana
Copy link
Contributor

@nekomeowww Would it be OK if we remove spaces from the fullname?

"my name has spaces" -> "mynamehasspaces"

@nekomeowww
Copy link
Owner Author

@nekomeowww Would it be OK if we remove spaces from the fullname?


"my name has spaces" -> "mynamehasspaces"

If you are talking about the space between first name and last name, pkg/bots/tgbot.FullNameFromFirstAndLastName should have done the jobs for you. If you are talking about the spaces within the full name, I think it is not ok, because the full name will eventually be displayed in messages.

@nekomeowww
Copy link
Owner Author

How about just return the full name if it exceeded the 10 char limits and username is empty for now?

There are planning features (#47, #48) to process the user info more elegantly. We can discover and come up with more better ideas to improve the processing logics for those issues.

@rafiramadhana
Copy link
Contributor

@nekomeowww Sounds good to me. We can implement it that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants