-
Notifications
You must be signed in to change notification settings - Fork 518
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
Add new feature to count messages by date #272
Add new feature to count messages by date #272
Conversation
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 great @WLSF! just one small request to give the methods slightly more descriptive names 😄
lib/chat_api/reporting.ex
Outdated
@@ -31,6 +31,20 @@ defmodule ChatApi.Reporting do | |||
|> Repo.all() | |||
end | |||
|
|||
def count_sent_messages() do |
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.
let's rename this to count_sent_messages_by_date
, so that it's clearer what the output looks like :)
(otherwise it sounds like this method returns a number)
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.
hmm, makes sense! thanks!
lib/chat_api/reporting.ex
Outdated
|> Repo.all() | ||
end | ||
|
||
def count_received_messages() do |
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.
same as above, let's just add _by_date
to the end of this method's name to make it clear what's going on 👍
%{date: ~D[2020-09-02], count: 1}, | ||
%{date: ~D[2020-09-03], count: 1} | ||
] = Reporting.count_received_messages() | ||
end |
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.
hooray for tests! 🎉
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, thanks for the quick turnaround on this! 🚀
let me know if there are any other tickets you'd be interested in tackling @WLSF 😁 |
Description
Added 2 new functions on
reporting.ex
:Issue
#255
Screenshots
N/A updates on frontend
Checklist
mix test
mix format