feat: add notifications filtering by processors

[ydayagi]

Hey, I just made a Pull Request!

Allow processors to distinguish low and high-importance notifications The first use would be for email processor Users usually want only high-importance notifications in their emails Implementation is based on this suggestion https://github.com/backstage/backstage/pull/24322#issuecomment-2075131200

[backstage-goalie[bot]]

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage/plugin-notifications-backend-module-email	plugins/notifications-backend-module-email	minor	v0.0.1
@backstage/plugin-notifications-backend	plugins/notifications-backend	minor	v0.2.1
@backstage/plugin-notifications-node	plugins/notifications-node	minor	v0.1.4

[drodil]

Missing changeset + API report updates

[ydayagi]

Missing changeset + API report updates

fixed

[ydayagi]

@drodil I made the requested changes. who approves/merges? you?

[drodil]

@ydayagi no I cannot do that, it's either @mareklibra @Rugvip or @freben

[Rugvip]

@drodil is it good to ship from your PoV?

[drodil]

Hm. It's not a major release for starters. Also not sure if the naming convention is understandable as there are no docs. Maybe also there could be other filters included in the first place and not just to get one specific functionality to work; maximumSeverity, topics from the top of my head.

Additionally this change itself does nothing yet and additional changes are required to the actual processors to make this work but that can be done in a separate PR.

[ydayagi]

Hm. It's not a major release for starters. Also not sure if the naming convention is understandable as there are no docs. Maybe also there could be other filters included in the first place and not just to get one specific functionality to work; maximumSeverity, topics from the top of my head.

Additionally this change itself does nothing yet and additional changes are required to the actual processors to make this work but that can be done in a separate PR.

there is no real use for filters other than minSeverity at the moment. after this is merged I will create PR for email processor to use minSeverity filter. I don't see any real life use case for maxSeverity nor for topics

[drodil]

@ydayagi you mean you don't have a use case for those filters? Backstage is a framework and should be developed as such.

[ydayagi]

@ydayagi you mean you don't have a use case for those filters? Backstage is a framework and should be developed as such.

You are right. I meant that maxSeverity to my knowledge/experience might not have a legitimate use case. But you are right. It is better to have it. Lets be productive here. What would you like to add in order to have this merged and used by all of backstage users? Topics and maxSeverity? There are string fields such as scope and topic. What about replacing the topic filter with a 'text filter' that searches all descriptive string fields? I mean title description scope topic. I think that nowadays people do a more broad text search rather then a specific one. @Rugvip WDYT too?

[ydayagi]

@mareklibra @drodil @Rugvip I added all filters suggested by drodil plus implementation in email processor

[mareklibra]

Honestly, I do not see a use-case for the maxSeverity either, but there is just no harm in having it.

Once we have several processors implemented, such filtering will be copy-pasted in each of them. I would rather see the filtering logic to be shared and implemented once before passing a notification down to a processor.

I will not block merging of this PR on that, let's keep this idea for a future refactoring.

[mareklibra]

It LGTM, just the change is minor, not major (pushing a commit changing it)

[github-actions[bot]]

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.28.0 release, scheduled for Tue, 18 Jun 2024.

[github-actions[bot]]

Uffizzi Cluster pr-24731 was deleted.