-
Notifications
You must be signed in to change notification settings - Fork 924
fix(site): show error on duplicate template rename attempt #15348
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @ethanndickson and the rest of your teammates on |
3e07a6c
to
35b9beb
Compare
35b9beb
to
6755b76
Compare
coderd/templates.go
Outdated
httpapi.InternalServerError(rw, err) | ||
if database.IsUniqueViolation(err) { | ||
httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ | ||
Message: fmt.Sprintf("Template %q already exists.", req.Name), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely out of scope of this PR, but we should develop some centralised mapping of database violations to human readable error messages (codersdk.Response
?).
6755b76
to
e54c6a2
Compare
@ethanndickson the PR looks good but I'm missing tests for FE and BE |
e54c6a2
to
f05ef70
Compare
if !dbtestutil.WillUsePostgres() { | ||
t.Skip("This test requires Postgres constraints") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dbmem
is getting removed in the near future, where this'll get removed too.
f05ef70
to
f277a27
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, ty!
Fixes #15311.