From a6fa2b72d1375e554cc6f2e5ca2fd4884a8509f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Mon, 1 Oct 2018 10:22:31 +0200 Subject: Migrate to idiomatic error handling app/notification*.go (#9487) --- app/notification.go | 14 +++---- app/notification_email.go | 38 ++++++++--------- app/notification_push.go | 102 +++++++++++++++++++++++++--------------------- 3 files changed, 81 insertions(+), 73 deletions(-) diff --git a/app/notification.go b/app/notification.go index 8092ba436..02f766d4d 100644 --- a/app/notification.go +++ b/app/notification.go @@ -35,19 +35,17 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod fchan = a.Srv.Store.FileInfo().GetForPost(post.Id, true, true) } - var profileMap map[string]*model.User - if result := <-pchan; result.Err != nil { + result := <-pchan + if result.Err != nil { return nil, result.Err - } else { - profileMap = result.Data.(map[string]*model.User) } + profileMap := result.Data.(map[string]*model.User) - var channelMemberNotifyPropsMap map[string]model.StringMap - if result := <-cmnchan; result.Err != nil { + result = <-cmnchan + if result.Err != nil { return nil, result.Err - } else { - channelMemberNotifyPropsMap = result.Data.(map[string]model.StringMap) } + channelMemberNotifyPropsMap := result.Data.(map[string]model.StringMap) mentionedUserIds := make(map[string]bool) threadMentionedUserIds := make(map[string]string) diff --git a/app/notification_email.go b/app/notification_email.go index cfc1bb4fd..117988e2a 100644 --- a/app/notification_email.go +++ b/app/notification_email.go @@ -22,27 +22,28 @@ func (a *App) sendNotificationEmail(notification *postNotification, user *model. post := notification.post if channel.IsGroupOrDirect() { - if result := <-a.Srv.Store.Team().GetTeamsByUserId(user.Id); result.Err != nil { + result := <-a.Srv.Store.Team().GetTeamsByUserId(user.Id) + if result.Err != nil { return result.Err - } else { - // if the recipient isn't in the current user's team, just pick one - teams := result.Data.([]*model.Team) - found := false - - for i := range teams { - if teams[i].Id == team.Id { - found = true - break - } - } + } + + // if the recipient isn't in the current user's team, just pick one + teams := result.Data.([]*model.Team) + found := false - if !found && len(teams) > 0 { - team = teams[0] - } else { - // in case the user hasn't joined any teams we send them to the select_team page - team = &model.Team{Name: "select_team", DisplayName: a.Config().TeamSettings.SiteName} + for i := range teams { + if teams[i].Id == team.Id { + found = true + break } } + + if !found && len(teams) > 0 { + team = teams[0] + } else { + // in case the user hasn't joined any teams we send them to the select_team page + team = &model.Team{Name: "select_team", DisplayName: a.Config().TeamSettings.SiteName} + } } if *a.Config().EmailSettings.EnableEmailBatching { @@ -357,7 +358,6 @@ func (a *App) GetMessageForNotification(post *model.Post, translateFunc i18n.Tra if onlyImages { return translateFunc("api.post.get_message_for_notification.images_sent", len(filenames), props) - } else { - return translateFunc("api.post.get_message_for_notification.files_sent", len(filenames), props) } + return translateFunc("api.post.get_message_for_notification.files_sent", len(filenames), props) } diff --git a/app/notification_push.go b/app/notification_push.go index 0a24ba1e0..a17ccb375 100644 --- a/app/notification_push.go +++ b/app/notification_push.go @@ -148,42 +148,45 @@ func (a *App) sendPushNotification(notification *postNotification, user *model.U func (a *App) getPushNotificationMessage(postMessage string, explicitMention, channelWideMention, hasFiles bool, senderName, channelName, channelType, replyToThreadType string, userLocale i18n.TranslateFunc) string { - message := "" + + // If the post only has images then push an appropriate message + if len(postMessage) == 0 && hasFiles { + if channelType == model.CHANNEL_DIRECT { + return strings.Trim(userLocale("api.post.send_notifications_and_forget.push_image_only"), " ") + } + return "@" + senderName + userLocale("api.post.send_notifications_and_forget.push_image_only") + } contentsConfig := *a.Config().EmailSettings.PushNotificationContents if contentsConfig == model.FULL_NOTIFICATION { if channelType == model.CHANNEL_DIRECT { - message = model.ClearMentionTags(postMessage) - } else { - message = "@" + senderName + ": " + model.ClearMentionTags(postMessage) - } - } else { - if channelType == model.CHANNEL_DIRECT { - message = userLocale("api.post.send_notifications_and_forget.push_message") - } else if channelWideMention { - message = "@" + senderName + userLocale("api.post.send_notification_and_forget.push_channel_mention") - } else if explicitMention { - message = "@" + senderName + userLocale("api.post.send_notifications_and_forget.push_explicit_mention") - } else if replyToThreadType == THREAD_ROOT { - message = "@" + senderName + userLocale("api.post.send_notification_and_forget.push_comment_on_post") - } else if replyToThreadType == THREAD_ANY { - message = "@" + senderName + userLocale("api.post.send_notification_and_forget.push_comment_on_thread") - } else { - message = "@" + senderName + userLocale("api.post.send_notifications_and_forget.push_general_message") + return model.ClearMentionTags(postMessage) } + return "@" + senderName + ": " + model.ClearMentionTags(postMessage) } - // If the post only has images then push an appropriate message - if len(postMessage) == 0 && hasFiles { - if channelType == model.CHANNEL_DIRECT { - message = strings.Trim(userLocale("api.post.send_notifications_and_forget.push_image_only"), " ") - } else { - message = "@" + senderName + userLocale("api.post.send_notifications_and_forget.push_image_only") - } + if channelType == model.CHANNEL_DIRECT { + return userLocale("api.post.send_notifications_and_forget.push_message") + } + + if channelWideMention { + return "@" + senderName + userLocale("api.post.send_notification_and_forget.push_channel_mention") + } + + if explicitMention { + return "@" + senderName + userLocale("api.post.send_notifications_and_forget.push_explicit_mention") + } + + if replyToThreadType == THREAD_ROOT { + return "@" + senderName + userLocale("api.post.send_notification_and_forget.push_comment_on_post") + } + + if replyToThreadType == THREAD_ANY { + return "@" + senderName + userLocale("api.post.send_notification_and_forget.push_comment_on_thread") } - return message + return "@" + senderName + userLocale("api.post.send_notifications_and_forget.push_general_message") } func (a *App) ClearPushNotificationSync(userId string, channelId string) { @@ -272,32 +275,34 @@ func (a *App) sendToPushProxy(msg model.PushNotification, session *model.Session request, _ := http.NewRequest("POST", strings.TrimRight(*a.Config().EmailSettings.PushNotificationServer, "/")+model.API_URL_SUFFIX_V1+"/send_push", strings.NewReader(msg.ToJson())) - if resp, err := a.HTTPService.MakeClient(true).Do(request); err != nil { + resp, err := a.HTTPService.MakeClient(true).Do(request) + if err != nil { mlog.Error(fmt.Sprintf("Device push reported as error for UserId=%v SessionId=%v message=%v", session.UserId, session.Id, err.Error()), mlog.String("user_id", session.UserId)) - } else { - pushResponse := model.PushResponseFromJson(resp.Body) - if resp.Body != nil { - consumeAndClose(resp) - } + return + } - if pushResponse[model.PUSH_STATUS] == model.PUSH_STATUS_REMOVE { - mlog.Info(fmt.Sprintf("Device was reported as removed for UserId=%v SessionId=%v removing push for this session", session.UserId, session.Id), mlog.String("user_id", session.UserId)) - a.AttachDeviceId(session.Id, "", session.ExpiresAt) - a.ClearSessionCacheForUser(session.UserId) - } + pushResponse := model.PushResponseFromJson(resp.Body) + if resp.Body != nil { + consumeAndClose(resp) + } - if pushResponse[model.PUSH_STATUS] == model.PUSH_STATUS_FAIL { - mlog.Error(fmt.Sprintf("Device push reported as error for UserId=%v SessionId=%v message=%v", session.UserId, session.Id, pushResponse[model.PUSH_STATUS_ERROR_MSG]), mlog.String("user_id", session.UserId)) - } + if pushResponse[model.PUSH_STATUS] == model.PUSH_STATUS_REMOVE { + mlog.Info(fmt.Sprintf("Device was reported as removed for UserId=%v SessionId=%v removing push for this session", session.UserId, session.Id), mlog.String("user_id", session.UserId)) + a.AttachDeviceId(session.Id, "", session.ExpiresAt) + a.ClearSessionCacheForUser(session.UserId) + } + + if pushResponse[model.PUSH_STATUS] == model.PUSH_STATUS_FAIL { + mlog.Error(fmt.Sprintf("Device push reported as error for UserId=%v SessionId=%v message=%v", session.UserId, session.Id, pushResponse[model.PUSH_STATUS_ERROR_MSG]), mlog.String("user_id", session.UserId)) } } func (a *App) getMobileAppSessions(userId string) ([]*model.Session, *model.AppError) { - if result := <-a.Srv.Store.Session().GetSessionsWithActiveDeviceIds(userId); result.Err != nil { + result := <-a.Srv.Store.Session().GetSessionsWithActiveDeviceIds(userId) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.([]*model.Session), nil } + return result.Data.([]*model.Session), nil } func ShouldSendPushNotification(user *model.User, channelNotifyProps model.StringMap, wasMentioned bool, status *model.Status, post *model.Post) bool { @@ -352,11 +357,16 @@ func DoesStatusAllowPushNotification(userNotifyProps model.StringMap, status *mo return false } - if pushStatus, ok := userNotifyProps["push_status"]; (pushStatus == model.STATUS_ONLINE || !ok) && (status.ActiveChannel != channelId || model.GetMillis()-status.LastActivityAt > model.STATUS_CHANNEL_TIMEOUT) { + pushStatus, ok := userNotifyProps["push_status"] + if (pushStatus == model.STATUS_ONLINE || !ok) && (status.ActiveChannel != channelId || model.GetMillis()-status.LastActivityAt > model.STATUS_CHANNEL_TIMEOUT) { return true - } else if pushStatus == model.STATUS_AWAY && (status.Status == model.STATUS_AWAY || status.Status == model.STATUS_OFFLINE) { + } + + if pushStatus == model.STATUS_AWAY && (status.Status == model.STATUS_AWAY || status.Status == model.STATUS_OFFLINE) { return true - } else if pushStatus == model.STATUS_OFFLINE && status.Status == model.STATUS_OFFLINE { + } + + if pushStatus == model.STATUS_OFFLINE && status.Status == model.STATUS_OFFLINE { return true } -- cgit v1.2.3-1-g7c22