Skip to content

feat: Create/Update changes for RSS feed notif channel support#2397

Open
neilv-g wants to merge 1 commit intoneilv-rss-2from
neilv-rss-3
Open

feat: Create/Update changes for RSS feed notif channel support#2397
neilv-g wants to merge 1 commit intoneilv-rss-2from
neilv-rss-3

Conversation

@neilv-g
Copy link
Copy Markdown
Collaborator

@neilv-g neilv-g commented Apr 7, 2026

The Read and Delete NotifChannel APIs should work as is and not need any changes for RSS feeds.

The Read and Delete NotifChannel APIs should work as is and not need any changes for RSS feeds.
@neilv-g neilv-g requested a review from jcscottiii April 7, 2026 16:57
Copy link
Copy Markdown
Collaborator

@jcscottiii jcscottiii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with two nits to help. Feel free to do those now or in a follow up.

Comment on lines +52 to 56
} else if cfg, err := request.Config.AsRSSConfig(); err == nil &&
cfg.Type == backend.RSSConfigTypeRss {
// RSS channels currently have no configuration fields to validate.
_ = cfg
} else {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See other note

}
} else if cfg, err := input.Config.AsRSSConfig(); err == nil && cfg.Type == backend.RSSConfigTypeRss {
// RSS channels currently have no configuration fields to validate.
_ = cfg
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's time we leverage the switch case (and get coverage from the exhaustive) linter. Feel free to do this in this PR or a follow up:

func validateUpdateNotificationChannel(request *backend.UpdateNotificationChannelRequest) fieldErrors {
	var fieldErrors &fieldValidationErrors{fieldErrorMap: nil} 

	if len(input.Name) < notificationChannelNameMinLength || len(input.Name) > notificationChannelNameMaxLength {
		fieldErrors.addFieldError("name", errNotificationChannelInvalidNameLength)
	}

	// 1. Get the explicit type discriminator from the JSON
	discriminator, err := request.Config.Discriminator()
	if err != nil {
		fieldErrors.addFieldError("config", errors.New("invalid configuration structure"))
		return fieldErrors
	}

	// 2. Switch on the type (cast to enum for exhaustiveness and type safety)
	switch backend.NotificationChannelType(discriminator) {

	case backend.NotificationChannelTypeRss:
		// Valid! RSS channels currently have no configuration fields to validate.
		// No variable is created, so no unused variable issue.

	case backend.NotificationChannelTypeWebhook:
		cfg, err := request.Config.AsWebhookConfig()
		if err != nil {
			fieldErrors.addFieldError("config", errors.New("invalid webhook config"))
			break
		}
		if err := httputils.ValidateSlackWebhookURL(cfg.Url); err != nil {
			fieldErrors.addFieldError("config.url", err)
		}

	case backend.NotificationChannelTypeEmail:
		// Explicitly fall through to default since email channels
		// cannot be managed manually via this endpoint.
		fallthrough

	default:
		fieldErrors.addFieldError("config", errors.New("invalid config: only webhook or rss updates are supported"))
	}

	if fieldErrors.hasErrors() {
		return fieldErrors
	}

	return nil
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants