Skip to content

Commit 9160b65

Browse files
wesleynwwesleynw
authored
fix: show comment image in notification (#537)
1 parent 478fa73 commit 9160b65

5 files changed

Lines changed: 64 additions & 235 deletions

File tree

‎Splajompy/Notifications/NotificationRow.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ struct NotificationRow: View {
6060
}
6161
}
6262

63-
if let comment = notification.comment {
63+
if let comment = notification.comment, !comment.text.isEmpty {
6464
MiniNotificationView(text: comment.text)
6565
} else if let post = notification.post, let text = post.text,
6666
!text.isEmpty

‎api/internal/handler/handler.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,6 @@ func (h *Handler) RegisterRoutes(handleFunc func(pattern string, handlerFunc fun
7979
handleFuncWithAuth("DELETE /posts/pin", h.UnpinPost)
8080

8181
// notifications
82-
handleFuncWithAuth("GET /notifications", h.GetAllNotificationByUserId)
83-
handleFuncWithAuth("GET /notifications/unread", h.GetUnreadNotificationsByUserId)
8482
handleFuncWithAuth("POST /notifications/markRead", h.MarkAllNotificationsAsRead)
8583
handleFuncWithAuth("POST /notifications/{id}/markRead", h.MarkNotificationAsReadById)
8684
handleFuncWithAuth("GET /notifications/hasUnread", h.HasUnreadNotifications)

‎api/internal/handler/notifications.go‎

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -10,35 +10,6 @@ import (
1010
"splajompy.com/api/v2/internal/utilities"
1111
)
1212

13-
// GetAllNotificationByUserId GET /notifications
14-
//
15-
// Deprecated: prefer other methods with cursor offset
16-
func (h *Handler) GetAllNotificationByUserId(w http.ResponseWriter, r *http.Request) {
17-
currentUser := h.getAuthenticatedUser(r)
18-
19-
offset := 0
20-
if offsetStr := r.URL.Query().Get("offset"); offsetStr != "" {
21-
if parsedOffset, err := strconv.Atoi(offsetStr); err == nil && parsedOffset >= 0 {
22-
offset = parsedOffset
23-
}
24-
}
25-
26-
limit := 20
27-
if limitStr := r.URL.Query().Get("limit"); limitStr != "" {
28-
if parsedLimit, err := strconv.Atoi(limitStr); err == nil && parsedLimit > 0 {
29-
limit = parsedLimit
30-
}
31-
}
32-
33-
notifications, err := h.notificationService.GetNotificationsByUserId(r.Context(), *currentUser, offset, limit)
34-
if err != nil {
35-
utilities.HandleError(w, http.StatusInternalServerError, "Something went wrong")
36-
return
37-
}
38-
39-
utilities.HandleSuccess(w, notifications)
40-
}
41-
4213
// MarkAllNotificationsAsRead POST /notifications/markRead
4314
func (h *Handler) MarkAllNotificationsAsRead(w http.ResponseWriter, r *http.Request) {
4415
currentUser := h.getAuthenticatedUser(r)
@@ -96,33 +67,6 @@ func (h *Handler) GetUnreadNotificationCount(w http.ResponseWriter, r *http.Requ
9667
utilities.HandleSuccess(w, count)
9768
}
9869

99-
// GetUnreadNotificationsByUserId GET /notifications/unread
100-
func (h *Handler) GetUnreadNotificationsByUserId(w http.ResponseWriter, r *http.Request) {
101-
currentUser := h.getAuthenticatedUser(r)
102-
103-
offset := 0
104-
if offsetStr := r.URL.Query().Get("offset"); offsetStr != "" {
105-
if parsedOffset, err := strconv.Atoi(offsetStr); err == nil && parsedOffset >= 0 {
106-
offset = parsedOffset
107-
}
108-
}
109-
110-
limit := 10
111-
if limitStr := r.URL.Query().Get("limit"); limitStr != "" {
112-
if parsedLimit, err := strconv.Atoi(limitStr); err == nil && parsedLimit > 0 {
113-
limit = parsedLimit
114-
}
115-
}
116-
117-
notifications, err := h.notificationService.GetUnreadNotificationsByUserId(r.Context(), *currentUser, offset, limit)
118-
if err != nil {
119-
utilities.HandleError(w, http.StatusInternalServerError, "Something went wrong")
120-
return
121-
}
122-
123-
utilities.HandleSuccess(w, notifications)
124-
}
125-
12670
// GetReadNotificationsByUserIdWithTimeOffset GET /notifications/read/time
12771
func (h *Handler) GetReadNotificationsByUserIdWithTimeOffset(w http.ResponseWriter, r *http.Request) {
12872
currentUser := h.getAuthenticatedUser(r)

‎api/internal/service/NotificationService.go‎

Lines changed: 14 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -29,56 +29,6 @@ func NewNotificationService(notificationRepository repositories.NotificationRepo
2929
}
3030
}
3131

32-
func (s *NotificationService) GetNotificationsByUserId(ctx context.Context, user models.PublicUser, offset int, limit int) ([]models.DetailedNotification, error) {
33-
notifications, err := s.notificationRepository.GetNotificationsForUserId(ctx, user.UserID, offset, limit)
34-
if err != nil {
35-
return nil, errors.New("unable to retrieve notifications")
36-
}
37-
38-
if notifications == nil {
39-
return []models.DetailedNotification{}, nil
40-
}
41-
42-
detailedNotifications := make([]models.DetailedNotification, 0, len(notifications))
43-
44-
for _, notification := range notifications {
45-
var detailedNotification models.DetailedNotification
46-
detailedNotification.Notification = *notification
47-
48-
if notification.PostID != nil {
49-
post, err := s.postRepository.GetPostById(ctx, *notification.PostID, user.UserID)
50-
if err != nil {
51-
return nil, errors.New("unable to retrieve post")
52-
}
53-
detailedNotification.Post = post
54-
55-
imageBlob, err := s.postRepository.GetImagesForPost(ctx, *notification.PostID)
56-
if err != nil && !errors.Is(err, sql.ErrNoRows) {
57-
return nil, errors.New("unable to retrieve image blob")
58-
}
59-
60-
if len(imageBlob) > 0 {
61-
url := "https://splajompy-bucket.nyc3.cdn.digitaloceanspaces.com/" + imageBlob[0].ImageBlobUrl
62-
detailedNotification.ImageBlob = &url
63-
detailedNotification.ImageWidth = &imageBlob[0].Width
64-
detailedNotification.ImageHeight = &imageBlob[0].Height
65-
}
66-
}
67-
68-
if notification.CommentID != nil {
69-
comment, err := s.commentRepository.GetCommentById(ctx, *notification.CommentID)
70-
if err != nil {
71-
return nil, errors.New("unable to retrieve comment")
72-
}
73-
detailedNotification.Comment = &comment
74-
}
75-
76-
detailedNotifications = append(detailedNotifications, detailedNotification)
77-
}
78-
79-
return detailedNotifications, nil
80-
}
81-
8232
func (s *NotificationService) MarkNotificationAsReadById(ctx context.Context, user models.PublicUser, notificationId int) error {
8333
notification, err := s.notificationRepository.GetNotificationById(ctx, notificationId)
8434
if err != nil {
@@ -108,57 +58,6 @@ func (s *NotificationService) GetUserUnreadNotificationCount(ctx context.Context
10858
return s.notificationRepository.GetUserUnreadNotificationCount(ctx, user.UserID)
10959
}
11060

111-
func (s *NotificationService) GetUnreadNotificationsByUserId(ctx context.Context, user models.PublicUser, offset int, limit int) ([]models.DetailedNotification, error) {
112-
notifications, err := s.notificationRepository.GetUnreadNotificationsForUserId(ctx, user.UserID, offset, limit)
113-
if err != nil {
114-
return nil, errors.New("unable to retrieve unread notifications")
115-
}
116-
117-
if notifications == nil {
118-
return []models.DetailedNotification{}, nil
119-
}
120-
121-
detailedNotifications := make([]models.DetailedNotification, 0, len(notifications))
122-
123-
// todo refactor this into a method, this is duplicated from above
124-
for _, notification := range notifications {
125-
var detailedNotification models.DetailedNotification
126-
detailedNotification.Notification = *notification
127-
128-
if notification.PostID != nil {
129-
post, err := s.postRepository.GetPostById(ctx, *notification.PostID, user.UserID)
130-
if err != nil {
131-
return nil, errors.New("unable to retrieve post")
132-
}
133-
detailedNotification.Post = post
134-
135-
imageBlob, err := s.postRepository.GetImagesForPost(ctx, *notification.PostID)
136-
if err != nil && !errors.Is(err, sql.ErrNoRows) {
137-
return nil, errors.New("unable to retrieve image blob")
138-
}
139-
140-
if len(imageBlob) > 0 {
141-
url := "https://splajompy-bucket.nyc3.cdn.digitaloceanspaces.com/" + imageBlob[0].ImageBlobUrl
142-
detailedNotification.ImageBlob = &url
143-
detailedNotification.ImageWidth = &imageBlob[0].Width
144-
detailedNotification.ImageHeight = &imageBlob[0].Height
145-
}
146-
}
147-
148-
if notification.CommentID != nil {
149-
comment, err := s.commentRepository.GetCommentById(ctx, *notification.CommentID)
150-
if err != nil {
151-
return nil, errors.New("unable to retrieve comment")
152-
}
153-
detailedNotification.Comment = &comment
154-
}
155-
156-
detailedNotifications = append(detailedNotifications, detailedNotification)
157-
}
158-
159-
return detailedNotifications, nil
160-
}
161-
16261
func (s *NotificationService) GetReadNotificationsByUserIdWithTimeOffset(ctx context.Context, user models.PublicUser, beforeTime time.Time, limit int, notificationType *string) ([]models.DetailedNotification, error) {
16362
notifications, err := s.notificationRepository.GetReadNotificationsForUserIdWithTimeOffset(ctx, user.UserID, beforeTime, limit, notificationType)
16463
if err != nil {
@@ -221,6 +120,20 @@ func (s *NotificationService) buildDetailedNotifications(ctx context.Context, cu
221120
return nil, errors.New("unable to retrieve comment")
222121
}
223122
detailedNotification.Comment = &comment
123+
124+
commentImages, err := s.commentRepository.GetImagesByCommentId(ctx, *notification.CommentID)
125+
if err != nil {
126+
return nil, errors.New("unable to retrieve comment images")
127+
}
128+
if len(commentImages) > 0 {
129+
presignedUrl, err := s.bucketRepository.GetPresignedGetObject(ctx, commentImages[0].ImageBlobUrl)
130+
if err != nil {
131+
return nil, errors.New("unable to presign comment image")
132+
}
133+
detailedNotification.ImageBlob = presignedUrl
134+
detailedNotification.ImageWidth = &commentImages[0].Width
135+
detailedNotification.ImageHeight = &commentImages[0].Height
136+
}
224137
}
225138

226139
if notification.TargetUserId != nil {

‎api/internal/service/NotificationService_test.go‎

Lines changed: 49 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
type notificationTestEnv struct {
1616
svc service.NotificationService
17+
commentSvc *service.CommentService
1718
notificationRepository repositories.NotificationRepository
1819
userRepository repositories.UserRepository
1920
postRepository repositories.PostRepository
@@ -28,59 +29,22 @@ func setupNotificationService(t *testing.T) notificationTestEnv {
2829
postRepository := repositories.NewDBPostRepository(testDb.Queries)
2930
notificationRepository := repositories.NewDBNotificationRepository(testDb.Queries)
3031
userRepository := repositories.NewDBUserRepository(testDb.Queries)
32+
likeRepository := repositories.NewDBLikeRepository(testDb.Queries)
33+
bucketRepository := &fakeBucketRepository{}
3134

32-
notificationService := service.NewNotificationService(notificationRepository, postRepository, commentRepository, userRepository, repositories.NewS3BucketRepository(nil))
35+
notificationService := service.NewNotificationService(notificationRepository, postRepository, commentRepository, userRepository, bucketRepository)
36+
commentService := service.NewCommentService(commentRepository, postRepository, notificationRepository, userRepository, likeRepository, bucketRepository)
3337

3438
return notificationTestEnv{
3539
svc: *notificationService,
40+
commentSvc: commentService,
3641
notificationRepository: notificationRepository,
3742
userRepository: userRepository,
3843
postRepository: postRepository,
3944
commentRepository: commentRepository,
4045
}
4146
}
4247

43-
func TestGetNotificationsByUserId(t *testing.T) {
44-
env := setupNotificationService(t)
45-
user := testutil.CreateTestUser(t, env.userRepository, "testUser")
46-
47-
notifications, err := env.svc.GetNotificationsByUserId(t.Context(), user, 0, 10)
48-
require.NoError(t, err)
49-
assert.NotNil(t, notifications)
50-
assert.Len(t, notifications, 0)
51-
52-
require.NoError(t, env.notificationRepository.InsertNotification(t.Context(), user.UserID, nil, nil, nil, "Test notification 1", models.NotificationTypeLike, nil))
53-
require.NoError(t, env.notificationRepository.InsertNotification(t.Context(), user.UserID, nil, nil, nil, "Test notification 2", models.NotificationTypeLike, nil))
54-
require.NoError(t, env.notificationRepository.InsertNotification(t.Context(), user.UserID, nil, nil, nil, "Test notification 3", models.NotificationTypeLike, nil))
55-
56-
notifications, err = env.svc.GetNotificationsByUserId(t.Context(), user, 0, 10)
57-
require.NoError(t, err)
58-
assert.NotNil(t, notifications)
59-
assert.Len(t, notifications, 3)
60-
61-
notifications, err = env.svc.GetNotificationsByUserId(t.Context(), user, 0, 2)
62-
require.NoError(t, err)
63-
assert.NotNil(t, notifications)
64-
assert.Len(t, notifications, 2)
65-
66-
notifications, err = env.svc.GetNotificationsByUserId(t.Context(), user, 2, 2)
67-
require.NoError(t, err)
68-
assert.NotNil(t, notifications)
69-
assert.Len(t, notifications, 1)
70-
71-
notifications, err = env.svc.GetNotificationsByUserId(t.Context(), user, 5, 2)
72-
require.NoError(t, err)
73-
assert.NotNil(t, notifications)
74-
assert.Len(t, notifications, 0)
75-
76-
anotherUser := testutil.CreateTestUser(t, env.userRepository, "anotherUser")
77-
78-
notifications, err = env.svc.GetNotificationsByUserId(t.Context(), anotherUser, 0, 10)
79-
require.NoError(t, err)
80-
assert.NotNil(t, notifications)
81-
assert.Len(t, notifications, 0)
82-
}
83-
8448
func TestMarkNotificationAsReadById(t *testing.T) {
8549
env := setupNotificationService(t)
8650
user := testutil.CreateTestUser(t, env.userRepository, "testUser")
@@ -99,10 +63,10 @@ func TestMarkNotificationAsReadById(t *testing.T) {
9963
err = env.svc.MarkNotificationAsReadById(t.Context(), user, notificationId)
10064
require.NoError(t, err)
10165

102-
notifications, err := env.svc.GetNotificationsByUserId(t.Context(), user, 0, 10)
66+
notifications, err := env.svc.GetReadNotificationsByUserIdWithTimeOffset(t.Context(), user, time.Now().UTC(), 10, nil)
10367
require.NoError(t, err)
10468
assert.NotNil(t, notifications)
105-
assert.Len(t, notifications, 1)
69+
require.Len(t, notifications, 1)
10670
assert.True(t, notifications[0].Viewed)
10771
}
10872

@@ -121,7 +85,7 @@ func TestMarkAllNotificationsAsReadForUserId(t *testing.T) {
12185
err = env.svc.MarkAllNotificationsAsReadForUserId(t.Context(), user)
12286
require.NoError(t, err)
12387

124-
notifications, err := env.svc.GetNotificationsByUserId(t.Context(), user, 0, 10)
88+
notifications, err := env.svc.GetReadNotificationsByUserIdWithTimeOffset(t.Context(), user, time.Now().UTC(), 10, nil)
12589
require.NoError(t, err)
12690
assert.NotNil(t, notifications)
12791
assert.Len(t, notifications, 3)
@@ -165,36 +129,6 @@ func TestUserHasUnreadNotifications(t *testing.T) {
165129
assert.False(t, hasUnread)
166130
}
167131

168-
func TestMultipleUsersNotifications(t *testing.T) {
169-
env := setupNotificationService(t)
170-
171-
user1 := testutil.CreateTestUser(t, env.userRepository, "user1")
172-
user2 := testutil.CreateTestUser(t, env.userRepository, "user2")
173-
174-
require.NoError(t, env.notificationRepository.InsertNotification(t.Context(), user1.UserID, nil, nil, nil, "User 1 notification 1", models.NotificationTypeLike, nil))
175-
require.NoError(t, env.notificationRepository.InsertNotification(t.Context(), user1.UserID, nil, nil, nil, "User 1 notification 2", models.NotificationTypeLike, nil))
176-
require.NoError(t, env.notificationRepository.InsertNotification(t.Context(), user2.UserID, nil, nil, nil, "User 2 notification 1", models.NotificationTypeLike, nil))
177-
178-
notificationsUser1, err := env.svc.GetNotificationsByUserId(t.Context(), user1, 0, 10)
179-
require.NoError(t, err)
180-
assert.Len(t, notificationsUser1, 2)
181-
182-
notificationsUser2, err := env.svc.GetNotificationsByUserId(t.Context(), user2, 0, 10)
183-
require.NoError(t, err)
184-
assert.Len(t, notificationsUser2, 1)
185-
186-
err = env.svc.MarkAllNotificationsAsReadForUserId(t.Context(), user1)
187-
require.NoError(t, err)
188-
189-
hasUnreadUser1, err := env.svc.UserHasUnreadNotifications(t.Context(), user1)
190-
require.NoError(t, err)
191-
assert.False(t, hasUnreadUser1)
192-
193-
hasUnreadUser2, err := env.svc.UserHasUnreadNotifications(t.Context(), user2)
194-
require.NoError(t, err)
195-
assert.True(t, hasUnreadUser2)
196-
}
197-
198132
func TestErrorWhenMarkingNonExistentNotification(t *testing.T) {
199133
env := setupNotificationService(t)
200134
user := testutil.CreateTestUser(t, env.userRepository, "testUser")
@@ -295,3 +229,43 @@ func TestGetComments_DoesNotFailLinkingToDeletedUsers(t *testing.T) {
295229
assert.NoError(t, err)
296230
assert.Len(t, notifications, 1)
297231
}
232+
233+
func TestCommentNotification_ImagePopulatedFromComment(t *testing.T) {
234+
env := setupNotificationService(t)
235+
236+
postOwner := testutil.CreateTestUser(t, env.userRepository, "postOwner")
237+
commenter := testutil.CreateTestUser(t, env.userRepository, "commenter")
238+
239+
visibility := models.VisibilityPublic
240+
post, err := env.postRepository.InsertPost(t.Context(), postOwner.UserID, "test post", nil, nil, &visibility)
241+
require.NoError(t, err)
242+
243+
imageKey := "images/test-image.jpg"
244+
imageKeyMap := map[int]models.ImageData{
245+
0: {S3Key: imageKey, Width: 640, Height: 480},
246+
}
247+
_, err = env.commentSvc.AddCommentToPost(t.Context(), commenter, post.PostID, "test comment with image", imageKeyMap)
248+
require.NoError(t, err)
249+
250+
notifications, err := env.svc.GetUnreadNotificationsByUserIdWithTimeOffset(t.Context(), postOwner, time.Now().UTC(), 10, nil)
251+
require.NoError(t, err)
252+
require.Len(t, notifications, 1)
253+
require.NotNil(t, notifications[0].ImageBlob, "imageBlob should be set from the comment image")
254+
assert.Equal(t, imageKey, *notifications[0].ImageBlob)
255+
assert.Equal(t, 640, *notifications[0].ImageWidth)
256+
assert.Equal(t, 480, *notifications[0].ImageHeight)
257+
258+
unread, err := env.svc.GetUnreadNotificationsByUserIdWithTimeOffset(t.Context(), postOwner, time.Now().UTC(), 10, nil)
259+
require.NoError(t, err)
260+
require.Len(t, unread, 1)
261+
require.NotNil(t, unread[0].ImageBlob)
262+
assert.Equal(t, imageKey, *unread[0].ImageBlob)
263+
264+
require.NoError(t, env.svc.MarkAllNotificationsAsReadForUserId(t.Context(), postOwner))
265+
266+
read, err := env.svc.GetReadNotificationsByUserIdWithTimeOffset(t.Context(), postOwner, time.Now().UTC(), 10, nil)
267+
require.NoError(t, err)
268+
require.Len(t, read, 1)
269+
require.NotNil(t, read[0].ImageBlob)
270+
assert.Equal(t, imageKey, *read[0].ImageBlob)
271+
}

0 commit comments

Comments
 (0)