Telegram notifications for users #1029

paskal · 2021-05-26T00:49:48Z

Would add backend support #830.

Verification message:

Reply notification:

For frontend (@akellbl4): when config option telegram_bot_username is not empty, telegram notifications are enabled. We need to ask the user to write a bot with that username (https://tg.me/$username) and then use his ID for subscription: send /getid to https://tg.me/myidbot in Telegram messenger.

codecov · 2021-05-26T00:51:08Z

Codecov Report

Merging #1029 (761b5da) into master (dfcf728) will decrease coverage by 0.44%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1029      +/-   ##
==========================================
- Coverage   44.18%   43.73%   -0.45%     
==========================================
  Files         126      126              
  Lines        2904     2991      +87     
  Branches      653      680      +27     
==========================================
+ Hits         1283     1308      +25     
- Misses       1609     1670      +61     
- Partials       12       13       +1

Impacted Files	Coverage Δ
frontend/app/common/static-store.ts	`100.00% <ø> (ø)`
frontend/app/common/types.ts	`100.00% <ø> (ø)`
frontend/app/common/fetcher.ts	`96.15% <0.00%> (-0.28%)`	⬇️
frontend/app/components/root/index.ts	`0.00% <0.00%> (ø)`
frontend/app/components/user-info/user-info.tsx	`0.00% <0.00%> (ø)`
frontend/app/components/root/root.tsx	`2.07% <0.00%> (+2.07%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1847184...761b5da. Read the comment docs.

github-actions · 2021-05-26T00:52:40Z

size-limit report 📦

Path	Size
public/embed.mjs	2.23 KB (0%)
public/remark.mjs	65.31 KB (+0.03% 🔺)
public/remark.css	9.4 KB (0%)
public/last-comments.mjs	30.08 KB (+0.06% 🔺)
public/last-comments.css	5.07 KB (0%)
public/deleteme.mjs	9.79 KB (+0.19% 🔺)
public/counter.mjs	601 B (0%)

coveralls · 2021-05-26T00:53:10Z

Pull Request Test Coverage Report for Build 936678403

187 of 233 (80.26%) changed or added relevant lines in 4 files are covered.
3 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.04%) to 83.566%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
backend/app/notify/telegram.go	45	49	91.84%
backend/app/cmd/server.go	34	54	62.96%
backend/app/rest/api/rest_private.go	70	92	76.09%

Files with Coverage Reduction	New Missed Lines	%
backend/app/cmd/server.go	3	78.17%

Totals
Change from base Build 934148517:	-0.04%
Covered Lines:	5639
Relevant Lines:	6748

💛 - Coveralls

umputun

just a quick view, plannig to go deeper later on

backend/app/cmd/server.go

backend/app/rest/api/rest_private.go

umputun · 2021-06-14T00:46:18Z

backend/app/rest/api/rest_private.go

+			errors.New("missing parameter"), "address parameter is required", rest.ErrInternal)
+		return
+	}
+	existingAddress, err := s.dataService.GetUserTelegram(siteID, user.ID)


btw, if i understant "tricky to use SendErrorJSON" part this is about incorrect frame report? this can be solved with an extra param "skipFrames" of some sort. Not saying we have to refactor this, just a note

Nope, just we'll need to have a nested function which also could potentially send an answer to the user (SendErrorJSON) so it would need request and response passed to it, and some boolean flag to check if we can carry on with our logic or that function took care about it for us and we need to just return immediately. It will hurt readability more than code duplication in my opinion.

backend/app/rest/api/rest_private.go

backend/app/rest/api/rest_private_test.go

umputun · 2021-06-14T00:54:00Z

README.md

@@ -156,7 +156,7 @@ _this is the recommended way to run remark42_
 | auth.email.subj         | AUTH_EMAIL_SUBJ         | `remark42 confirmation`  | email subject                                   |
 | auth.email.content-type | AUTH_EMAIL_CONTENT_TYPE | `text/html`              | email content type                              |
 | auth.email.template     | AUTH_EMAIL_TEMPLATE     | none (predefined)        | custom email message template file              |
-| notify.users            | NOTIFY_USERS            | none                     | type of user notifications (email)              |
+| notify.users            | NOTIFY_USERS            | none                     | type of user notifications (telegram, email)    |


probably we need some detailed info about user's tg notif somewhere

Please elaborate on this one, documentation note on what is the trickiness of telegram notify?

paskal · 2021-06-14T17:53:54Z

I've fixed most of the review comments in the second commit for the reviewer convenience, looking forward to the next review round.

umputun

LGTM

paskal added the backend label May 26, 2021

paskal requested review from umputun and akellbl4 May 26, 2021 00:49

paskal marked this pull request as draft May 26, 2021 00:49

paskal added this to the v1.9 milestone May 27, 2021

paskal force-pushed the paskal/telegram_notify branch 6 times, most recently from 86c9c2c to 236e907 Compare June 7, 2021 23:13

paskal force-pushed the paskal/telegram_notify branch 6 times, most recently from 18b4857 to a9cf849 Compare June 13, 2021 23:02

paskal mentioned this pull request Jun 13, 2021

Multiple notify module improvements #1048

Merged

paskal force-pushed the paskal/telegram_notify branch 2 times, most recently from 45c386f to da048bc Compare June 13, 2021 23:22

umputun force-pushed the master branch from cd34238 to 5ce89c6 Compare June 13, 2021 23:42

paskal force-pushed the paskal/telegram_notify branch from da048bc to feb9355 Compare June 14, 2021 00:11

paskal marked this pull request as ready for review June 14, 2021 00:11

add user telegram notifications

5eed05f

paskal force-pushed the paskal/telegram_notify branch from feb9355 to 5eed05f Compare June 14, 2021 00:28

umputun requested changes Jun 14, 2021

View reviewed changes

paskal force-pushed the paskal/telegram_notify branch from a776e9f to 0655e2a Compare June 14, 2021 17:51

address review commends

761b5da

paskal force-pushed the paskal/telegram_notify branch from 0655e2a to 761b5da Compare June 17, 2021 18:29

paskal requested a review from umputun June 20, 2021 13:49

umputun approved these changes Jul 3, 2021

View reviewed changes

umputun merged commit 83ae758 into master Jul 3, 2021

umputun deleted the paskal/telegram_notify branch July 3, 2021 19:57

paskal mentioned this pull request Jun 28, 2023

Feature: Telegram Notify #830

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Telegram notifications for users #1029

Telegram notifications for users #1029

paskal commented May 26, 2021 •

edited

Loading

codecov bot commented May 26, 2021 •

edited

Loading

github-actions bot commented May 26, 2021 •

edited

Loading

coveralls commented May 26, 2021 •

edited

Loading

umputun left a comment

umputun Jun 14, 2021

paskal Jun 14, 2021

umputun Jun 14, 2021

paskal Jun 14, 2021

paskal commented Jun 14, 2021

umputun left a comment

Telegram notifications for users #1029

Telegram notifications for users #1029

Conversation

paskal commented May 26, 2021 • edited Loading

codecov bot commented May 26, 2021 • edited Loading

Codecov Report

github-actions bot commented May 26, 2021 • edited Loading

size-limit report 📦

coveralls commented May 26, 2021 • edited Loading

Pull Request Test Coverage Report for Build 936678403

💛 - Coveralls

umputun left a comment

Choose a reason for hiding this comment

umputun Jun 14, 2021

Choose a reason for hiding this comment

paskal Jun 14, 2021

Choose a reason for hiding this comment

umputun Jun 14, 2021

Choose a reason for hiding this comment

paskal Jun 14, 2021

Choose a reason for hiding this comment

paskal commented Jun 14, 2021

umputun left a comment

Choose a reason for hiding this comment

paskal commented May 26, 2021 •

edited

Loading

codecov bot commented May 26, 2021 •

edited

Loading

github-actions bot commented May 26, 2021 •

edited

Loading

coveralls commented May 26, 2021 •

edited

Loading