Skip to content

Commit c7909e7

Browse files
Matovidloclaude
andcommitted
fix(notifications): address claude[bot] review feedback
- Check filter operator (==) when matching subscription to parent config, preventing false attribution for inequality filters on job.configuration.id - Replace fragile strings.Contains("400") error detection with proper errors.As(err, &NotificationError) + StatusCode() == 400 check - Call ValidateNotificationFilters from MapAfterLocalLoad so invalid filter field names are caught at load time instead of being dead code - Sort suggestions in findSimilarFieldNames for deterministic output Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent f1ca8fa commit c7909e7

3 files changed

Lines changed: 8 additions & 5 deletions

File tree

internal/pkg/mapper/notification/local_load.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ func (m *mapper) MapAfterLocalLoad(ctx context.Context, recipe *model.LocalLoadR
1818
errs := errors.NewMultiError()
1919
if err := m.loadConfigFile(ctx, recipe, notification); err != nil {
2020
errs.Append(err)
21+
} else if err := model.ValidateNotificationFilters(notification.Filters); err != nil {
22+
errs.Append(err)
2123
}
2224
if err := m.loadMetaFile(ctx, recipe); err != nil {
2325
errs.Append(err)

internal/pkg/model/notification_validation.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package model
22

33
import (
44
"slices"
5+
"sort"
56
"strings"
67

78
"github.com/keboola/keboola-sdk-go/v2/pkg/keboola"
@@ -110,5 +111,6 @@ func findSimilarFieldNames(input string) []string {
110111
}
111112
}
112113

114+
sort.Strings(suggestions)
113115
return suggestions
114116
}

internal/pkg/state/remote/notification.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package remote
22

33
import (
44
"context"
5-
"strings"
65
"time"
76

87
"github.com/keboola/keboola-sdk-go/v2/pkg/keboola"
@@ -36,8 +35,8 @@ func (u *UnitOfWork) loadNotifications(ctx context.Context) error {
3635
// Find which config this notification belongs to by checking filters
3736
var parentConfig *model.ConfigManifest
3837
for _, filter := range apiSub.Filters {
39-
if filter.Field == "job.configuration.id" {
40-
// Found the config ID filter - look up the config
38+
if filter.Field == "job.configuration.id" && filter.Operator == keboola.NotificationFilterOperatorEquals {
39+
// Found the config ID equality filter - look up the config
4140
configID := keboola.ConfigID(filter.Value)
4241
if cfg, found := configsByID[configID]; found {
4342
parentConfig = cfg
@@ -143,8 +142,8 @@ func (u *UnitOfWork) buildNotificationCreateRequest(
143142
) request.APIRequest[*keboola.NotificationSubscription] {
144143
return u.createNotificationRequest(notification).Build().
145144
WithOnError(func(_ context.Context, err error) error {
146-
errMsg := err.Error()
147-
if strings.Contains(errMsg, "400") || strings.Contains(errMsg, "Bad Request") {
145+
var notifErr *keboola.NotificationError
146+
if errors.As(err, &notifErr) && notifErr.StatusCode() == 400 {
148147
return errors.Errorf(
149148
`failed to create notification "%s": %w. `+
150149
`This may be caused by invalid filter field names. `+

0 commit comments

Comments
 (0)