Implement monthly overview feature#30
Implement monthly overview feature#30VishakhaSainani-Josh wants to merge 1 commit intofeat/leaderboard-servicefrom
Conversation
| ContributionType string `db:"contribution_type"` | ||
| ContributionCount int `db:"contribution_count"` |
There was a problem hiding this comment.
rename to Type and Count
There was a problem hiding this comment.
Also why this struct as db tags?
| response.WriteJson(w, http.StatusOK, "user contributions fetched successfully", userContributions) | ||
| } | ||
|
|
||
| func (h *handler) GetContributionTypeSummaryForMonth(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
Name like ListMonlyContributionSummary would be more preferable
| UpdatedAt time.Time `db:"updated_at"` | ||
| } | ||
|
|
||
| type ContributionTypeSummary struct { |
| month := r.URL.Query().Get("month") | ||
|
|
There was a problem hiding this comment.
validate month before passing it to service layer
|
|
||
| contributionTypeSummaryForMonth, err := h.contributionService.GetContributionTypeSummaryForMonth(ctx, month) | ||
| if err != nil { | ||
| slog.Error("error fetching contribution type summary for month") |
There was a problem hiding this comment.
also print error in the message
| func (s *service) GetContributionTypeSummaryForMonth(ctx context.Context, monthParam string) ([]ContributionTypeSummary, error) { | ||
| month, err := time.Parse("2006-01", monthParam) | ||
| if err != nil { | ||
| slog.Error("error parsing month query parameter", "error", err) |
There was a problem hiding this comment.
In Go programs, printing an error typically indicates that we've handled it. Therefore, it's best practice to either print the error or return it, but not both.
(same for all other)
There was a problem hiding this comment.
to add more detail you can always wrap errors
| return nil, err | ||
| } | ||
|
|
||
| contributionTypes, err := s.contributionRepository.GetAllContributionTypes(ctx, nil) |
There was a problem hiding this comment.
As a practice in go, Use List as prefix and not get whenever fetching an array.
| var contributionTypeSummaryForMonth []ContributionTypeSummary | ||
|
|
||
| for _, contributionType := range contributionTypes { | ||
| contributionTypeSummary, err := s.contributionRepository.GetContributionTypeSummaryForMonth(ctx, nil, contributionType.ContributionType, month) | ||
| if err != nil { | ||
| if errors.Is(err, apperrors.ErrNoContributionForContributionType) { | ||
| contributionTypeSummaryForMonth = append(contributionTypeSummaryForMonth, ContributionTypeSummary{ContributionType: contributionType.ContributionType}) | ||
| continue | ||
| } | ||
| slog.Error("error fetching contribution type summary", "error", err) | ||
| return nil, err | ||
| } | ||
|
|
||
| contributionTypeSummaryForMonth = append(contributionTypeSummaryForMonth, ContributionTypeSummary(contributionTypeSummary)) | ||
| } |
There was a problem hiding this comment.
We could fetch all the Contribution summary for this month in a single db call then categorise them. This would save a lot of db calls.
(This problem is generally known as n + 1 query problem)
| router.HandleFunc("PATCH /api/v1/user/email", middleware.Authentication(deps.UserHandler.UpdateUserEmail, deps.AppCfg)) | ||
|
|
||
| router.HandleFunc("GET /api/v1/user/contributions/all", middleware.Authentication(deps.ContributionHandler.FetchUserContributions, deps.AppCfg)) | ||
| router.HandleFunc("GET /api/v1/user/monthlyoverview", middleware.Authentication(deps.ContributionHandler.GetContributionTypeSummaryForMonth, deps.AppCfg)) |
There was a problem hiding this comment.
With standard naming convention it should be /api/v1/user/overview/monthly or monthly can be query param, like /api/v1/user/overview?type=monthly
| } | ||
|
|
||
| func (cr *contributionRepository) GetContributionTypeSummaryForMonth(ctx context.Context, tx *sqlx.Tx, contributionType string, month time.Time) (ContributionTypeSummary, error) { | ||
| userIdValue := ctx.Value(middleware.UserIdKey) |
There was a problem hiding this comment.
userId should be passed by the service layer, and ctx values are generally accessed in the handler layer itself.
It Makes the function independent of the caller state.
3 participants
Allow user to view monthly overview, where in each contribution type it's monthly total count and coins is shown