address review commends

This commit is contained in:
Dmitry Verkhoturov
2021-07-03 14:57:09 -05:00
committed by Umputun
parent 200733ed03
commit 83ae758573
7 changed files with 198 additions and 93 deletions
+13 -32
View File
@@ -462,22 +462,22 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) {
}
var emailNotifications bool
var telegramNotificationsBotUsername string
notifyService, telegramBotUsername, err := s.makeNotify(dataService, authenticator)
if contains("email", s.Notify.Users) {
emailNotifications = true
}
if contains("telegram", s.Notify.Users) {
telegramNotificationsBotUsername = telegramBotUsername
// we pass telegramBotUsername to Rest server only if user notifications are enabled
if !contains("telegram", s.Notify.Users) {
telegramBotUsername = ""
}
if err != nil {
log.Printf("[WARN] failed to make notify service, %s", err)
notifyService = notify.NopService // disable notifier
emailNotifications = false // email notifications are not available in this case
telegramNotificationsBotUsername = "" // telegram notifications are not available in this case either
notifyService = notify.NopService // disable notifier
emailNotifications = false // email notifications are not available in this case
telegramBotUsername = "" // telegram notifications are not available in this case either
}
imgProxy := &proxy.Image{
@@ -516,7 +516,7 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) {
UpdateLimiter: s.UpdateLimit,
ImageService: imageService,
EmailNotifications: emailNotifications,
TelegramBotUsername: telegramNotificationsBotUsername,
TelegramBotUsername: telegramBotUsername,
EmojiEnabled: s.EnableEmoji,
AnonVote: s.AnonymousVote && s.RestrictVoteIP,
SimpleView: s.SimpleView,
@@ -877,35 +877,16 @@ func (s *ServerCommand) loadEmailTemplate() (string, error) {
// aside from notify.Service and error, returns telegram bot name which will be passed to the frontend
func (s *ServerCommand) makeNotify(dataStore *service.DataStore, authenticator *auth.Service) (*notify.Service, string, error) {
var notifyService *notify.Service
notifyService := notify.NopService
var destinations []notify.Destination
var telegramBotUsername string
for _, t := range s.Notify.Admins {
switch t {
case "slack":
slack, err := notify.NewSlack(s.Notify.Slack.Token, s.Notify.Slack.Channel)
if err != nil {
return nil, "", errors.Wrap(err, "failed to create slack notification destination")
}
destinations = append(destinations, slack)
case "telegram":
case "email":
case "none":
notifyService = notify.NopService
default:
return nil, "", errors.Errorf("unsupported admin notification type %q", s.Notify.Type)
}
}
for _, t := range s.Notify.Users {
switch t {
case "telegram":
case "email":
case "none":
notifyService = notify.NopService
default:
return nil, "", errors.Errorf("unsupported user notification type %q", s.Notify.Type)
if contains("slack", s.Notify.Admins) {
slack, err := notify.NewSlack(s.Notify.Slack.Token, s.Notify.Slack.Channel)
if err != nil {
return nil, "", errors.Wrap(err, "failed to create slack notification destination")
}
destinations = append(destinations, slack)
}
if contains("telegram", s.Notify.Users) || contains("telegram", s.Notify.Admins) {
+24 -11
View File
@@ -181,29 +181,42 @@ func (t *Telegram) sendMessage(ctx context.Context, b []byte, chatID string) err
func buildTelegramMessage(req Request) ([]byte, error) {
commentURLPrefix := req.Comment.Locator.URL + uiNav
msg := "New reply to comment"
msg := ""
if req.Comment.PostTitle != "" {
msg += fmt.Sprintf(" for %q", req.Comment.PostTitle)
msg += fmt.Sprintf("\"%q\" ", req.Comment.PostTitle)
}
msg += ":"
msg += fmt.Sprintf(
"[%s](%s)",
escapeText(req.Comment.User.Name),
commentURLPrefix+req.Comment.ID,
)
if req.Comment.ParentID != "" {
msg += fmt.Sprintf(
"\n[Original comment](%s) from %s at %s:\n%s",
commentURLPrefix+req.parent.ID,
" -> [%s](%s)",
escapeText(req.parent.User.Name),
escapeText(req.parent.Timestamp.Format("02.01.2006 at 15:04")),
commentURLPrefix+req.parent.ID,
)
}
msg += fmt.Sprintf(
"at %s",
escapeText(req.Comment.Timestamp.Format("02.01.2006 at 15:04")),
)
if req.Comment.ParentID != "" {
msg += fmt.Sprintf(
"\n\n\"_%s_\"",
escapeText(req.parent.Orig),
)
}
msg += fmt.Sprintf(
"\n[Reply](%s) from %s at %s:\n%s",
commentURLPrefix+req.Comment.ID,
escapeText(req.Comment.User.Name),
escapeText(req.Comment.Timestamp.Format("02.01.2006 at 15:04")),
"\n\n_%s_",
escapeText(req.Comment.Orig),
)
msg = html.UnescapeString(msg)
body := telegramMsg{Text: msg, ParseMode: "MarkdownV2"}
b, err := json.Marshal(body)
@@ -254,7 +267,7 @@ func (t *Telegram) SendVerification(ctx context.Context, req VerificationRequest
// buildVerificationMessage generates verification telegram message based on given input
func (t *Telegram) buildVerificationMessage(user, token, site string) ([]byte, error) {
result := fmt.Sprintf("This is confirmation for %s on site %s\n"+
result := fmt.Sprintf("Confirmation for *%s* on site %s\n"+
"Please copy and paste this text into “token” field on comments page to confirm subscription:\n\n\n"+
"```%s```",
escapeText(user), escapeText(site), escapeCode(token))
+1 -1
View File
@@ -170,7 +170,7 @@ func TestTelegram_SendVerification(t *testing.T) {
// test buildVerificationMessage separately for message text
res, err := tb.buildVerificationMessage(req.User, req.Token, req.SiteID)
assert.NoError(t, err)
assert.Contains(t, string(res), "This is confirmation for test\\\\_username on site remark")
assert.Contains(t, string(res), `Confirmation for *test\\_username* on site remark`)
assert.Contains(t, string(res), `secret_`)
}
+7
View File
@@ -331,6 +331,8 @@ func (s *private) sendTelegramConfirmationCtrl(w http.ResponseWriter, r *http.Re
}
existingAddress, err := s.dataService.GetUserTelegram(siteID, user.ID)
if err != nil {
// we don't care as much if we can't retrieve the current value of that field for the user,
// as it's only used to check if we're trying to set to the same value it's already set to
log.Printf("[WARN] can't read telegram for %s, %v", user.ID, err)
}
if address == existingAddress {
@@ -388,6 +390,7 @@ func (s *private) setConfirmedEmailCtrl(w http.ResponseWriter, r *http.Request)
return
}
// Handshake.ID is user.ID + "::" + address
elems := strings.Split(confClaims.Handshake.ID, "::")
if len(elems) != 2 || elems[0] != user.ID {
rest.SendErrorJSON(w, r, http.StatusBadRequest, errors.New(confClaims.Handshake.ID), "invalid handshake token", rest.ErrInternal)
@@ -440,6 +443,7 @@ func (s *private) setConfirmedTelegramCtrl(w http.ResponseWriter, r *http.Reques
return
}
// Handshake.ID is user.ID + "::" + address
elems := strings.Split(confClaims.Handshake.ID, "::")
if len(elems) != 2 || elems[0] != user.ID {
rest.SendErrorJSON(w, r, http.StatusBadRequest, errors.New(confClaims.Handshake.ID), "invalid handshake token", rest.ErrInternal)
@@ -480,6 +484,7 @@ func (s *private) emailUnsubscribeCtrl(w http.ResponseWriter, r *http.Request) {
return
}
// Handshake.ID is user.ID + "::" + address
elems := strings.Split(confClaims.Handshake.ID, "::")
if len(elems) != 2 {
rest.SendErrorHTML(w, r, http.StatusBadRequest,
@@ -491,6 +496,8 @@ func (s *private) emailUnsubscribeCtrl(w http.ResponseWriter, r *http.Request) {
existingAddress, err := s.dataService.GetUserEmail(siteID, userID)
if err != nil {
// we don't care as much if we can't retrieve the current value of that field for the user,
// as it's only used to check if we're trying to set to the same value it's already set to
log.Printf("[WARN] can't read email for %s, %v", userID, err)
}
if existingAddress == "" {
+151 -47
View File
@@ -656,7 +656,7 @@ func TestRest_EmailAndTelegram(t *testing.T) {
}
}
func TestRest_EmailAndTelegramNotification(t *testing.T) {
func TestRest_EmailNotification(t *testing.T) {
ts, srv, teardown := startupT(t)
defer teardown()
@@ -686,9 +686,8 @@ func TestRest_EmailAndTelegramNotification(t *testing.T) {
time.Sleep(time.Millisecond * 30)
require.Equal(t, 1, len(mockDestination.Get()))
assert.Empty(t, mockDestination.Get()[0].Emails)
assert.Empty(t, mockDestination.Get()[0].Telegrams)
// create child comment from another user, email and telegram notification only to admin expected
// create child comment from another user, email notification only to admin expected
req, err = http.NewRequest("POST", ts.URL+"/api/v1/comment", strings.NewReader(fmt.Sprintf(
`{"text": "test 456",
"pid": "%s",
@@ -707,7 +706,6 @@ func TestRest_EmailAndTelegramNotification(t *testing.T) {
time.Sleep(time.Millisecond * 30)
require.Equal(t, 2, len(mockDestination.Get()))
assert.Empty(t, mockDestination.Get()[1].Emails)
assert.Empty(t, mockDestination.Get()[1].Telegrams)
// send confirmation token for email
req, err = http.NewRequest(http.MethodPost, ts.URL+"/api/v1/email/subscribe?site=remark42&address=good@example.com", nil)
@@ -723,37 +721,10 @@ func TestRest_EmailAndTelegramNotification(t *testing.T) {
time.Sleep(time.Millisecond * 30)
require.Equal(t, 1, len(mockDestination.GetVerify()))
assert.Equal(t, "good@example.com", mockDestination.GetVerify()[0].Email)
emailVerificationToken := mockDestination.GetVerify()[0].Token
verificationToken := mockDestination.GetVerify()[0].Token
// verify email
req, err = http.NewRequest(http.MethodPost, ts.URL+fmt.Sprintf("/api/v1/email/confirm?site=remark42&tkn=%s", emailVerificationToken), nil)
require.NoError(t, err)
req.Header.Add("X-JWT", devToken)
resp, err = client.Do(req)
require.NoError(t, err)
body, err = ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
require.Equal(t, http.StatusOK, resp.StatusCode, string(body))
// send confirmation token for telegram
req, err = http.NewRequest(http.MethodPost, ts.URL+"/api/v1/telegram/subscribe?site=remark42&address=good_telegram", nil)
require.NoError(t, err)
req.Header.Add("X-JWT", devToken)
resp, err = client.Do(req)
require.NoError(t, err)
body, err = ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
require.Equal(t, http.StatusOK, resp.StatusCode, string(body))
// wait for mock notification Submit to kick off
time.Sleep(time.Millisecond * 30)
require.Equal(t, 2, len(mockDestination.GetVerify()))
assert.Equal(t, "good_telegram", mockDestination.GetVerify()[1].Telegram)
telegramVerificationToken := mockDestination.GetVerify()[1].Token
// verify telegram
req, err = http.NewRequest(http.MethodPost, ts.URL+fmt.Sprintf("/api/v1/telegram/confirm?site=remark42&tkn=%s", telegramVerificationToken), nil)
req, err = http.NewRequest(http.MethodPost, ts.URL+fmt.Sprintf("/api/v1/email/confirm?site=remark42&tkn=%s", verificationToken), nil)
require.NoError(t, err)
req.Header.Add("X-JWT", devToken)
resp, err = client.Do(req)
@@ -779,7 +750,7 @@ func TestRest_EmailAndTelegramNotification(t *testing.T) {
assert.Equal(t, store.User{Name: "developer one", ID: "dev", EmailSubscription: true,
Picture: "http://example.com/pic.png", IP: "127.0.0.1", SiteID: "remark42"}, user)
// create child comment from another user, email and telegram notification expected
// create child comment from another user, email notification expected
req, err = http.NewRequest("POST", ts.URL+"/api/v1/comment", strings.NewReader(fmt.Sprintf(
`{"text": "test 789",
"pid": "%s",
@@ -798,7 +769,6 @@ func TestRest_EmailAndTelegramNotification(t *testing.T) {
time.Sleep(time.Millisecond * 30)
require.Equal(t, 3, len(mockDestination.Get()))
assert.Equal(t, []string{"good@example.com"}, mockDestination.Get()[2].Emails)
assert.Equal(t, []string{"good_telegram"}, mockDestination.Get()[2].Telegrams)
// delete user's email
req, err = http.NewRequest(http.MethodDelete, ts.URL+"/api/v1/email?site=remark42", nil)
@@ -811,18 +781,7 @@ func TestRest_EmailAndTelegramNotification(t *testing.T) {
require.NoError(t, resp.Body.Close())
assert.Equal(t, http.StatusOK, resp.StatusCode, string(body))
// delete user's telegram
req, err = http.NewRequest(http.MethodDelete, ts.URL+"/api/v1/telegram?site=remark42", nil)
require.NoError(t, err)
req.Header.Add("X-JWT", devToken)
resp, err = client.Do(req)
require.NoError(t, err)
body, err = ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
assert.Equal(t, http.StatusOK, resp.StatusCode, string(body))
// create child comment from another user, no email or telegram notification
// create child comment from another user, no email notification
req, err = http.NewRequest("POST", ts.URL+"/api/v1/comment", strings.NewReader(
`{"text": "test 321",
"user": {"name": "other_user"},
@@ -840,6 +799,151 @@ func TestRest_EmailAndTelegramNotification(t *testing.T) {
time.Sleep(time.Millisecond * 30)
require.Equal(t, 4, len(mockDestination.Get()))
assert.Empty(t, mockDestination.Get()[3].Emails)
}
func TestRest_TelegramNotification(t *testing.T) {
ts, srv, teardown := startupT(t)
defer teardown()
mockDestination := &notify.MockDest{}
srv.privRest.notifyService = notify.NewService(srv.DataService, 1, mockDestination)
defer srv.privRest.notifyService.Close()
client := http.Client{}
// create new comment from dev user
req, err := http.NewRequest("POST", ts.URL+"/api/v1/comment", strings.NewReader(
`{"text": "test 123",
"user": {"name": "dev::good@example.com"},
"locator":{"url": "https://radio-t.com/blah1",
"site": "remark42"}}`))
assert.NoError(t, err)
req.Header.Add("X-JWT", devToken)
resp, err := client.Do(req)
assert.NoError(t, err)
body, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
require.Equal(t, http.StatusCreated, resp.StatusCode, string(body))
parentComment := store.Comment{}
require.NoError(t, render.DecodeJSON(strings.NewReader(string(body)), &parentComment))
// wait for mock notification Submit to kick off
time.Sleep(time.Millisecond * 30)
require.Equal(t, 1, len(mockDestination.Get()))
assert.Empty(t, mockDestination.Get()[0].Telegrams)
// create child comment from another user, telegram notification only to admin expected
req, err = http.NewRequest("POST", ts.URL+"/api/v1/comment", strings.NewReader(fmt.Sprintf(
`{"text": "test 456",
"pid": "%s",
"user": {"name": "other_user"},
"locator":{"url": "https://radio-t.com/blah1",
"site": "remark42"}}`, parentComment.ID)))
assert.NoError(t, err)
req.Header.Add("X-JWT", anonToken)
resp, err = client.Do(req)
assert.NoError(t, err)
body, err = ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
require.Equal(t, http.StatusCreated, resp.StatusCode, string(body))
// wait for mock notification Submit to kick off
time.Sleep(time.Millisecond * 30)
require.Equal(t, 2, len(mockDestination.Get()))
assert.Empty(t, mockDestination.Get()[1].Telegrams)
// send confirmation token for telegram
req, err = http.NewRequest(http.MethodPost, ts.URL+"/api/v1/telegram/subscribe?site=remark42&address=good_telegram", nil)
require.NoError(t, err)
req.Header.Add("X-JWT", devToken)
resp, err = client.Do(req)
require.NoError(t, err)
body, err = ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
require.Equal(t, http.StatusOK, resp.StatusCode, string(body))
// wait for mock notification Submit to kick off
time.Sleep(time.Millisecond * 30)
require.Equal(t, 1, len(mockDestination.GetVerify()))
assert.Equal(t, "good_telegram", mockDestination.GetVerify()[0].Telegram)
verificationToken := mockDestination.GetVerify()[0].Token
// verify telegram
req, err = http.NewRequest(http.MethodPost, ts.URL+fmt.Sprintf("/api/v1/telegram/confirm?site=remark42&tkn=%s", verificationToken), nil)
require.NoError(t, err)
req.Header.Add("X-JWT", devToken)
resp, err = client.Do(req)
require.NoError(t, err)
body, err = ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
require.Equal(t, http.StatusOK, resp.StatusCode, string(body))
// get user information to verify the subscription
req, err = http.NewRequest(http.MethodGet, ts.URL+"/api/v1/user?site=remark42", nil)
require.NoError(t, err)
req.Header.Add("X-JWT", devToken)
resp, err = client.Do(req)
require.NoError(t, err)
body, err = ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
require.Equal(t, http.StatusOK, resp.StatusCode, string(body))
var user store.User
err = json.Unmarshal(body, &user)
assert.NoError(t, err)
assert.Equal(t, store.User{Name: "developer one", ID: "dev",
Picture: "http://example.com/pic.png", IP: "127.0.0.1", SiteID: "remark42"}, user)
// create child comment from another user, telegram notification expected
req, err = http.NewRequest("POST", ts.URL+"/api/v1/comment", strings.NewReader(fmt.Sprintf(
`{"text": "test 789",
"pid": "%s",
"user": {"name": "other_user"},
"locator":{"url": "https://radio-t.com/blah1",
"site": "remark42"}}`, parentComment.ID)))
assert.NoError(t, err)
req.Header.Add("X-JWT", anonToken)
resp, err = client.Do(req)
assert.NoError(t, err)
body, err = ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
require.Equal(t, http.StatusCreated, resp.StatusCode, string(body))
// wait for mock notification Submit to kick off
time.Sleep(time.Millisecond * 30)
require.Equal(t, 3, len(mockDestination.Get()))
assert.Equal(t, []string{"good_telegram"}, mockDestination.Get()[2].Telegrams)
// delete user's telegram
req, err = http.NewRequest(http.MethodDelete, ts.URL+"/api/v1/telegram?site=remark42", nil)
require.NoError(t, err)
req.Header.Add("X-JWT", devToken)
resp, err = client.Do(req)
require.NoError(t, err)
body, err = ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
assert.Equal(t, http.StatusOK, resp.StatusCode, string(body))
// create child comment from another user, no telegram notification
req, err = http.NewRequest("POST", ts.URL+"/api/v1/comment", strings.NewReader(
`{"text": "test 321",
"user": {"name": "other_user"},
"locator":{"url": "https://radio-t.com/blah1",
"site": "remark42"}}`))
assert.NoError(t, err)
req.Header.Add("X-JWT", devToken)
resp, err = client.Do(req)
assert.NoError(t, err)
body, err = ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
require.Equal(t, http.StatusCreated, resp.StatusCode, string(body))
// wait for mock notification Submit to kick off
time.Sleep(time.Millisecond * 30)
require.Equal(t, 4, len(mockDestination.Get()))
assert.Empty(t, mockDestination.Get()[3].Telegrams)
}
+1 -1
View File
@@ -16,7 +16,7 @@ beforeEach(() => {
simple_view: false,
anon_vote: false,
email_notifications: false,
telegram_bot_username: "",
telegram_bot_username: '',
emoji_enabled: true,
};
});
+1 -1
View File
@@ -29,7 +29,7 @@ export const StaticStore: StaticStoreType = {
simple_view: false,
anon_vote: false,
email_notifications: false,
telegram_bot_username: "",
telegram_bot_username: '',
emoji_enabled: false,
},
query: querySettings as QuerySettingsType,