fix: adds additional check for dangling enrollments (#1296)

## Filter dangling enrollments in getAllEnrollment

### Summary
Added validation to filter out dangling site enrollments - enrollments
that reference sites belonging to a different organization than the
entitlement.

### Changes
- Fetches associated sites using `batchGetByKeys` after retrieving
enrollments
- Filters out dangling enrollments where site's orgId doesn't match
entitlement's orgId
- Logs warnings with details (enrollmentId, siteId, siteOrgId,
expectedOrgId) for monitoring
- Also handles orphaned enrollments where the site no longer exists

### Why
Dangling enrollments can occur when sites are moved between
organizations but their enrollments aren't cleaned up. This fix ensures
the API returns only valid enrollments, preventing issues downstream.

Please ensure your pull request adheres to the following guidelines:
- [ ] make sure to link the related issues in this description
- [ ] when merging / squashing, make sure the fixed issue references are
visible in the commits, for easy compilation of release notes

## Related Issues


Thanks for contributing!

Author: sandsinh

packages/spacecat-shared-tier-client/src/tier-client.js

@@ -220,6 +220,7 @@ class TierClient {
    * Gets all enrollments based on context, filtered by productCode.
    * - If site is provided: returns site enrollment for the entitlement matching productCode
    * - If org-only: returns all site enrollments for the entitlement matching productCode
+   * - Filters out enrollments where the site's orgId doesn't match the entitlement's orgId
    * @returns {Promise<object>} Object with entitlement and enrollments array.
    */
   async getAllEnrollment() {
@@ -234,16 +235,41 @@ class TierClient {
 
       const allEnrollments = await this.SiteEnrollment.allByEntitlementId(entitlement.getId());
 
+      if (allEnrollments.length === 0) {
+        return { entitlement, enrollments: [] };
+      }
+
+      // Fetch all sites using batchGetByKeys
+      const siteKeys = allEnrollments.map((enrollment) => ({ siteId: enrollment.getSiteId() }));
+      const sitesResult = await this.Site.batchGetByKeys(siteKeys);
+      const sitesMap = new Map(sitesResult.data.map((site) => [site.getId(), site]));
+
+      // Filter enrollments where site's orgId matches the entitlement's orgId
+      const validEnrollments = [];
+
+      for (const enrollment of allEnrollments) {
+        const site = sitesMap.get(enrollment.getSiteId());
+        if (!site) {
+          // Site not found, log warning and skip
+          this.log.warn(`Site not found for enrollment ${enrollment.getId()} with siteId ${enrollment.getSiteId()}`);
+        } else {
+          const siteOrgId = site.getOrganizationId();
+          if (siteOrgId === orgId) {
+            validEnrollments.push(enrollment);
+          }
+        }
+      }
+
       if (this.site) {
         // Return site enrollments matching the entitlement and site
         const siteId = this.site.getId();
-        const matchingEnrollments = allEnrollments.filter(
+        const matchingEnrollments = validEnrollments.filter(
           (se) => se.getSiteId() === siteId,
         );
         return { entitlement, enrollments: matchingEnrollments };
       } else {
-        // Return all enrollments for the entitlement
-        return { entitlement, enrollments: allEnrollments };
+        // Return all valid enrollments for the entitlement
+        return { entitlement, enrollments: validEnrollments };
       }
     } catch (error) {
       this.log.error(`Error getting all enrollments: ${error.message}`);

packages/spacecat-shared-tier-client/test/tier-client.test.js

@@ -80,6 +80,7 @@ describe('TierClient', () => {
     },
     Site: {
       findById: sandbox.stub(),
+      batchGetByKeys: sandbox.stub(),
     },
   };
 
@@ -726,11 +727,25 @@ describe('TierClient', () => {
         getEntitlementId: () => 'entitlement-123',
       };
 
+      const mockSiteForEnrollment1 = {
+        getId: () => siteId,
+        getOrganizationId: () => orgId,
+      };
+
+      const mockSiteForEnrollment2 = {
+        getId: () => '789-site-id',
+        getOrganizationId: () => orgId,
+      };
+
       mockDataAccess.Entitlement.findByOrganizationIdAndProductCode.resolves(mockEntitlement);
       mockDataAccess.SiteEnrollment.allByEntitlementId.resolves([
         mockSiteEnrollment,
         mockEnrollment2,
       ]);
+      mockDataAccess.Site.batchGetByKeys.resolves({
+        data: [mockSiteForEnrollment1, mockSiteForEnrollment2],
+        unprocessed: [],
+      });
 
       const result = await tierClientWithoutSite.getAllEnrollment();
 
@@ -742,6 +757,10 @@ describe('TierClient', () => {
         .to.have.been.calledWith(orgId, productCode);
       expect(mockDataAccess.SiteEnrollment.allByEntitlementId)
         .to.have.been.calledWith('entitlement-123');
+      expect(mockDataAccess.Site.batchGetByKeys).to.have.been.calledWith([
+        { siteId },
+        { siteId: '789-site-id' },
+      ]);
     });
 
     it('should return filtered enrollments when site is provided', async () => {
@@ -751,11 +770,25 @@ describe('TierClient', () => {
         getEntitlementId: () => 'entitlement-123',
       };
 
+      const mockSiteForEnrollment1 = {
+        getId: () => siteId,
+        getOrganizationId: () => orgId,
+      };
+
+      const mockSiteForEnrollment2 = {
+        getId: () => 'other-site-id',
+        getOrganizationId: () => orgId,
+      };
+
       mockDataAccess.Entitlement.findByOrganizationIdAndProductCode.resolves(mockEntitlement);
       mockDataAccess.SiteEnrollment.allByEntitlementId.resolves([
         mockSiteEnrollment,
         mockEnrollment2,
       ]);
+      mockDataAccess.Site.batchGetByKeys.resolves({
+        data: [mockSiteForEnrollment1, mockSiteForEnrollment2],
+        unprocessed: [],
+      });
 
       const result = await tierClient.getAllEnrollment();
 
@@ -811,18 +844,96 @@ describe('TierClient', () => {
         getEntitlementId: () => 'entitlement-123',
       };
 
+      const mockSiteForEnrollment1 = {
+        getId: () => siteId,
+        getOrganizationId: () => orgId,
+      };
+
+      const mockSiteForEnrollment2 = {
+        getId: () => 'different-site-id',
+        getOrganizationId: () => orgId,
+      };
+
       mockDataAccess.Entitlement.findByOrganizationIdAndProductCode.resolves(mockEntitlement);
       mockDataAccess.SiteEnrollment.allByEntitlementId.resolves([
         mockSiteEnrollment,
         mockEnrollment2,
         mockEnrollment3,
       ]);
+      mockDataAccess.Site.batchGetByKeys.resolves({
+        data: [mockSiteForEnrollment1, mockSiteForEnrollment2, mockSiteForEnrollment1],
+        unprocessed: [],
+      });
 
       const result = await tierClient.getAllEnrollment();
 
       expect(result.enrollments).to.have.lengthOf(2);
       expect(result.enrollments).to.deep.equal([mockSiteEnrollment, mockEnrollment3]);
     });
+
+    it('should filter out enrollments with mismatching orgId', async () => {
+      const tierClientWithoutSite = new TierClient(
+        mockContext,
+        organizationInstance,
+        null,
+        productCode,
+      );
+
+      const mismatchingOrgId = 'different-org-id';
+      const mockEnrollment2 = {
+        getId: () => 'enrollment-456',
+        getSiteId: () => 'site-with-wrong-org',
+        getEntitlementId: () => 'entitlement-123',
+      };
+
+      const mockSiteForEnrollment1 = {
+        getId: () => siteId,
+        getOrganizationId: () => orgId,
+      };
+
+      const mockSiteForEnrollment2 = {
+        getId: () => 'site-with-wrong-org',
+        getOrganizationId: () => mismatchingOrgId,
+      };
+
+      mockDataAccess.Entitlement.findByOrganizationIdAndProductCode.resolves(mockEntitlement);
+      mockDataAccess.SiteEnrollment.allByEntitlementId.resolves([
+        mockSiteEnrollment,
+        mockEnrollment2,
+      ]);
+      mockDataAccess.Site.batchGetByKeys.resolves({
+        data: [mockSiteForEnrollment1, mockSiteForEnrollment2],
+        unprocessed: [],
+      });
+
+      const result = await tierClientWithoutSite.getAllEnrollment();
+
+      expect(result.enrollments).to.have.lengthOf(1);
+      expect(result.enrollments[0].getId()).to.equal('enrollment-123');
+    });
+
+    it('should log warning when site not found for enrollment', async () => {
+      const tierClientWithoutSite = new TierClient(
+        mockContext,
+        organizationInstance,
+        null,
+        productCode,
+      );
+
+      mockDataAccess.Entitlement.findByOrganizationIdAndProductCode.resolves(mockEntitlement);
+      mockDataAccess.SiteEnrollment.allByEntitlementId.resolves([mockSiteEnrollment]);
+      mockDataAccess.Site.batchGetByKeys.resolves({
+        data: [],
+        unprocessed: [],
+      });
+
+      const result = await tierClientWithoutSite.getAllEnrollment();
+
+      expect(result.enrollments).to.have.lengthOf(0);
+      expect(mockContext.log.warn).to.have.been.calledWith(
+        `Site not found for enrollment ${mockSiteEnrollment.getId()} with siteId ${siteId}`,
+      );
+    });
   });
 
   describe('getFirstEnrollment', () => {
@@ -834,10 +945,15 @@ describe('TierClient', () => {
       const mockSiteObject = {
         getId: () => siteId,
         getName: () => 'Test Site',
+        getOrganizationId: () => orgId,
       };
 
       mockDataAccess.Entitlement.findByOrganizationIdAndProductCode.resolves(mockEntitlement);
       mockDataAccess.SiteEnrollment.allByEntitlementId.resolves([mockSiteEnrollment]);
+      mockDataAccess.Site.batchGetByKeys.resolves({
+        data: [mockSiteObject],
+        unprocessed: [],
+      });
       mockDataAccess.Site.findById.resolves(mockSiteObject);
 
       const tierClientWithoutSite = new TierClient(
@@ -861,6 +977,13 @@ describe('TierClient', () => {
       const mockSiteObject = {
         getId: () => siteId,
         getName: () => 'Test Site',
+        getOrganizationId: () => orgId,
+      };
+
+      const mockSiteObject2 = {
+        getId: () => 'other-site-id',
+        getName: () => 'Other Site',
+        getOrganizationId: () => orgId,
       };
 
       const mockEnrollment2 = {
@@ -874,6 +997,10 @@ describe('TierClient', () => {
         mockSiteEnrollment,
         mockEnrollment2,
       ]);
+      mockDataAccess.Site.batchGetByKeys.resolves({
+        data: [mockSiteObject, mockSiteObject2],
+        unprocessed: [],
+      });
       mockDataAccess.Site.findById.resolves(mockSiteObject);
 
       const tierClientWithoutSite = new TierClient(
@@ -917,8 +1044,18 @@ describe('TierClient', () => {
     });
 
     it('should return null site when site not found in database', async () => {
+      const mockSiteObject = {
+        getId: () => siteId,
+        getName: () => 'Test Site',
+        getOrganizationId: () => orgId,
+      };
+
       mockDataAccess.Entitlement.findByOrganizationIdAndProductCode.resolves(mockEntitlement);
       mockDataAccess.SiteEnrollment.allByEntitlementId.resolves([mockSiteEnrollment]);
+      mockDataAccess.Site.batchGetByKeys.resolves({
+        data: [mockSiteObject],
+        unprocessed: [],
+      });
       mockDataAccess.Site.findById.resolves(null);
 
       const tierClientWithoutSite = new TierClient(
@@ -951,10 +1088,15 @@ describe('TierClient', () => {
       const mockSiteObject = {
         getId: () => siteId,
         getName: () => 'Test Site',
+        getOrganizationId: () => orgId,
       };
 
       mockDataAccess.Entitlement.findByOrganizationIdAndProductCode.resolves(mockEntitlement);
       mockDataAccess.SiteEnrollment.allByEntitlementId.resolves([mockSiteEnrollment]);
+      mockDataAccess.Site.batchGetByKeys.resolves({
+        data: [mockSiteObject],
+        unprocessed: [],
+      });
       mockDataAccess.Site.findById.resolves(mockSiteObject);
 
       const result = await tierClient.getFirstEnrollment();
@@ -966,10 +1108,10 @@ describe('TierClient', () => {
       });
     });
 
-    it('should handle error when fetching site', async () => {
+    it('should handle error when fetching site via batchGetByKeys', async () => {
       mockDataAccess.Entitlement.findByOrganizationIdAndProductCode.resolves(mockEntitlement);
       mockDataAccess.SiteEnrollment.allByEntitlementId.resolves([mockSiteEnrollment]);
-      mockDataAccess.Site.findById.rejects(new Error('Site fetch error'));
+      mockDataAccess.Site.batchGetByKeys.rejects(new Error('Site fetch error'));
 
       await expect(tierClient.getFirstEnrollment()).to.be.rejectedWith('Site fetch error');
     });