-
Notifications
You must be signed in to change notification settings - Fork 876
fix: /projects endpoint returning null instead of empty array #140
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
Codecov Report
@@ Coverage Diff @@
## main #140 +/- ##
==========================================
+ Coverage 68.15% 68.24% +0.09%
==========================================
Files 111 111
Lines 6004 6003 -1
Branches 68 67 -1
==========================================
+ Hits 4092 4097 +5
+ Misses 1521 1515 -6
Partials 391 391
Continue to review full report at Codecov.
|
coderd/projects.go
Outdated
@@ -49,6 +49,10 @@ func (api *api) projects(rw http.ResponseWriter, r *http.Request) { | |||
}) | |||
return | |||
} | |||
// Don't return 'null' | |||
if projects == nil { | |||
projects = []database.Project{} |
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.
I'd rather us to this inside the sql.ErrNoRows
like before, and depend on tests to catch the incorrect behavior.
I'm worried about a developer making a mistake for something else, and this masking the error.
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.
Looks like this change is also already in main
with #166 🎉
This turns out to be much simpler now since the BE fixes for |
While working on the projects page, I noticed the
/projects
endpoint would return 'null' instead of an empty array. An empty array would simplify the behavior on the client page.There were already tests in place for this, but they only validated that the item's length is 0... and it turns out
len(nil)
is also0
.So this does a couple things:
NotNil
as well/projects
page for populating data, as it is no longer needed