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

Allow for username max length to be 50 chars instead of 20, 20 is too limiting #1263

Closed
McFlat opened this issue Oct 13, 2018 · 8 comments · Fixed by #1265
Closed

Allow for username max length to be 50 chars instead of 20, 20 is too limiting #1263

McFlat opened this issue Oct 13, 2018 · 8 comments · Fixed by #1265

Comments

@McFlat
Copy link
Contributor

McFlat commented Oct 13, 2018

We need to allow longer usernames. Youtube api version 1 had shorter usernames, I think it was 20 chars, then in v3 they made the usernames to be 52 or 54 chars(not sure right now, I'll have to find that stackoverflow discussion again if you insist).

Maximum length of a Facebook username is currently 50 characters.

I reported this issue before, but it disappeared into thin air, so I'm bringing it up again because it's still an issue. I will make a pull request sometime today to make the username max length 50 chars.

However 64 chars would be better, just because Google and Facebook set a limit of 50, doesn't mean we have to set their exact limit. We are in fact creating this project because youtube is too limiting, if I'm not mistaken.

@McFlat
Copy link
Contributor Author

McFlat commented Oct 13, 2018

When a username is too long below the videos it just shows the three dots ..., so no big deal, no good reason for us to set such a short limit. I'll make it 50 because it's already kind long at 50, but in like 5 years we can talk about this again to see if maybe we should set it to 64. Also we should allow underscores _ in the username, for readability in case some users want them. looks like the pattern allows dots and underscores in the username as it is but for some reason there's a bug Validators.pattern(/^[a-z0-9._]+$/) which doesn't allow it on the frontend. In the database all these fields are already VARCHAR(255) so there's no need to create a migration for this.

@McFlat
Copy link
Contributor Author

McFlat commented Oct 13, 2018

Since I'm updating username to be longer I will also update channelName to be longer too at 50 chars.

@McFlat
Copy link
Contributor Author

McFlat commented Oct 13, 2018

Before I tried to make it so the admin can set the max length for the usernames in the config, but really that's not needed, what is better is to just increase the limit to 50 and all instances can have the same max limit of 50

@McFlat
Copy link
Contributor Author

McFlat commented Oct 13, 2018

For when the username gets cut off with the ... we can make it show the full username on hover.

@McFlat McFlat changed the title Allow for username max length to be 50 or 64 chars instead of 20, 20 is too limiting Allow for username max length to be 50 chars instead of 20, 20 is too limiting Oct 13, 2018
@McFlat
Copy link
Contributor Author

McFlat commented Oct 13, 2018

We should also make the minimum to be a single char, not sure why 3 is the minimum, but twitter allows a single char for a username, so long as it starts with a letter and not a _ or a . or a -. We should also allow uppercase in the usernames and dots, underscores and dashes. So users can have camelCase usernames if they want, or separated by underscores or dashes or dots.

Validators.pattern(/^[a-zA-Z0-9]+[a-zA-Z0-9-._]+$/)

A good place to validate this regex online is https://regexr.com/

@McFlat
Copy link
Contributor Author

McFlat commented Oct 13, 2018

I've created another issues about field min/max lengths in regards to the user_ban_reason #1264

@McFlat
Copy link
Contributor Author

McFlat commented Oct 13, 2018

In the constants we have channel_name max 120, but on the frontend we have 20. In that case I will just make it 50 just like the username and make the display name also 50, considering that youtube sets a limit of 30 on the channel title.

McFlat added a commit to McFlat/PeerTube that referenced this issue Oct 14, 2018
…ax length 50; still some bug on the frontend complains but if you remove the disabled property it creates the account just fine (fixes Chocobozzz#1263)
@McFlat
Copy link
Contributor Author

McFlat commented Oct 14, 2018

pull request created to resolve this issue #1265

McFlat added a commit to McFlat/PeerTube that referenced this issue Oct 15, 2018
…ax length 50; (fixes Chocobozzz#1263)

    ! still some bug on the frontend complains but if you remove the disabled property it creates the account just fine
McFlat added a commit to McFlat/PeerTube that referenced this issue Oct 15, 2018
…ax length 50; (fixes Chocobozzz#1263)

    ! still some bug on the frontend complains but if you remove the disabled property it creates the account just fine;
allow for usernames to start with a number
McFlat added a commit to McFlat/PeerTube that referenced this issue Oct 15, 2018
…ax length 50; (fixes Chocobozzz#1263)

    ! still some bug on the frontend complains but if you remove the disabled property it creates the account just fine;
allow for usernames to start with a number;
fix test, since username can be 1 char now make test check empty
McFlat added a commit to McFlat/PeerTube that referenced this issue Oct 15, 2018
…ax length 50; (fixes Chocobozzz#1263)

    ! still some bug on the frontend complains but if you remove the disabled property it creates the account just fine;
allow for usernames to start with a number;
fix test, since username can be 1 char now make test check empty;
fix test, Should fail with a too long username
McFlat added a commit to McFlat/PeerTube that referenced this issue Oct 15, 2018
…ax length 50; (fixes Chocobozzz#1263);

    ! still some bug on the frontend complains but if you remove the disabled property it creates the account just fine;
allow for usernames to start with a number;
fix test, since username can be 1 char now make test check empty;
fix test, Should fail with a too long username;
fix test, Should fail with a too small username;
McFlat added a commit to McFlat/PeerTube that referenced this issue Oct 15, 2018
…ax length 50; (fixes Chocobozzz#1263);

    ! still some bug on the frontend complains but if you remove the disabled property it creates the account just fine;
allow for usernames to start with a number;
fix test, since username can be 1 char now make test check empty;
fix test, Should fail with a too long username;
fix test, Should fail with a too small username
McFlat added a commit to McFlat/PeerTube that referenced this issue Oct 15, 2018
…ax length 50; (fixes Chocobozzz#1263);

    ! still some bug on the frontend complains but if you remove the disabled property it creates the account just fine;
allow for usernames to start with a number;
fix test, since username can be 1 char now make test check empty;
fix test, Should fail with a too long username;
fix test, Should fail with a too small username;
McFlat added a commit to McFlat/PeerTube that referenced this issue Oct 16, 2018
…ax length 50; (fixes Chocobozzz#1263);

    ! still some bug on the frontend complains but if you remove the disabled property it creates the account just fine;
allow for usernames to start with a number;
fix test, since username can be 1 char now make test check empty;
fix test, Should fail with a too long username;
fix test, Should fail with a too small username;
fix regular expression for username and videoChannel;
McFlat added a commit to McFlat/PeerTube that referenced this issue Oct 17, 2018
…ax length 50; (fixes Chocobozzz#1263);

    ! still some bug on the frontend complains but if you remove the disabled property it creates the account just fine;
allow for usernames to start with a number;
fix test, since username can be 1 char now make test check empty;
fix test, Should fail with a too long username;
fix test, Should fail with a too small username;
fix regular expression for username and videoChannel;
change username, videoChannel to be lowercase and fix message;
McFlat added a commit to McFlat/PeerTube that referenced this issue Oct 18, 2018
…ax length 50; (fixes Chocobozzz#1263);

    ! still some bug on the frontend complains but if you remove the disabled property it creates the account just fine;
allow for usernames to start with a number;
fix test, since username can be 1 char now make test check empty;
fix test, Should fail with a too long username;
fix test, Should fail with a too small username;
fix regular expression for username and videoChannel;
change username, videoChannel to be lowercase and fix message
McFlat added a commit to McFlat/PeerTube that referenced this issue Oct 18, 2018
…ax length 50; (fixes Chocobozzz#1263);

    ! still some bug on the frontend complains but if you remove the disabled property it creates the account just fine;
allow for usernames to start with a number;
fix test, since username can be 1 char now make test check empty;
fix test, Should fail with a too long username;
fix test, Should fail with a too small username;
fix regular expression for username and videoChannel;
change username, videoChannel to be lowercase and fix message;
Chocobozzz pushed a commit that referenced this issue Dec 7, 2018
…50 (#1265)

* make username, display_name and video_channel_name min length 1 and max length 50; (fixes #1263);
    ! still some bug on the frontend complains but if you remove the disabled property it creates the account just fine;
allow for usernames to start with a number;
fix test, since username can be 1 char now make test check empty;
fix test, Should fail with a too long username;
fix test, Should fail with a too small username;
fix regular expression for username and videoChannel;
change username, videoChannel to be lowercase and fix message;

* change 1 characters to 1 character
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 a pull request may close this issue.

1 participant