seatgeek / backstage-plugins

SeatGeek Backstage Plugins Collection
Apache License 2.0
34 stars 6 forks source link

slack-catalog: conditional image update #53

Closed nikolaik closed 7 months ago

nikolaik commented 8 months ago

We want to use your slack-plugin-catalog plugin, but have this use case were we want to conditionally update slack profile images. We have an authoritative source for our users where most users have an image. In the case were there is no image in the authoritative source, we want to use the slack avatar instead. So we run your code with a somewhat hacky patch similar to this:

export class SlackUserProcessor implements CatalogProcessor {
    //...
    if (slackUser.profile?.image_192 && (!entity.spec.profile?.picture || entityHasSlackPicture(entity))) {
      entity.spec.profile.picture = slackUser.profile.image_192;
    }
    //...
}

function entityHasSlackPicture(entity: UserEntity) {
  return entity.spec.profile?.picture?.includes('slack-edge.com');
}

The idea here is to allow updating when

Would you accept a PR for adding this as an option?

zhammer commented 7 months ago

apologies for the delay & thanks for the issue. i think actually all we have to do here is update the following https://github.com/seatgeek/backstage-plugins/blob/0c99da52fa7dce1f35cc631652d7987ae711aa2a/plugins/slack-catalog-backend/src/SlackUserProcessor.ts#L141-L143 to be

    // if the user entity doesn't already have a profile picture set, *and* there's a slack avatar for the user, add that.
    if (!entity.spec.profile.picture && slackUser.profile?.image_192) {
      entity.spec.profile.picture = slackUser.profile.image_192;
    }

i'm a bit confused about why we'd need to logic "there is an existing slack avatar-ish URL in use". it took me a bit to realize this myself in writing custom plugins, but the way the entity lifecycle works is thus:

so in this case you don't have to check if there's a slack avatar-ish url since that won't persist from the last processor run. all you have to do is have the slack processor run after your authoritative source avatar has been added (either when the entity is provided or from some previous processor)

does that make sense? am i missing something?

nikolaik commented 7 months ago

Thank you for explaining @zhammer ! This simplifies things <3 I also found and read through https://backstage.io/docs/features/software-catalog/life-of-an-entity/ now.

So is it OK to make this the default behaviour for the plugin and avoid having it as an option? If so I'll make a PR right away

zhammer commented 7 months ago

yes please do!