diff --git a/changelog/unreleased/improve-notification-logs.md b/changelog/unreleased/improve-notification-logs.md new file mode 100644 index 000000000..137197d80 --- /dev/null +++ b/changelog/unreleased/improve-notification-logs.md @@ -0,0 +1,6 @@ +Enhancement: Improve the notification logs + +Improve the notification logs when the user has no email address + +https://github.com/owncloud/ocis/pull/6862 +https://github.com/owncloud/ocis/issues/6855 diff --git a/services/notifications/pkg/service/service.go b/services/notifications/pkg/service/service.go index c55eef6af..9ac5ea291 100644 --- a/services/notifications/pkg/service/service.go +++ b/services/notifications/pkg/service/service.go @@ -8,6 +8,7 @@ import ( "os" "os/signal" "path" + "strings" "syscall" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" @@ -126,6 +127,17 @@ func (s eventsNotifier) send(ctx context.Context, recipientList []*channels.Mess } } +func (s eventsNotifier) ensureGranteeList(ctx context.Context, executant, u *user.UserId, g *group.GroupId) []*user.User { + granteeList, err := s.getGranteeList(ctx, executant, u, g) + if err != nil { + s.logger.Error().Err(err).Str("event", "ensureGranteeList").Msg("Could not get grantee list") + return nil + } else if len(granteeList) == 0 { + return nil + } + return granteeList +} + func (s eventsNotifier) getGranteeList(ctx context.Context, executant, u *user.UserId, g *group.GroupId) ([]*user.User, error) { switch { case u != nil: @@ -136,6 +148,10 @@ func (s eventsNotifier) getGranteeList(ctx context.Context, executant, u *user.U if err != nil { return nil, err } + if strings.TrimSpace(usr.GetMail()) == "" { + s.logger.Debug().Str("event", "getGranteeList").Msgf("User %s has no email, skipped", usr.GetUsername()) + return nil, nil + } return []*user.User{usr}, nil case g != nil: gatewayClient, err := s.gatewaySelector.Next() @@ -165,6 +181,10 @@ func (s eventsNotifier) getGranteeList(ctx context.Context, executant, u *user.U if err != nil { return nil, err } + if strings.TrimSpace(usr.GetMail()) == "" { + s.logger.Debug().Str("event", "getGranteeList").Msgf("User %s has no email, skipped", usr.GetUsername()) + continue + } userList = append(userList, usr) } return userList, nil diff --git a/services/notifications/pkg/service/shares.go b/services/notifications/pkg/service/shares.go index 9328c7f88..b6d7d844b 100644 --- a/services/notifications/pkg/service/shares.go +++ b/services/notifications/pkg/service/shares.go @@ -41,9 +41,8 @@ func (s eventsNotifier) handleShareCreated(e events.ShareCreated) { return } - granteeList, err := s.getGranteeList(ownerCtx, owner.GetId(), e.GranteeUserID, e.GranteeGroupID) - if err != nil { - s.logger.Error().Err(err).Str("event", "ShareCreated").Msg("Could not get grantee list") + granteeList := s.ensureGranteeList(ownerCtx, owner.GetId(), e.GranteeUserID, e.GranteeGroupID) + if granteeList == nil { return } @@ -88,9 +87,8 @@ func (s eventsNotifier) handleShareExpired(e events.ShareExpired) { return } - granteeList, err := s.getGranteeList(ownerCtx, owner.GetId(), e.GranteeUserID, e.GranteeGroupID) - if err != nil { - s.logger.Error().Err(err).Str("event", "ShareExpired").Msg("Could not get grantee name") + granteeList := s.ensureGranteeList(ownerCtx, owner.GetId(), e.GranteeUserID, e.GranteeGroupID) + if granteeList == nil { return } diff --git a/services/notifications/pkg/service/spaces.go b/services/notifications/pkg/service/spaces.go index cd8f07278..3922c81ba 100644 --- a/services/notifications/pkg/service/spaces.go +++ b/services/notifications/pkg/service/spaces.go @@ -54,9 +54,8 @@ func (s eventsNotifier) handleSpaceShared(e events.SpaceShared) { // Note: We're using the 'executantCtx' (authenticated as the share executant) here for requesting // the Grantees of the shares. Ideally the notfication service would use some kind of service // user for this. - spaceGrantee, err := s.getGranteeList(executantCtx, executant.GetId(), e.GranteeUserID, e.GranteeGroupID) - if err != nil { - logger.Error().Err(err).Str("event", "SpaceGrantee").Msg("Could not get grantee list") + granteeList := s.ensureGranteeList(executantCtx, executant.GetId(), e.GranteeUserID, e.GranteeGroupID) + if granteeList == nil { return } @@ -67,7 +66,7 @@ func (s eventsNotifier) handleSpaceShared(e events.SpaceShared) { "SpaceSharer": sharerDisplayName, "SpaceName": resourceInfo.GetSpace().GetName(), "ShareLink": shareLink, - }, spaceGrantee, sharerDisplayName) + }, granteeList, sharerDisplayName) if err != nil { s.logger.Error().Err(err).Str("event", "SharedSpace").Msg("could not get render the email") return @@ -120,9 +119,8 @@ func (s eventsNotifier) handleSpaceUnshared(e events.SpaceUnshared) { // Note: We're using the 'executantCtx' (authenticated as the share executant) here for requesting // the Grantees of the shares. Ideally the notfication service would use some kind of service // user for this. - spaceGrantee, err := s.getGranteeList(executantCtx, executant.GetId(), e.GranteeUserID, e.GranteeGroupID) - if err != nil { - logger.Error().Err(err).Str("event", "SpaceGrantee").Msg("Could not get grantee list") + granteeList := s.ensureGranteeList(executantCtx, executant.GetId(), e.GranteeUserID, e.GranteeGroupID) + if granteeList == nil { return } @@ -133,7 +131,7 @@ func (s eventsNotifier) handleSpaceUnshared(e events.SpaceUnshared) { "SpaceSharer": sharerDisplayName, "SpaceName": resourceInfo.GetSpace().Name, "ShareLink": shareLink, - }, spaceGrantee, sharerDisplayName) + }, granteeList, sharerDisplayName) if err != nil { s.logger.Error().Err(err).Str("event", "UnsharedSpace").Msg("Could not get render the email") return @@ -159,9 +157,8 @@ func (s eventsNotifier) handleSpaceMembershipExpired(e events.SpaceMembershipExp return } - granteeList, err := s.getGranteeList(ownerCtx, owner.GetId(), e.GranteeUserID, e.GranteeGroupID) - if err != nil { - s.logger.Error().Err(err).Str("event", "SpaceUnshared").Msg("Could not get grantee list") + granteeList := s.ensureGranteeList(ownerCtx, owner.GetId(), e.GranteeUserID, e.GranteeGroupID) + if granteeList == nil { return }