-
-
Notifications
You must be signed in to change notification settings - Fork 272
[Internal] Replace ids with urls in send-failures log #5774
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
[Internal] Replace ids with urls in send-failures log #5774
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5774 +/- ##
==========================================
- Coverage 82.66% 82.66% -0.01%
==========================================
Files 458 458
Lines 35799 35802 +3
Branches 5843 5844 +1
==========================================
+ Hits 29592 29594 +2
- Misses 4505 4506 +1
Partials 1702 1702 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| my $key = join ', ', @{ $row->body_names }; | ||
| $bodies{$key} ||= []; | ||
| push @{ $bodies{$key} }, $row->id; | ||
| push @{ $bodies{$key} }, "$base_url/admin/report_edit/" . $row->id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For waste, or TfL reports (where the BASE_URL won't work and only give a 404, dunno if there are other cases), could this know that and provide a working link?
7f3f574 to
ee388a5
Compare
dracos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One suggestion to simplify getting the base using existing code, have pushed a fixup, see what you think
|
@dracos Tests were failing as I'd hardcoded the ids in the tests, so latest commit has fixed that. I think what you've done is great, so not sure if you're wanting anything else added - asked for re-review for clarity as it's still awaiting sign-off. |
dracos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think so, let's give it a try, thanks :)
Preliminary effort to make log message more practical mysociety/societyworks#4984
0b5b40d to
b654f02
Compare
Preliminary effort to make log message more practical.
https://github.com/mysociety/societyworks/issues/4984
[skip changelog]