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

[Brent] [Waste] Initial small items setup. #4595

Merged
merged 8 commits into from
Oct 11, 2023
Merged

Conversation

dracos
Copy link
Member

@dracos dracos commented Sep 6, 2023

Provides an initial small items form, pretty much identical to bulky apart from wording and other small differences (e.g. sort order, photos). Uses the same controller/role/flags as Brent is having small items instead of bulky, and given timescales seemed quite a bit more straightforward than trying to allow both.

Takes a little bit of @MorayMySoc code for saving the items in the report metadata (though only if set), and little bit of @nephila-nacrea code for calling the clear cached slot.

[skip changelog]

@dracos dracos requested a review from chrismytton September 6, 2023 17:31
@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Merging #4595 (81f129d) into master (e6af91c) will decrease coverage by 15.68%.
The diff coverage is n/a.

❗ Current head 81f129d differs from pull request most recent head 95ce157. Consider uploading reports for the commit 95ce157 to get more accurate results

@@             Coverage Diff             @@
##           master    #4595       +/-   ##
===========================================
- Coverage   84.93%   69.25%   -15.68%     
===========================================
  Files         336       42      -294     
  Lines       23778     4951    -18827     
  Branches     4491        0     -4491     
===========================================
- Hits        20196     3429    -16767     
+ Misses       2235     1522      -713     
+ Partials     1347        0     -1347     

see 378 files with indirect coverage changes

Copy link
Member

@chrismytton chrismytton left a comment

Choose a reason for hiding this comment

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

Looks good! One comment but doesn't necessarily needs actioning.

@@ -82,7 +82,8 @@ Display a report.
sub display :PathPart('') :Chained('id') :Args(0) {
my ( $self, $c ) = @_;

if ($c->stash->{problem}->cobrand_data eq 'waste' && $c->stash->{problem}->category eq 'Bulky collection' ) {
my $problem = $c->stash->{problem};
if ($problem->cobrand_data eq 'waste' && ($problem->category eq 'Bulky collection' || $problem->category eq 'Small items collection')) {
Copy link
Member

Choose a reason for hiding this comment

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

Part of me thinks we should have a $problem->is_bulky method or similar, but not sure if that's a bit premature, seeing as this check is only done in two places currently.

@dracos dracos force-pushed the brent-initial-setup branch 4 times, most recently from ee4e2ac to 92106b3 Compare September 15, 2023 11:33
@dracos dracos force-pushed the brent-initial-setup branch 3 times, most recently from e4e1738 to f1ce1ae Compare September 21, 2023 06:51
@dracos dracos force-pushed the master branch 2 times, most recently from 8522ff9 to 11e15b2 Compare September 21, 2023 13:31
@dracos dracos force-pushed the brent-initial-setup branch from f1ce1ae to bd4f69c Compare September 21, 2023 13:44
@dracos dracos force-pushed the brent-initial-setup branch 5 times, most recently from 683a6da to fade136 Compare September 22, 2023 17:13
@dracos dracos force-pushed the brent-initial-setup branch 11 times, most recently from ab825c8 to 405ba4c Compare October 10, 2023 09:09
On page load after clicking back, the form values can be filled in with
their previous values by the browser, but this happens later, so switch
to the pageshow event to update the autocomplete/messages and check the
hidden select - as the autocomplete will have picked up an empty select
at the time it was created.
This is so if the page being loaded isn't a bin days one,
the heading gets replaced as well.
@dracos dracos force-pushed the brent-initial-setup branch 4 times, most recently from 490166c to ad474f9 Compare October 11, 2023 08:30
dracos and others added 6 commits October 11, 2023 09:31
Uses the bulky waste controllers and configuration, but renamed to small
items with a similar form. Includes various text tweaks requested and no
per-item photos. Item list sorted by ID, not name, so we can specify the
ordering the admin. Direct picture upload needed as reports are private.
Adds count of how many small electrical items have been selected
and how many different 'types' of item have been selected.

Add error if more than 4 small electrical or more than 3 types.

Added text to clarify the limits allowed.
Changes templates to adapt for Brent and to add small
items in relevant places instead of bulky waste.
@dracos dracos force-pushed the brent-initial-setup branch from ad474f9 to 95ce157 Compare October 11, 2023 08:32
@dracos dracos merged commit 95ce157 into master Oct 11, 2023
18 of 20 checks passed
@dracos dracos temporarily deployed to github-pages October 11, 2023 08:51 — with GitHub Pages Inactive
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