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

[Bexley][WW] Fetch successful collections alongside other in-cab logs #4969

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

nephila-nacrea
Copy link
Contributor

For https://3.basecamp.com/4020879/buckets/35109031/todos/7411178677.

Logs with a reason of 'N/A' were being ignored, but we need them to check whether a collection has already been made today for a given service.

[skip changelog]

@nephila-nacrea nephila-nacrea changed the base branch from master to bexley-end-of-round May 22, 2024 09:36
Copy link

codecov bot commented May 22, 2024

Codecov Report

Attention: Patch coverage is 77.77778% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 82.60%. Comparing base (cf1ca81) to head (78fabdd).

Files Patch % Lines
perllib/FixMyStreet/Cobrand/Bexley/Waste.pm 77.77% 0 Missing and 4 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4969   +/-   ##
=======================================
  Coverage   82.59%   82.60%           
=======================================
  Files         393      393           
  Lines       30751    30756    +5     
  Branches     4876     4877    +1     
=======================================
+ Hits        25400    25405    +5     
+ Misses       3898     3897    -1     
- Partials     1453     1454    +1     

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

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.

I think this is a step in the right direction, but we also need to allow for the fact that there aren't always round completion logs, so the presence of any log should be taken as a sign that the round has been completed.

See this Basecamp message and the subsequent discussions for details about this problem.

The fix I did was 6ca9636, which gets logs for all UPRNs under the relevant USRN, which meant because the code was checking property and street logs it would allow a report if there was a red tag log against the street (but lock the user out if the red tag was against one of their containers).

Guess we could have the _in_cab_logs method just return all logs and then add some other methods that filter down the list of logs to look for ones we're interested in, which I think is:

  • Red tags for the current property
  • Service updates for the street ("No access" type things, I assume)
  • Round completion indicters
    • Any kind of logs within the relevant USRN on the relevant date can be taken as an indictor of round completion, I think.

Sorry!

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.

Yay, this is a huge improvement!

I think as discussed in Slack it would be good to just use the USRN logs, but that can be done in a separate PR.

In the meantime lets get this change on staging for them 🙂

return [
{
LogID => 1,
Reason => 'Bin has gone feral',
Copy link
Member

Choose a reason for hiding this comment

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

🤣

@nephila-nacrea nephila-nacrea self-assigned this May 30, 2024
@chrismytton chrismytton changed the base branch from bexley-end-of-round to master June 3, 2024 08:48
@mysociety-pusher mysociety-pusher force-pushed the bexley-ww-successful-collection-wording branch from 5731184 to 0c214a8 Compare June 3, 2024 08:49
Logs with a reason of 'N/A' were being ignored, but we need them to
check whether a collection has already been made today for a given
service.
We had been assigning the size of an array rather than an arrayref
to {service_updates}.
@mysociety-pusher mysociety-pusher force-pushed the bexley-ww-successful-collection-wording branch from 0c214a8 to 78fabdd Compare June 3, 2024 11:26
@mysociety-pusher mysociety-pusher merged commit 78fabdd into master Jun 3, 2024
21 of 22 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.

None yet

3 participants