Skip to content

feat: expose owner_name in coder_workspace resource #11639

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.

Sign up for GitHub

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

Merged
merged 18 commits into from
Jan 17, 2024
Merged

feat: expose owner_name in coder_workspace resource #11639

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
6653b24
Update go.mod
mtojek Jan 16, 2024
2b239ab
Proto changes
mtojek Jan 16, 2024
7fd0577
WIP
mtojek Jan 16, 2024
2defb89
make gen
mtojek Jan 16, 2024
06208e5
Fix TODO
mtojek Jan 16, 2024
e0d8551
UpdateUserProfile name
mtojek Jan 16, 2024
e3565e6
Patching frontend
mtojek Jan 16, 2024
e6c23aa
name not confidential
mtojek Jan 16, 2024
7df71de
find and replace
mtojek Jan 16, 2024
afced2a
Fix golden
mtojek Jan 16, 2024
8e6f929
Fix ts
mtojek Jan 16, 2024
97dfbcf
Merge branch 'main' into 9883-add-owner-to-workspace
mtojek Jan 17, 2024
4d8934d
bad linting
mtojek Jan 17, 2024
15367f0
Unit test
mtojek Jan 17, 2024
891705f
Validate real name
mtojek Jan 17, 2024
f3c0c42
Fix
mtojek Jan 17, 2024
d116566
Address PR comments
mtojek Jan 17, 2024
2bc609e
Fix strings.TrimSpace
mtojek Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address PR comments
  • Loading branch information
@mtojek
mtojek committed Jan 17, 2024
commit d1165664a1a7fce9990db1c44d054971e15c109c
4 changes: 2 additions & 2 deletions coderd/httpapi/name.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ func TemplateDisplayNameValid(str string) error {

// UserRealNameValid returns whether the input string is a valid real user name.
func UserRealNameValid(str string) error {
if len(str) > 64 {
return xerrors.New("must be <= 64 characters")
if len(str) > 128 {
Copy link
Member

Choose a reason for hiding this comment

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

What's the motivation for this limit? Hubert will be sad: https://www.guinnessworldrecords.com/world-records/67285-longest-personal-name

I think it's unlikely we'll hit this, but arbitrary limits have a way of making people sad (think email regex). We should at least document this decision a bit better. Should length also be enforced in the DB side?

PS. If we're thinking about frontend, they could always use ellipse over there.

Copy link
Member

Choose a reason for hiding this comment

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

I would expect we don't want to allow someone to fill up the database with a name that's gigabytes in size.

Copy link
Member

@mafredri mafredri Jan 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I understand that risk could be a concern, I was specifically thinking about this limit (128), though. Seems on the smaller to just be a safety-net. And if we're really worried about this case, I do think database limits would be a good idea, otherwise we'll always run the risk of introducing new holes. And I'd be surprised if we didn't have some gaps in this defense elsewhere currently as well 😄.

Copy link
Member Author

Choose a reason for hiding this comment

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

The form Mr. Hubert used, HUBERT BLAINE WOLFE­SCHLEGEL­STEIN­HAUSEN­BERGER­DORFF SR, is within the limit!

The original use case for this feature is to populate all places where UI or CLI can render it, like Git commiter/message. I'm sure that Hubert Blain Wolf... will crash UIs in many different places, so I would stick to the artificial limit.

Anyway, we can always disable the limit. The other way round might be hard to do as we might need to chop some names...

I do think database limits would be a good idea, otherwise we'll always run the risk of introducing new holes. And I'd be surprised if we didn't have some gaps in this defense elsewhere currently as well 😄.

This is valid risk, and I wouldn't like to enable another gate to pump bytes into the database.

return xerrors.New("must be <= 128 characters")
}

trimmed := strings.TrimSpace(str)
Expand Down
4 changes: 4 additions & 0 deletions coderd/httpapi/name_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,10 @@ func TestUserRealNameValid(t *testing.T) {
{"Mr Bean", true},
{"Severus Snape", true},
{"Prof. Albus Percival Wulfric Brian Dumbledore", true},
{"Pablo Diego José Francisco de Paula Juan Nepomuceno María de los Remedios Cipriano de la Santísima Trinidad Ruiz y Picasso", true},
{"Hector Ó hEochagáin", true},
{"Małgorzata Kalinowska-Iszkowska", true},
{"成龍", true},
{". .", true},
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a smiley, I approve! 👀


{"Lord Voldemort ", false},
Expand Down