From 642e39be2944115b8d48b48e2834b4989ece27e0 Mon Sep 17 00:00:00 2001
From: Brian Phillips <28457+brianphillips@users.noreply.github.com>
Date: Fri, 17 Mar 2023 08:54:24 -0500
Subject: [PATCH] Apply code review feedback

Allow more flexible comparisons where kind is not defined or the entity
being compared against has some non-string value for `kind` or `spec.type`.

Signed-off-by: Brian Phillips <28457+brianphillips@users.noreply.github.com>
---
 .changeset/soft-roses-cover.md                |  2 +-
 .../EntitySwitch/conditions.test.ts           |  8 ++++++
 .../src/components/EntitySwitch/conditions.ts | 27 ++++++++-----------
 .../src/components/EntitySwitch/index.ts      |  2 ++
 4 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/.changeset/soft-roses-cover.md b/.changeset/soft-roses-cover.md
index 50a328729a..94cbe6d0bd 100644
--- a/.changeset/soft-roses-cover.md
+++ b/.changeset/soft-roses-cover.md
@@ -2,4 +2,4 @@
 '@backstage/plugin-catalog': minor
 ---
 
-Added isEntityWith condition helper
+Added `isEntityWith` condition helper for `EntitySwitch` case statements.
diff --git a/plugins/catalog/src/components/EntitySwitch/conditions.test.ts b/plugins/catalog/src/components/EntitySwitch/conditions.test.ts
index 76e9e7a7a8..66408aec18 100644
--- a/plugins/catalog/src/components/EntitySwitch/conditions.test.ts
+++ b/plugins/catalog/src/components/EntitySwitch/conditions.test.ts
@@ -142,6 +142,14 @@ describe('isEntityWith', () => {
     const checkEntity = isEntityWith({ kind: 'another-type', type: 'service' });
     expect(checkEntity(missingSpecType)).not.toBeTruthy();
   });
+  it('allows a check against no criteria', () => {
+    const checkEntity = isEntityWith({});
+    expect(checkEntity(apiKind)).toBeTruthy();
+  });
+  it('allows a check against empty criteria', () => {
+    const checkEntity = isEntityWith({ kind: [], type: [] });
+    expect(checkEntity(apiKind)).toBeTruthy();
+  });
 });
 
 describe('isKind', () => {
diff --git a/plugins/catalog/src/components/EntitySwitch/conditions.ts b/plugins/catalog/src/components/EntitySwitch/conditions.ts
index 5442a955ae..02d7cf9bcf 100644
--- a/plugins/catalog/src/components/EntitySwitch/conditions.ts
+++ b/plugins/catalog/src/components/EntitySwitch/conditions.ts
@@ -17,20 +17,22 @@
 import { Entity } from '@backstage/catalog-model';
 
 export interface EntityPredicates {
-  kind: string | string[];
+  kind?: string | string[];
   type?: string | string[];
 }
 
-function strCmp(a: string | undefined, b: string | undefined): boolean {
+function strCmp(a: unknown, b: string | undefined): boolean {
   return Boolean(
-    a && a?.toLocaleLowerCase('en-US') === b?.toLocaleLowerCase('en-US'),
+    a &&
+      typeof a === 'string' &&
+      a?.toLocaleLowerCase('en-US') === b?.toLocaleLowerCase('en-US'),
   );
 }
 
-function strCmpAll(value: string | undefined, cmpValues: string | string[]) {
+function strCmpAll(value: unknown, cmpValues: string | string[]) {
   return typeof cmpValues === 'string'
     ? strCmp(value, cmpValues)
-    : cmpValues.some(cmpVal => strCmp(value, cmpVal));
+    : cmpValues.length === 0 || cmpValues.some(cmpVal => strCmp(value, cmpVal));
 }
 
 /**
@@ -58,28 +60,21 @@ export function isResourceType(types: string | string[]) {
 }
 
 /**
- *
  * For use in EntitySwitch.Case. Matches if the entity is the specified kind and type (if present).
  * @public
  */
 export function isEntityWith(predicate: EntityPredicates) {
   return (entity: Entity) => {
-    if (!strCmpAll(entity.kind, predicate.kind)) {
+    if (predicate.kind && !strCmpAll(entity.kind, predicate.kind)) {
       return false;
     }
 
-    if (
-      !predicate.type ||
-      (Array.isArray(predicate.type) && predicate.type.length === 0)
-    ) {
-      // there's no type check, return true
-      return true;
-    } else if (typeof entity.spec?.type !== 'string') {
-      // we'll reject any entity's with some non-string (including undefined/null) spec.type value
+    if (predicate.type && !strCmpAll(entity.spec?.type, predicate.type)) {
       return false;
     }
 
-    return strCmpAll(entity.spec.type, predicate.type);
+    // there's no type check, return true
+    return true;
   };
 }
 
diff --git a/plugins/catalog/src/components/EntitySwitch/index.ts b/plugins/catalog/src/components/EntitySwitch/index.ts
index 0cb85ae6eb..8848f98866 100644
--- a/plugins/catalog/src/components/EntitySwitch/index.ts
+++ b/plugins/catalog/src/components/EntitySwitch/index.ts
@@ -16,9 +16,11 @@
 
 export { EntitySwitch } from './EntitySwitch';
 export type { EntitySwitchProps, EntitySwitchCaseProps } from './EntitySwitch';
+export type { EntityPredicates } from './conditions';
 export {
   isKind,
   isNamespace,
   isComponentType,
   isResourceType,
+  isEntityWith,
 } from './conditions';