-
-
Notifications
You must be signed in to change notification settings - Fork 387
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
Multiple notify module improvements #1048
Conversation
deb0bac
to
557332c
Compare
557332c
to
23a101d
Compare
Codecov Report
@@ Coverage Diff @@
## master #1048 +/- ##
=======================================
Coverage 44.18% 44.18%
=======================================
Files 126 126
Lines 2904 2904
Branches 653 653
=======================================
Hits 1283 1283
Misses 1609 1609
Partials 12 12 Continue to review full report at Codecov.
|
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.
not sure about such a rich escaping
b, err := json.Marshal(body) | ||
if err != nil { | ||
return nil, err | ||
} | ||
return b, nil | ||
} | ||
|
||
func escapeTitle(title string) string { | ||
escSymbols := []string{"[", "]", "(", ")"} | ||
func escapeText(title string) string { |
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.
I'm not sure about this one. The original ticket for all this escaping indicated no improvement for the initial problem #839
unless you see something really helpful due to this, I suggest dropping it completely
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.
I saw messages not sending without escaping, status code 400. We can't drop it according to my tests with the verify messages.
A couple of changes to telegram notifications and error handling code, email text improvements, ignore IntelliJ Idea rest requests variables file.
Reply notification text, now more consistent with the email notification, and the same which will be used for user notifications. Before:
After:
Part of #1029 is not strictly tied to the telegram user notifications, and I wanted to merge separately.