-
-
Notifications
You must be signed in to change notification settings - Fork 213
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
Fix scheduled reports encoding #28174
Conversation
couch_user=self.user, domain=domain, scheduled_report_id=report._id | ||
)[0] | ||
self.assertTrue(self.user.username in response.decode('utf-8')) | ||
self.assertTrue(self.user.username in report_text) |
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.
Just naively grepping, there are other places where we decode('utf-8')
in tests on a response
variable, mostly API calls (HQ APIs, SMS, case claim). Do you think those could point to similar issues that deserve followup? (Not in this PR.)
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.
Possibly. We may want to look into them.
@@ -878,7 +882,7 @@ def _render_report_configs(request, configs, domain, owner_id, couch_user, email | |||
|
|||
# Don't send an email if none of the reports configs have started | |||
if len(configs) == 0: | |||
return False, False | |||
return "", [] |
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.
Hooray for consistent return values.
The first commit is the fix, and the second is cleanup to make the code easier to understand.
This is the same type of issue fixed in #28157
See also https://docs.djangoproject.com/en/3.0/releases/2.0/#removed-support-for-bytestrings-in-some-places