Improvement for notifications related to admin/tech contacts

[nicholaspcr]

Summary

From internal discussions the suggestion of not fanning out the notification to an organization's collaborators. Instead, if an organization is specified as an Admin/Tech contact of another entity, we would propagate the notification to the organization's respective Admin/Tech contact.

For example:

• org-x is the admin_contact of an application.
• org-x has [usr-1,usr-2,usr-3] as collaborators.
• org-x has usr-Y as its admin_contact

With the suggested changes instead of fanning out the administrative notification to [usr-1,usr-2,usr-3] we would send it to usr-Y.

Current Situation

Currently when creating an notification we go through the lookupNotificationReceivers in order to know who are the receivers.

Flow of the method:

1. We fetch the admin/tech contacts from an entity and add it to the list of receivers.
   

   lorawan-stack/pkg/identityserver/notification_registry.go

   Lines 90 to 130 in fe015cd

   	var entityMask []string
   	if receiversContains(req.Receivers, ttnpb.NotificationReceiver_NOTIFICATION_RECEIVER_ADMINISTRATIVE_CONTACT) {
   	entityMask = append(entityMask, "administrative_contact")
   	}
   	if receiversContains(req.Receivers, ttnpb.NotificationReceiver_NOTIFICATION_RECEIVER_TECHNICAL_CONTACT) {
   	entityMask = append(entityMask, "technical_contact")
   	}
   	if len(entityMask) > 0 {
   	var (
   	entity interface {
   	GetAdministrativeContact() *ttnpb.OrganizationOrUserIdentifiers
   	GetTechnicalContact() *ttnpb.OrganizationOrUserIdentifiers
   	}
   	err error
   	)
   	switch req.EntityIds.EntityType() {
   	default:
   	// Entity doesn't have contacts. Just ignore.
   	case store.EntityApplication:
   	entity, err = st.GetApplication(ctx, req.EntityIds.GetApplicationIds(), entityMask)
   	case store.EntityClient:
   	entity, err = st.GetClient(ctx, req.EntityIds.GetClientIds(), entityMask)
   	case store.EntityEndDevice:
   	entity, err = st.GetApplication(ctx, req.EntityIds.GetDeviceIds().GetApplicationIds(), entityMask)
   	case store.EntityGateway:
   	entity, err = st.GetGateway(ctx, req.EntityIds.GetGatewayIds(), entityMask)
   	case store.EntityOrganization:
   	entity, err = st.GetOrganization(ctx, req.EntityIds.GetOrganizationIds(), entityMask)
   	}
   	if err != nil {
   	return err
   	}
   	if entity != nil { // NOTE: entity is nil for entities that don't support contacts.
   	if receiversContains(req.Receivers, ttnpb.NotificationReceiver_NOTIFICATION_RECEIVER_ADMINISTRATIVE_CONTACT) && entity.GetAdministrativeContact() != nil {
   	receiverIDs = append(receiverIDs, entity.GetAdministrativeContact())
   	}
   	if receiversContains(req.Receivers, ttnpb.NotificationReceiver_NOTIFICATION_RECEIVER_TECHNICAL_CONTACT) && entity.GetTechnicalContact() != nil {
   	receiverIDs = append(receiverIDs, entity.GetTechnicalContact())
   	}
   	}
   	}

2. Afterwards we check if any of the receivers is an organization and fetch its members as receivers:
   

   lorawan-stack/pkg/identityserver/notification_registry.go

   Lines 148 to 160 in fe015cd

   	// Expand organization IDs to organization collaborator IDs.
   	for _, ids := range uniqueOrganizationOrUserIdentifiers(ctx, receiverIDs) {
   	if ids.EntityType() != store.EntityOrganization {
   	continue
   	}
   	members, err := st.FindMembers(ctx, ids.GetEntityIdentifiers())
   	if err != nil {
   	return err
   	}
   	for _, v := range members {
   	receiverIDs = append(receiverIDs, v.Ids)
   	}
   	}

So currently when an organization is an admin/tech contact we always fetch the members as the notification receivers.

Why do we need this? Who uses it, and when?

The idea is to improve the notification system by avoiding excessive emails from being sent or try to find a way that its clearer for the user to organize its admin/tech contacts.

Proposed Implementation

The initial idea is to replace the act of fetching the organization's members and use its own admin/tech contact.

The problem here being that this disallows the (1:N) relationship that exists between a notification and its receivers.

So this issue needs a bit more of discussion regarding:

1. Is this a good improvement?
2. Is the way we create sets of notification receivers clear for the users?

cc: @adriansmares @KrishnaIyer

Contributing

• I can help by doing more research.
• I can help by implementing the feature after the proposal above is approved.
• I can help by testing the feature before it's released.

Code of Conduct

• I agree to follow TTN's Community Code of Conduct.

[adriansmares]

So for the record even today it is possible to have a subset of the organization members receive the notifications, by creating a secondary organization only with the member subset, adding it as a collaborator, then setting it as a contact. It has the annoying ergonomics of having to add a secondary collaborator, which you can forget to update and maintain.

At the same time, it is equally annoying to have to fanout notifications to a 10 members organization, even if they don't want to receive them. Setting an individual user as a contact for these entities is problematic because replacing it would mean updating each individual entity.

I would propose that we add a field in the organization (fanout_notifications, a BoolValue, initially experimental) which controls this behavior - all of existing organizations should have it set to true while adding the column, for backwards compatibility, but for newer organizations I would leave this as false.

From there it should be easy - instead of FindMembers we retrieve the organization in order to check the fanout_notifications, administrative_contact and technical_contact, and based on fanout_notifications we either append only the contacts or every collaborator.

[nicholaspcr]

So for the record even today it is possible to have a subset of the organization members receive the notifications, by creating a secondary organization only with the member subset, adding it as a collaborator, then setting it as a contact. It has the annoying ergonomics of having to add a secondary collaborator, which you can forget to update and maintain.

This is not possible because a admin/tech contact has to be an organization's collaborator and nested organizations aren't allowed.

Please disregard, wrote it as I was under the impression that you were talking about doing the operation on a organization entity but the operations described are valid in entities besides the organization.

[nicholaspcr]

With my misunderstanding clarified I do agree with the idea of having the fanout_notifications field.

[nicholaspcr]

With the improvement defined, it does require a migration but its quick work.
So I believe that putting it on the v3.29.0 milestone works fine.

[KrishnaIyer]

I read through the issue and ACK with the design. Btw very good issue writing @nicholaspcr.