Skip to content
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

admin dashboard updates #429

Merged
merged 42 commits into from
Nov 18, 2024
Merged

admin dashboard updates #429

merged 42 commits into from
Nov 18, 2024

Conversation

czue
Copy link
Member

@czue czue commented Nov 12, 2024

New Screenshot:

image

Changes:

  1. Change the current clusters to donuts, which show the approved/pending/rejected breakdown of the visits in that cluster. This is mostly just changing from the previous cluster example to this one: https://docs.mapbox.com/mapbox-gl-js/example/cluster-html/, and then getting it working with our setup.
  2. Add a loading state to the map. And make a few other minor tweaks.
  3. Add some charts. You can view that set of changes here if you want Add charts to the dahboard #431
  4. Migrate most of the JavaScript functions to a new external JS file.

(recommend reviewing all at once)

@czue czue requested a review from calellowitz November 12, 2024 14:57
@czue czue mentioned this pull request Nov 13, 2024
@czue czue changed the title change clusters to donut charts admin dashboard updates Nov 13, 2024
@czue
Copy link
Member Author

czue commented Nov 13, 2024

I don't think the build failure is related, though let me know if I should look into it

Copy link
Collaborator

@calellowitz calellowitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admittedly didn't look too closely at the JS but the rest looks fine. One question

@user_passes_test(lambda u: u.is_superuser)
def dashboard_charts_api(request):
filterset = DashboardFilters(request.GET)
queryset = UserVisit.objects.all()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How confident are you that you want UserVisit rather than CompletedWork (user visits can be combined to form a single completed work, which is the unit of payment). I think we typically view Completed Works as a unit of delivery instead of the form, since it can account for things a visit that includes 2 forms filled out at the same time that together mark a delivery.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's all using the same base queryset which I got from the superset dashboard's SQL, so I think it's right? Or at least, it's consistent with the superset view and itself.

But can definitely revisit this logic once the dashboard has been vetted by the team more.

Copy link
Collaborator

@calellowitz calellowitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't mean to approve and I think "requesting changes" is the only way I can get back to a not approved state, but depending on the answer to the question will approve again.

@czue czue merged commit 7b63bc5 into main Nov 18, 2024
5 checks passed
@czue czue deleted the cz/dashboard-tweaks branch November 18, 2024 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants