-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Backend: Add API endpoint for photo count per month #218
Backend: Add API endpoint for photo count per month #218
Conversation
I can merge this, but note that we'll probably change the URL in something like "/moments/time" as we also need it for the Web UI... additionally grouped by country, maybe like "/moments/country". Not sure where I have the query, but I've prepared it recently. |
|
||
result, err := q.GetPhotoCountPerMonth() | ||
if err != nil { | ||
c.AbortWithStatusJSON(400, gin.H{"error": txt.UcFirst(err.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.
Use the http.* constant instead of an integer
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.
good catch, would http.StatusInternalServerError
be a good choice?
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.
Maybe yes, since this should never happen? Otherwise StatusBadRequest would be OK.
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.
✔️
|
||
// GET /api/v1/photoCountPerMonth | ||
func GetPhotoCountPerMonth(router *gin.RouterGroup, conf *config.Config) { | ||
router.GET("/photoCountPerMonth", func(c *gin.Context) { |
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.
Use "/moments/time" as URL
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.
✔️
q = q.Table("photos"). | ||
Select("photos.photo_year, photos.photo_month, COUNT(*) AS count"). | ||
Group("photos.photo_year, photos.photo_month"). | ||
Order("photos.photo_year DESC, photos.photo_month DESC") |
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 good, but you might want to ignore deleted / archived photos for this?
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.
✔️
) | ||
|
||
// GET /api/v1/moments/time | ||
func GetPhotoCountPerMonth(router *gin.RouterGroup, conf *config.Config) { |
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.
Can you also rename this to GetMomentsTime()
? Looks good otherwise, probably need to add fixtures to properly test this with unit tests... we can do this later, so that you can focus on developing our mobile app :)
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.
sound good 👍
Cool, will be merged as soon as Travis CI is green 🥝 |
Also added you and @spaibs as team members to our org as you contribute a lot and maintain the mobile app. |
This new endpoint allows the mobile app to have a draggable scrollbar without loading the metadata for all photos.