[bugfix] Log + ignore unknown notification types (#3577)

* [bugfix] Log + ignore unknown notification types

* pass context to ParseNotificationTypes

3 files changed, 71 insertions, 62 deletions

diff --git a/internal/api/client/notifications/notificationsget.go b/internal/api/client/notifications/notificationsget.go
index 841768c63..7caadbe7d 100644
--- a/internal/api/client/notifications/notificationsget.go
+++ b/internal/api/client/notifications/notificationsget.go
@@ -18,11 +18,14 @@
 package notifications
 
 import (
+	"context"
 	"net/http"
 
 	"github.com/gin-gonic/gin"
 	apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
+	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
+	"github.com/superseriousbusiness/gotosocial/internal/log"
 	"github.com/superseriousbusiness/gotosocial/internal/oauth"
 	"github.com/superseriousbusiness/gotosocial/internal/paging"
 )
@@ -151,18 +154,6 @@ func (m *Module) NotificationsGETHandler(c *gin.Context) {
 		return
 	}
 
-	types, errWithCode := apiutil.ParseNotificationTypes(c.QueryArray(TypesKey))
-	if errWithCode != nil {
-		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
-		return
-	}
-
-	exclTypes, errWithCode := apiutil.ParseNotificationTypes(c.QueryArray(ExcludeTypesKey))
-	if errWithCode != nil {
-		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
-		return
-	}
-
 	page, errWithCode := paging.ParseIDPage(c,
 		1,  // min limit
 		80, // max limit
@@ -173,12 +164,13 @@ func (m *Module) NotificationsGETHandler(c *gin.Context) {
 		return
 	}
 
+	ctx := c.Request.Context()
 	resp, errWithCode := m.processor.Timeline().NotificationsGet(
-		c.Request.Context(),
+		ctx,
 		authed,
 		page,
-		types,
-		exclTypes,
+		ParseNotificationTypes(ctx, c.QueryArray(TypesKey)),        // Include types.
+		ParseNotificationTypes(ctx, c.QueryArray(ExcludeTypesKey)), // Exclude types.
 	)
 	if errWithCode != nil {
 		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
@@ -191,3 +183,28 @@ func (m *Module) NotificationsGETHandler(c *gin.Context) {
 
 	apiutil.JSON(c, http.StatusOK, resp.Items)
 }
+
+// ParseNotificationTypes converts the given slice of string values
+// to gtsmodel notification types, logging + skipping unknown types.
+func ParseNotificationTypes(
+	ctx context.Context,
+	values []string,
+) []gtsmodel.NotificationType {
+	if len(values) == 0 {
+		return nil
+	}
+
+	ntypes := make([]gtsmodel.NotificationType, 0, len(values))
+	for _, value := range values {
+		ntype := gtsmodel.NewNotificationType(value)
+		if ntype == gtsmodel.NotificationUnknown {
+			// Type we don't know about (yet), log and ignore it.
+			log.Debugf(ctx, "ignoring unknown type %s", value)
+			continue
+		}
+
+		ntypes = append(ntypes, ntype)
+	}
+
+	return ntypes
+}
diff --git a/internal/api/client/notifications/notificationsget_test.go b/internal/api/client/notifications/notificationsget_test.go
index 97d0e854b..5a6f83959 100644
--- a/internal/api/client/notifications/notificationsget_test.go
+++ b/internal/api/client/notifications/notificationsget_test.go
@@ -248,6 +248,45 @@ func (suite *NotificationsTestSuite) TestGetNotificationsIncludeOneType() {
 	}
 }
 
+// Test including an unknown notification type, it should be ignored.
+func (suite *NotificationsTestSuite) TestGetNotificationsIncludeUnknownType() {
+	testAccount := suite.testAccounts["local_account_1"]
+	testToken := suite.testTokens["local_account_1"]
+	testUser := suite.testUsers["local_account_1"]
+
+	suite.addMoreNotifications(testAccount)
+
+	maxID := ""
+	minID := ""
+	limit := 10
+	types := []string{"favourite", "something.weird"}
+	excludeTypes := []string(nil)
+	expectedHTTPStatus := http.StatusOK
+	expectedBody := ""
+
+	notifications, _, err := suite.getNotifications(
+		testAccount,
+		testToken,
+		testUser,
+		maxID,
+		minID,
+		limit,
+		types,
+		excludeTypes,
+		expectedHTTPStatus,
+		expectedBody,
+	)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	// This should only include the fav notification.
+	suite.Len(notifications, 1)
+	for _, notification := range notifications {
+		suite.Equal("favourite", notification.Type)
+	}
+}
+
 func TestBookmarkTestSuite(t *testing.T) {
 	suite.Run(t, new(NotificationsTestSuite))
 }
diff --git a/internal/api/util/parsequery.go b/internal/api/util/parsequery.go
index 9929524c5..9f4c02aed 100644
--- a/internal/api/util/parsequery.go
+++ b/internal/api/util/parsequery.go
@@ -18,13 +18,11 @@
 package util
 
 import (
-	"errors"
 	"fmt"
 	"strconv"
 	"strings"
 
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
-	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 )
 
 const (
@@ -218,51 +216,6 @@ func ParseInteractionReblogs(value string, defaultValue bool) (bool, gtserror.Wi
 	return parseBool(value, defaultValue, InteractionReblogsKey)
 }
 
-func ParseNotificationType(value string) (gtsmodel.NotificationType, gtserror.WithCode) {
-	switch strings.ToLower(value) {
-	case "follow":
-		return gtsmodel.NotificationFollow, nil
-	case "follow_request":
-		return gtsmodel.NotificationFollowRequest, nil
-	case "mention":
-		return gtsmodel.NotificationMention, nil
-	case "reblog":
-		return gtsmodel.NotificationReblog, nil
-	case "favourite":
-		return gtsmodel.NotificationFave, nil
-	case "poll":
-		return gtsmodel.NotificationPoll, nil
-	case "status":
-		return gtsmodel.NotificationStatus, nil
-	case "admin.sign_up":
-		return gtsmodel.NotificationSignup, nil
-	case "pending.favourite":
-		return gtsmodel.NotificationPendingFave, nil
-	case "pending.reply":
-		return gtsmodel.NotificationPendingReply, nil
-	case "pending.reblog":
-		return gtsmodel.NotificationPendingReblog, nil
-	default:
-		text := fmt.Sprintf("unrecognized notification type %s", value)
-		return 0, gtserror.NewErrorBadRequest(errors.New(text), text)
-	}
-}
-
-func ParseNotificationTypes(values []string) ([]gtsmodel.NotificationType, gtserror.WithCode) {
-	if len(values) == 0 {
-		return nil, nil
-	}
-	ntypes := make([]gtsmodel.NotificationType, len(values))
-	for i, value := range values {
-		ntype, errWithCode := ParseNotificationType(value)
-		if errWithCode != nil {
-			return nil, errWithCode
-		}
-		ntypes[i] = ntype
-	}
-	return ntypes, nil
-}
-
 /*
 	Parse functions for *REQUIRED* parameters.
 */