-
Notifications
You must be signed in to change notification settings - Fork 1
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 Date Range filtering to Dashboard #503
base: main
Are you sure you want to change the base?
Conversation
7020f53
to
c3fcb07
Compare
@calellowitz @sravfeyn bumping for review. |
flw_amount_paid = sum(sum_total_users.values()) | ||
avg_top_paid_flws = sum(sorted(sum_total_users.values(), reverse=True)[:top_five_percent_len]) | ||
visit_data_dict[group_key].update({"flw_amount_paid": flw_amount_paid, "avg_top_paid_flws": avg_top_paid_flws}) | ||
return list(visit_data_dict.values()) |
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.
This method is getting quite longer, it becomes difficult to read/modify latter. It will add value to refactor it into a sort of class with separate methods for various colulms/queries.
For e.g.
class VisitDataHelper:
def get_data(**filters):
def get_filter_kwargs()
def get_payment_data()
def get_visit_data()
Not exactly like that, but whatever to break it into smaller pieces will improve readability and testability
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 will try to address this in a future PR.
import monthSelectPlugin from 'flatpickr/dist/esm/plugins/monthSelect'; | ||
import 'flatpickr/dist/flatpickr.css'; | ||
import 'flatpickr/dist/plugins/monthSelect/style.css'; | ||
window.monthPickr = (id, flatPickrConfig = {}, monthPluginConfig = {}) => |
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.
Have you tried altering the default style to look more similar to our site? Font-colors, type etc.
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 have not tried updating the colors, since we are going to change the UI soon in the UI refactor.
end_date = make_aware(datetime(year, month or 12, month_end)) | ||
from_date = from_date or now().date() | ||
to_date = to_date if to_date and to_date <= now().date() else now().date() | ||
timeseries = get_month_series(from_date, to_date) |
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.
timeseries
is just months
, right?
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.
Yes, it is a list of month-year combinations between the from and to dates.
Add AVG and MAX time to payment metrics filter by payment date
Product Description
This PR removes the year and month filters and adds support for filtering using custom date ranges.
Technical Summary
Ticket Link
This PR also adds refactors for the way grouping in queryset works.
Safety Assurance
Safety story
Automated test coverage
Labels & Review