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

[Bromley] Include notes in updates from Echo #5277

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dracos
Copy link
Member

@dracos dracos commented Nov 27, 2024

And fix issue with being redirected from Echo to Bromley more than once.
Fixes https://github.com/mysociety/societyworks/issues/4633 [skip changelog]

Copy link

codecov bot commented Nov 27, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.42%. Comparing base (fbbc68c) to head (856bed4).

Files with missing lines Patch % Lines
perllib/FixMyStreet/Cobrand/Bromley.pm 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5277   +/-   ##
=======================================
  Coverage   82.42%   82.42%           
=======================================
  Files         415      415           
  Lines       32837    32832    -5     
  Branches     5265     5265           
=======================================
- Hits        27065    27063    -2     
+ Misses       4216     4213    -3     
  Partials     1556     1556           

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

@dracos dracos force-pushed the sw-4633-bromley-veolia-notes branch from d19e95a to abd10c5 Compare November 27, 2024 13:45
Copy link
Contributor

@nephila-nacrea nephila-nacrea left a comment

Choose a reason for hiding this comment

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

Looks alright to me as far as I can tell. I'm just not very clear on the journey of a report being referred to Bromley from Echo or vice versa.

foreach (@$data) {
$notes = $_->{Value} if $_->{DatatypeName} eq 'Veolia Notes';
}

# An update from Echo with resolution code 1252
my $code = $comment->get_extra_metadata('external_status_code') || '';
if ($code eq '1252') {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if it would be good to have a comment or constant to explain what code 1252 refers to exactly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this could certainly do with more description (and on the wiki too, now the project's live). Hopefully the "redirecting of reports between backends" tests in t/cobrand/bromley.t at least show the behaviour of things bouncing back and forth.
(Basically, if a report is sent to Echo, they can refer it to Bromley by sending a '1252' update code to us)

@dracos dracos force-pushed the sw-4633-bromley-veolia-notes branch from 65a3a6f to 856bed4 Compare December 11, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants