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

[FMS] Add parent categories to dashboard categories view #4700

Merged

Conversation

MorayMySoc
Copy link
Contributor

Add a parent categories column to show if a category is a subcategory.

Arbitrary decision to choose the first parent category from the groups method of a contact.

https://github.com/mysociety/societyworks/issues/1458

[skip changelog]

@MorayMySoc
Copy link
Contributor Author

MorayMySoc commented Nov 1, 2023

I'll need to fix and add tests, but I wanted to check that we were agreed this is what's meant to be achieved

@MorayMySoc MorayMySoc marked this pull request as draft November 1, 2023 15:33
@MorayMySoc MorayMySoc requested a review from davea November 1, 2023 15:33
Copy link

codecov bot commented Nov 1, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6ecf711) 0.00% compared to head (b986a12) 85.47%.

❗ Current head b986a12 differs from pull request most recent head 5b34f3c. Consider uploading reports for the commit 5b34f3c to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4700       +/-   ##
===========================================
+ Coverage        0   85.47%   +85.47%     
===========================================
  Files           0      340      +340     
  Lines           0    25022    +25022     
  Branches        0     4842     +4842     
===========================================
+ Hits            0    21388    +21388     
- Misses          0     2206     +2206     
- Partials        0     1428     +1428     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MorayMySoc
Copy link
Contributor Author

Ok, ultimately copied the wards stuff once I understood what was going on - but as far as I see it needs to have the labels selectable in some way (either by click or input check) to select the whole group. Gone in a bit of a long loop on this, so that's actually probably what the real work of this ticket was!

@mysociety-pusher mysociety-pusher force-pushed the 1458-add-parent-categories-to-dashboard-stats branch from a0b6ba0 to 59165af Compare November 23, 2023 15:42
@mysociety-pusher mysociety-pusher force-pushed the 1458-add-parent-categories-to-dashboard-stats branch 4 times, most recently from f068874 to 4d58ca4 Compare December 4, 2023 18:04
@MorayMySoc MorayMySoc marked this pull request as ready for review December 4, 2023 18:32
@MorayMySoc MorayMySoc removed the request for review from davea December 4, 2023 18:32
@MorayMySoc
Copy link
Contributor Author

MorayMySoc commented Dec 4, 2023

Changes the dropdown for categories in the dashboard stats to be multi-select checkboxes instead of only allowing selecting one category.

Adds group labels to group categories in the 'categories' dropdown menu of the dashboard with an all-group-name checkbox to select all of subcategories for that group.

Some conversation: https://mysociety.slack.com/archives/C01TK8P1K8T/p1700759729180759

https://github.com/mysociety/societyworks/issues/1458

@MorayMySoc MorayMySoc requested review from dracos and chrismytton and removed request for dracos December 4, 2023 18:35
@dracos dracos requested review from dracos and removed request for chrismytton December 5, 2023 09:10
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

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

Looking better, one issue with the template change, and hopeful couple of simplifications :)

perllib/FixMyStreet/Reporting.pm Outdated Show resolved Hide resolved
perllib/FixMyStreet/App/Controller/Dashboard.pm Outdated Show resolved Hide resolved
@MorayMySoc MorayMySoc requested a review from dracos December 5, 2023 14:49
@mysociety-pusher mysociety-pusher force-pushed the 1458-add-parent-categories-to-dashboard-stats branch from 82431d6 to 567c396 Compare December 5, 2023 15:21
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

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

Looking good, just one thing more for me checking I've understood it right :)

perllib/FixMyStreet/Script/CSVExport.pm Outdated Show resolved Hide resolved
@MorayMySoc MorayMySoc requested a review from dracos December 6, 2023 09:15
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

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

Thanks! All seems to work as expected :) I actually just spotted in TfL you also don't need to check the first bit of that condition now as you know it will be an arrayref, but it's fine as is as well.

@MorayMySoc
Copy link
Contributor Author

Sorry to force a re-review but figured I may as well get that done too - but wanted to check I haven't misunderstood and got it wrong either.

@MorayMySoc MorayMySoc requested a review from dracos December 6, 2023 09:38
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

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

No worries, yes, that's what I meant - think ->category is guaranteed to be an arrayref now, which makes things bit neater :)

Switch categories list on dashborad to multi checkbox
rather than single option.

First step towards:

mysociety/societyworks#1458
Add an 'All' subcategories checkbox to select all
the categories in one group

Arbitrary decision to choose the first parent category from
the groups method of a contact.

mysociety/societyworks#1458
@mysociety-pusher mysociety-pusher force-pushed the 1458-add-parent-categories-to-dashboard-stats branch from b986a12 to 5b34f3c Compare December 6, 2023 10:34
@mysociety-pusher mysociety-pusher merged commit 5b34f3c into master Dec 6, 2023
19 of 20 checks passed
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.

3 participants