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 team names of length 36 to accommodate UUIDs #31241

Closed
tobiasbp opened this issue Jun 4, 2024 · 24 comments
Closed

Allow team names of length 36 to accommodate UUIDs #31241

tobiasbp opened this issue Jun 4, 2024 · 24 comments
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@tobiasbp
Copy link
Contributor

tobiasbp commented Jun 4, 2024

Feature Description

When integrating Gitea with other systems, I have the need to create organization teams the correspond to an external resource. As there is no way to tag a team, I would like to name the with a UUID that matches the UUID of my external resource. Users will never see the team names, so there are no UI issues with users wondering about the UUID names for teams.

Currently, the maximum length of a team name is 30 characters. I would like to have that changed to 36, which is the length of a UUID (32 hex characters and 4 hyphens).

I'm happy to make a PR with the change, if someone could point to the current definition in the code. I am unable to find it.

Screenshots

image

@tobiasbp tobiasbp added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Jun 4, 2024
@KN4CK3R
Copy link
Member

KN4CK3R commented Jun 4, 2024

We could/should increase the length even more.

@silverwind
Copy link
Member

What is the maximum team length name on GitHub/GitLab?

@KN4CK3R
Copy link
Member

KN4CK3R commented Jun 5, 2024

Looks like they don't have a limit. These are 200 a:
grafik

@philippbeckmann
Copy link
Contributor

philippbeckmann commented Jun 5, 2024

This collection implies the max length on GitHub is 255: https://github.com/dead-claudia/github-limits

@delvh
Copy link
Member

delvh commented Jun 5, 2024

I'm fine with anything below 255, so if there's a PR to change it, I'll approve it.

@silverwind
Copy link
Member

I'm fine with anything below 255, so if there's a PR to change it, I'll approve it.

you likely meant "at or below" 😛

@tobiasbp
Copy link
Contributor Author

tobiasbp commented Jun 6, 2024

Where in the code should the change be made?

@delvh
Copy link
Member

delvh commented Jun 6, 2024

modules/structs/org_team.go
27:     Name                    string `json:"name" binding:"Required;AlphaDashDot;MaxSize(30)"`
43:     Name                    string  `json:"name" binding:"AlphaDashDot;MaxSize(30)"`

I just had a look at the data model (/models/organization/team.go#79), and it looks like that already supports chars up to length 255.
So no need to change anything else

@silverwind
Copy link
Member

silverwind commented Jun 6, 2024

I assume xorm would auto-migrate the column to the new size, right? If yes, then it is just a change in those structs that is ideally accompanied by a small test that creates a team at the new maximum length.

@techknowlogick
Copy link
Member

@silverwind we require a migration for struct changes that would affect the DB, because even though automigrations will happen, we've run into cases in the past with too large of a version jump in the migration, then some auto-assumed migrations would be skipped (I haven't looked at this specific case though if it'd need the migration/struct change, but from above it seems it is just the validation that needs to be updated).

@delvh
Copy link
Member

delvh commented Jun 6, 2024

I assume xorm would auto-migrate the column to the new size, right

As I said, not needed, it's already varchar 255

@tobiasbp
Copy link
Contributor Author

So, is the only thing needed then to change these values to 255?

modules/structs/org_team.go
27:     Name                    string `json:"name" binding:"Required;AlphaDashDot;MaxSize(30)"`
43:     Name                    string  `json:"name" binding:"AlphaDashDot;MaxSize(30)"`

@techknowlogick
Copy link
Member

@tobiasbp yes, although I'd suggest making the change locally, compiling, and testing to see if the names can then handle the additional length (in case we are missing another validation area).
I would recommend against bumping the length all the way up to 255 as some filesystems don't like really long paths.

@delvh
Copy link
Member

delvh commented Jun 10, 2024

So, let's say 63?

@silverwind
Copy link
Member

@tobiasbp yes, although I'd suggest making the change locally, compiling, and testing to see if the names can then handle the additional length (in case we are missing another validation area). I would recommend against bumping the length all the way up to 255 as some filesystems don't like really long paths.

255 is fine for all common file systems: https://github.com/sindresorhus/valid-filename/blob/main/index.js

@techknowlogick
Copy link
Member

@silverwind the folder length is not enough to look at, some FS (in mobile so I can't link) look at the full path as well. I want to say it's exFAT, but it might be another (common one).
I ran into it on my synology, and iirc that's using some btrfs derivative (caveat, I was using an encrypted drive which added additional constraints)

@silverwind
Copy link
Member

silverwind commented Jun 11, 2024

@tobiasbp yes, although I'd suggest making the change locally, compiling, and testing to see if the names can then handle the additional length (in case we are missing another validation area). I would recommend against bumping the length all the way up to 255 as some filesystems don't like really long paths.

255 is fine for all common file systems: https://github.com/sindresorhus/valid-filename/blob/main/index.js

Thought Windows does have a restriction called MAX_PATH that limits the full path length to 260 chars. I don't think team names can end up as filenames but for stuff that can (org name, repo name), I think 64 is a good limit.

@tobiasbp
Copy link
Contributor Author

tobiasbp commented Jun 13, 2024

So... What do we want?

  • 👍 63
  • ❤️ 127
  • 🎉 255

@KN4CK3R
Copy link
Member

KN4CK3R commented Jun 13, 2024

Maoam and 255.

@silverwind
Copy link
Member

255, to match GitHub and because team names will likely never represent in the file system so things like MAX_PATH are not an issue.

@techknowlogick
Copy link
Member

Ah, sorry, yes, my bad. I was thinking org name, even though you all have been saying team name this entire time. Please ignore my previous concerns. 255 is definitely a good choice for team name.

@silverwind
Copy link
Member

Repo and org name limitations need to be investigated separately, I posted some regexes in #4150 (comment), but there is also a length limit.

@tobiasbp
Copy link
Contributor Author

PR to change maximum length of team names to 255 characters: #31410

@techknowlogick
Copy link
Member

closing due to merged PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

6 participants