backstage-plugins icon indicating copy to clipboard operation
backstage-plugins copied to clipboard

Notifications: Unable to get group notifications

Open terencenjr opened this issue 1 year ago • 5 comments

Describe the bug

Group notifications are not showing for users belonging to the target group of the notification when viewed through the personal tab.

Expected Behavior

Group notifications should be shown together with the user's personal notifications.

What are the steps to reproduce this bug?

  1. Used Insomnia API platform to send a request to backend to create a group notification. The payload is as such:
{
    "title": "Group Notification",
    "message": "Testing",
    "origin": "example",
    "topic": "Notice",
    "targetGroups": [
        "default/example"
    ]
}
  1. Logged in to a user belonging to the example group
  2. Click on the notifications sidebar item

Versions of software used and environment

Backstage Version: 1.19.6

terencenjr avatar Feb 06 '24 06:02 terencenjr

I discovered the bug comes from the getUserGroups function in the backend side. When getting the group entity ref from the spec.memberOf, it comes with the 'group:' prefix (e.g. group:default/example). When querying the database, it does not match the group name stored, which is default/example. I suggest doing a slice to remove the 'group:' part before returning.

function getUserGroups(
  catalogClient: CatalogClient,
  user: string,
): Promise<string[]> {
  return catalogClient.getEntityByRef(`user:${user}`).then(userRef => {
    if (!userRef) {
      throw new Error(`user '${user}' does not exist`);
    }

    if (userRef.spec && Array.isArray(userRef.spec.memberOf)) {
      return userRef.spec.memberOf.map(value => {
        if (value) {
          return value.toString().slice('group:'.length); // << Added slice() to remove the group prefix
        }
        return '';
      });
    }

    return [];
  });
}

terencenjr avatar Feb 06 '24 06:02 terencenjr

cc @mareklibra @ydayagi ^^

invincibleJai avatar Feb 19 '24 06:02 invincibleJai

the 'group' string has to be there. it is part of an entity ref. an entity ref is just like k8s resources. kind:namespace:name.

check that in the catalog you have a group in namespace default. how was the user added to the group?

ydayagi avatar Feb 26 '24 19:02 ydayagi

Hi @ydayagi, I agree that the entityRef should follow the given format. The issue I am raising is towards the mismatch of the group entityRef format that is retrieved from the memberOf property and the one that is stored in the DB. Looking at the createNotifications function, the target group is retrieved from the request and stored in the DB without prepending 'group:'.

There are two ways of doing this:

  1. Remove the 'group:' when retrieving the group entity ref (which I have suggested)
  2. Prepend the 'group:' prefix to the target group before storing in DB (I don't recommend this to ensure consistency with the user notification side)

terencenjr avatar Feb 27 '24 02:02 terencenjr

@terencenjr fixed in PR https://github.com/janus-idp/backstage-plugins/pull/1273

ydayagi avatar Feb 27 '24 04:02 ydayagi

This issue has been closed due to the fact that the Janus community is being sunset.

For future plugin issues, please use https://github.com/backstage/community-plugins/issues

For future showcase issues, please use https://issues.redhat.com/browse/RHIDP

For more information on the sunset, see:

https://janus-idp.io/blog/2024/07/05/future-of-janus-community https://issues.redhat.com/browse/RHIDP-3690 https://issues.redhat.com/browse/RHIDP-1018

rhdh-bot avatar Sep 03 '24 17:09 rhdh-bot