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

[Northumberland] Being able to filter on one category in multiple parent categories #4895

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MorayMySoc
Copy link
Contributor

@MorayMySoc MorayMySoc commented Mar 26, 2024

Set up categories in index to include name of their group.

Use this group name to check if a report is in the selected category (through extra->group).

If there are reports that don't identify as being in that category, then still use a 'Multiple' category.

Fixes https://github.com/mysociety/societyworks/issues/4039

[skip changelog]

@MorayMySoc
Copy link
Contributor Author

@dracos Was hoping to get this finished but tests failing so need to look into it further. Hopefully 90% complete unless totally wrong approach so putting as a WIP so can get advice if you have time to look at it to get a rough idea.

@MorayMySoc MorayMySoc requested a review from dracos March 26, 2024 17:30
Copy link

codecov bot commented Mar 26, 2024

Codecov Report

Attention: Patch coverage is 93.87755% with 3 lines in your changes missing coverage. Please review.

Project coverage is 82.35%. Comparing base (1b6535b) to head (2a0b5f0).

Files with missing lines Patch % Lines
perllib/FixMyStreet/App/Controller/Dashboard.pm 93.87% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4895   +/-   ##
=======================================
  Coverage   82.34%   82.35%           
=======================================
  Files         409      409           
  Lines       31845    31876   +31     
  Branches     5063     5071    +8     
=======================================
+ Hits        26222    26250   +28     
- Misses       4126     4127    +1     
- Partials     1497     1499    +2     

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

@mysociety-pusher mysociety-pusher force-pushed the 4039-northumberland-filter-category branch from 927848b to 02ddf58 Compare April 3, 2024 10:48
@MorayMySoc MorayMySoc removed the request for review from dracos April 3, 2024 10:49
@mysociety-pusher mysociety-pusher force-pushed the 4039-northumberland-filter-category branch 3 times, most recently from f9cb3dc to 1cbb8f8 Compare April 3, 2024 11:46
@MorayMySoc MorayMySoc requested a review from dracos April 5, 2024 14:22
@MorayMySoc MorayMySoc marked this pull request as ready for review April 5, 2024 14:23
@mysociety mysociety deleted a comment from lizettal Apr 26, 2024
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.

I first got "Not a HASH reference" from Dashboard.pm line 291 when trying to view the dashboard. I added a && ref $col3 eq 'HASH' there which fixed that (old extra might not be a hash). But I can't then seem to get this to work, I'm afraid?
I have "Street cleaning" in three parent categories, "Parent A", "Parent B", "Parent C". Each has 2 reports in it. I selected the Parent A Street cleaning, hit submit, the URL has category=Street+cleaning-group-Parent+A in it, but all three are ticked in the resultant page and all 6 reports are still shown in the summary table, still under "Multiple". Same as selecting "All Parent A". I think the first part is due to the reverse which means "Street cleaning" is set as the key, just like it was beforehand; and the category filter still seems to have the same information/display.
Given that, I'm afraid I haven't really looked at the display code further on, but I'm finding it quite hard to understand what it's doing, so it might need talking through the best way to proceed. I don't think we want this to be this complicated if we can help it.

t/app/controller/dashboard.t Outdated Show resolved Hide resolved
perllib/FixMyStreet/App/Controller/Dashboard.pm Outdated Show resolved Hide resolved
@mysociety-pusher mysociety-pusher force-pushed the 4039-northumberland-filter-category branch from 1cbb8f8 to 9aef9f4 Compare July 30, 2024 13:43
@MorayMySoc
Copy link
Contributor Author

If I add && ref $col3 eq 'HASH' it means it's never true when I run the test file, so not sure if I've misunderstood that.
I've not managed to add the ->>group to the order as getting an error on that.
The test that is failing seems to be due to some kind of escaping of characters - I've tried to mitigate that with [% SET category_built = mark_safe(cat.category) _ '-' _ mark_safe(group.group_id) %] in index.html and the text looks correct if I log it out, but doesn't seem to match.
But I think that the numbers are matching up - not sure whether I'm slugging my way down a dead end and should look to start from base on this or try and sort out these issues on this code...

@mysociety-pusher mysociety-pusher force-pushed the 4039-northumberland-filter-category branch 2 times, most recently from 4330bd8 to f25639b Compare July 31, 2024 11:54
@MorayMySoc MorayMySoc requested a review from dracos July 31, 2024 12:06
MorayMySoc and others added 2 commits October 4, 2024 13:06
…e parent categories

Set up categories in index to include name of their group.

Use this group name to check if a report is in the
selected category (through extra->group).

If there are reports that don't identify as being in that category, then
still use a 'Multiple' category.

mysociety/societyworks#4039
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