fix: address data fabric access review comments

Author: dsuresh-ap

README.md

@@ -282,7 +282,7 @@ import { Cases, CaseInstances } from '@uipath/uipath-typescript/cases';
 import { Tasks, TaskType } from '@uipath/uipath-typescript/tasks';
 import { Processes } from '@uipath/uipath-typescript/processes';
 import { Buckets } from '@uipath/uipath-typescript/buckets';
-import { ChoiceSets, DataFabricDirectory, DataFabricRoles, Entities } from '@uipath/uipath-typescript/entities';
+import { ChoiceSets, Entities } from '@uipath/uipath-typescript/entities';
 
 // Initialize SDK
 const sdk = new UiPath({ /* config */ });
@@ -297,8 +297,6 @@ const processes = new Processes(sdk);
 const buckets = new Buckets(sdk);
 const entities = new Entities(sdk);
 const choiceSets = new ChoiceSets(sdk);
-const dataFabricRoles = new DataFabricRoles(sdk);
-const dataFabricDirectory = new DataFabricDirectory(sdk);
 
 // Maestro - Get processes and their instances
 const allProcesses = await maestroProcesses.getAll();

src/services/data-fabric/directory.ts

@@ -14,7 +14,6 @@ import {
 } from '../../models/data-fabric/directory.types';
 import {
   DataFabricDirectoryAssignPayload,
-  RawDataFabricDirectoryEntry,
   DataFabricDirectoryRevokePayload,
   RawDataFabricDirectoryListResponse,
 } from '../../models/data-fabric/directory.internal-types';
@@ -186,7 +185,7 @@ export class DataFabricDirectoryService extends BaseService implements DataFabri
   async list(options: DataFabricDirectoryListOptions = {}): Promise<DataFabricDirectoryListResponse> {
     const params = createParams({
       skip: options.skip,
-      top: options.top ?? DEFAULT_DIRECTORY_PAGE_SIZE,
+      top: clampDirectoryPageSize(options.top),
     });
     const response = await this.get<RawDataFabricDirectoryListResponse>(
       DATA_FABRIC_ENDPOINTS.DIRECTORY.GET_ALL,

src/utils/constants/common.ts

@@ -56,6 +56,7 @@ export const CHOICESET_VALUES_PAGINATION = {
   TOTAL_COUNT_FIELD: 'totalRecordCount'
 };
 
+/**
  * Bucket pagination constants for token-based pagination
  */
 export const BUCKET_PAGINATION = {

src/utils/pagination/helpers.ts

@@ -11,7 +11,7 @@ import { createHeaders } from '../http/headers';
 import { FOLDER_ID } from '../constants/headers';
 import { ODATA_PREFIX, HTTP_METHODS } from '../constants/common';
 import { addPrefixToKeys } from '../transform';
-import { DEFAULT_ITEMS_FIELD, DEFAULT_PAGE_SIZE, DEFAULT_TOTAL_COUNT_FIELD } from './constants';
+import { DEFAULT_ITEMS_FIELD, DEFAULT_TOTAL_COUNT_FIELD } from './constants';
 import { filterUndefined, resolveNestedField } from '../object';
 import { decodeBase64 } from '../encoding/base64';
 
@@ -364,41 +364,4 @@ export class PaginationHelpers {
       }
     }) as any;
   }
-
-  /**
-   * Fetches every page for offset/token paginated APIs and returns a flat item array.
-   *
-   * Use this when a service must preserve an array-returning `getAll()` surface
-   * but the backend endpoint only exposes paginated responses.
-   *
-   * @param config - Configuration for the paginated operation
-   * @param options - Request options including pageSize and non-pagination filters
-   * @returns Promise resolving to all transformed items
-   */
-  static async getAllPages<TOptions extends Record<string, unknown>, TRaw, TTransformed = TRaw>(
-    config: GetAllConfig<TRaw, TTransformed>,
-    options: TOptions & { pageSize?: number } = {} as TOptions & { pageSize?: number }
-  ): Promise<TTransformed[]> {
-    const pageSize = options.pageSize ?? DEFAULT_PAGE_SIZE;
-    const items: TTransformed[] = [];
-    let cursor: PaginationCursor | undefined;
-    let hasNextPage = true;
-
-    while (hasNextPage) {
-      const pageOptions = cursor
-        ? { ...options, pageSize, cursor }
-        : { ...options, pageSize };
-      const page = await PaginationHelpers.getAll<
-        TOptions & { pageSize: number; cursor?: PaginationCursor },
-        TRaw,
-        TTransformed
-      >(config, pageOptions) as PaginatedResponse<TTransformed>;
-
-      items.push(...page.items);
-      hasNextPage = page.hasNextPage;
-      cursor = page.nextCursor;
-    }
-
-    return items;
-  }
 }

tests/.env.integration.example

@@ -37,15 +37,6 @@ INTEGRATION_TEST_FOLDER_ID=
 # Requires DataFabric.Data.Read and DataFabric.Data.Write on the configured token/app.
 DATA_FABRIC_ACCESS_INTEGRATION=false
 
-# Enable Agent Feedback integration tests.
-# Requires feedback API access on the configured token/app.
-AGENT_FEEDBACK_INTEGRATION=false
-
-# Enable Observability Traces integration tests.
-# Requires traces API access and a pre-existing trace ID.
-TRACES_INTEGRATION=false
-TRACES_TEST_TRACE_ID=
-
 # Folder key (GUID) matching INTEGRATION_TEST_FOLDER_ID
 # Used by getByName integration tests for assets/processes
 INTEGRATION_TEST_FOLDER_KEY=

tests/integration/README.md

@@ -98,14 +98,11 @@ tests/integration/
    INTEGRATION_TEST_SKIP_CLEANUP=false
    INTEGRATION_TEST_FOLDER_ID=
    DATA_FABRIC_ACCESS_INTEGRATION=false
-   AGENT_FEEDBACK_INTEGRATION=false
-   TRACES_INTEGRATION=false
 
    # Optional: Pre-existing Test Data
    MAESTRO_TEST_PROCESS_KEY=
    ORCHESTRATOR_TEST_PROCESS_KEY=
    DATA_FABRIC_TEST_ENTITY_ID=
-   TRACES_TEST_TRACE_ID=
    ```
 
 ### 4. Install Dependencies
@@ -166,24 +163,6 @@ prepared for access-management validation:
 DATA_FABRIC_ACCESS_INTEGRATION=true npm run test:integration -- tests/integration/shared/data-fabric/access.integration.test.ts
 ```
 
-### Run Optional Agent Feedback Tests
-
-Agent Feedback integration tests are opt-in because they require feedback API
-authorization on the configured external app/PAT:
-
-```bash
-AGENT_FEEDBACK_INTEGRATION=true npm run test:integration -- tests/integration/shared/agents/feedback.integration.test.ts
-```
-
-### Run Optional Observability Traces Tests
-
-Observability Traces integration tests are opt-in because they require traces API
-authorization and a pre-existing trace ID:
-
-```bash
-TRACES_INTEGRATION=true TRACES_TEST_TRACE_ID=<trace-id> npm run test:integration -- tests/integration/shared/observability/traces.integration.test.ts
-```
-
 ### Watch Mode
 
 ```bash
@@ -267,9 +246,6 @@ Tests skip gracefully when:
 | `INTEGRATION_TEST_SKIP_CLEANUP` | Skip cleanup after tests (useful for debugging) | `false` |
 | `INTEGRATION_TEST_FOLDER_ID` | Default folder ID for tests | (uses default folder) |
 | `DATA_FABRIC_ACCESS_INTEGRATION` | Enable opt-in Data Fabric role and directory access tests | `false` |
-| `AGENT_FEEDBACK_INTEGRATION` | Enable opt-in Agent Feedback tests requiring feedback API access | `false` |
-| `TRACES_INTEGRATION` | Enable opt-in Observability Traces tests requiring traces API access | `false` |
-| `TRACES_TEST_TRACE_ID` | Pre-existing trace ID for Observability Traces tests | (required when `TRACES_INTEGRATION=true`) |
 | `MAESTRO_TEST_PROCESS_KEY` | Pre-existing Maestro process for read-only tests | (optional) |
 | `ORCHESTRATOR_TEST_PROCESS_KEY` | Pre-existing Orchestrator process for start tests | (optional) |
 | `DATA_FABRIC_TEST_ENTITY_ID` | Pre-existing Data Fabric entity for record tests | (optional) |

tests/integration/shared/agents/feedback.integration.test.ts

@@ -6,22 +6,12 @@ import { registerResource } from '../../utils/cleanup';
 import { generateRandomString } from '../../utils/helpers';
 
 const modes: InitMode[] = ['v1'];
-const hasAgentFeedbackIntegration = process.env.AGENT_FEEDBACK_INTEGRATION === 'true';
 
 describe.each(modes)('Agent Feedback - Integration Tests [%s]', (mode) => {
   setupUnifiedTests(mode);
 
   let feedback: Feedback;
 
-  beforeAll(() => {
-    if (!hasAgentFeedbackIntegration) {
-      throw new Error(
-        'AGENT_FEEDBACK_INTEGRATION must be set to run Agent Feedback integration tests ' +
-        '(requires feedback API access on the configured external app/PAT)'
-      );
-    }
-  });
-
   beforeEach(() => {
     feedback = getServices().feedback!;
   });

tests/integration/shared/observability/traces.integration.test.ts

@@ -7,7 +7,6 @@ import {
 } from '../../../../src/models/observability/traces/traces.types';
 
 const modes: InitMode[] = ['v1'];
-const hasTracesIntegration = process.env.TRACES_INTEGRATION === 'true';
 
 describe.each(modes)('Traces - Integration Tests [%s]', (mode) => {
   setupUnifiedTests(mode);
@@ -17,13 +16,6 @@ describe.each(modes)('Traces - Integration Tests [%s]', (mode) => {
   let existingSpanId!: string;
 
   beforeAll(async () => {
-    if (!hasTracesIntegration) {
-      throw new Error(
-        'TRACES_INTEGRATION must be set to run Traces integration tests ' +
-        '(requires traces API access and a pre-existing trace ID)'
-      );
-    }
-
     if (!process.env.TRACES_TEST_TRACE_ID) {
       throw new Error('TRACES_TEST_TRACE_ID env var required for Traces integration tests');
     }

tests/unit/services/data-fabric/directory.test.ts

@@ -93,6 +93,17 @@ describe('DataFabricDirectoryService Unit Tests', () => {
       });
     });
 
+    it('should clamp top to the maximum directory page size', async () => {
+      mockApiClient.get.mockResolvedValue({ totalCount: 0, results: [] });
+
+      await directoryService.list({ top: 500 });
+
+      expect(mockApiClient.get).toHaveBeenCalledWith(
+        DATA_FABRIC_ENDPOINTS.DIRECTORY.GET_ALL,
+        { params: { top: 100 } }
+      );
+    });
+
     it('should reject invalid directory response formats', async () => {
       mockApiClient.get.mockResolvedValue({ value: [] });
 

tests/unit/utils/pagination/helpers.test.ts

@@ -1,8 +1,7 @@
 // ===== IMPORTS =====
 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
 import { PaginationHelpers } from '../../../../src/utils/pagination/helpers';
-import { PaginationType } from '../../../../src/utils/pagination/internal-types';
-import type { GetAllConfig, PaginationServiceAccess } from '../../../../src/utils/pagination/internal-types';
+import { PaginationServiceAccess, PaginationType } from '../../../../src/utils/pagination/internal-types';
 import { PaginationCursor, PaginationOptions } from '../../../../src/utils/pagination/types';
 import { encodeBase64 } from '../../../../src/utils/encoding/base64';
 import { TEST_CONSTANTS, PAGINATION_TEST_CONSTANTS } from '../../../utils/constants';
@@ -23,9 +22,6 @@ import {
 import { DEFAULT_TOTAL_COUNT_FIELD, DEFAULT_ITEMS_FIELD } from '../../../../src/utils/pagination/constants';
 import { FOLDER_ID } from '../../../../src/utils/constants/headers';
 
-type MockRawItem = ReturnType<typeof createMockRawItem>;
-type MockTransformedItem = ReturnType<typeof createMockTransformedItem>;
-
 // ===== TEST SUITE =====
 describe('PaginationHelpers Unit Tests', () => {
   afterEach(() => {
@@ -936,73 +932,4 @@ describe('PaginationHelpers Unit Tests', () => {
       });
     });
   });
-
-  describe('getAllPages', () => {
-    let mockServiceAccess: PaginationServiceAccess;
-    let mockConfig: GetAllConfig<MockRawItem, MockTransformedItem>;
-
-    beforeEach(() => {
-      mockServiceAccess = createMockPaginationServiceAccess();
-      mockConfig = {
-        serviceAccess: mockServiceAccess,
-        getEndpoint: () => PAGINATION_TEST_CONSTANTS.ENDPOINT_API_ITEMS,
-        transformFn: mockTransformFunction,
-        pagination: {
-          paginationType: PaginationType.OFFSET,
-          itemsField: DEFAULT_ITEMS_FIELD,
-          totalCountField: DEFAULT_TOTAL_COUNT_FIELD
-        }
-      };
-    });
-
-    it('should follow cursors and return a flattened item array', async () => {
-      const nextCursor = createMockPaginationCursor();
-      const firstRawItem = createMockRawItem(PAGINATION_TEST_CONSTANTS.ITEM_ID_1, PAGINATION_TEST_CONSTANTS.ITEM_NAME_1);
-      const secondRawItem = createMockRawItem(PAGINATION_TEST_CONSTANTS.ITEM_ID_2, PAGINATION_TEST_CONSTANTS.ITEM_NAME_2);
-      const firstTransformedItem = createMockTransformedItem(PAGINATION_TEST_CONSTANTS.ITEM_ID_1, PAGINATION_TEST_CONSTANTS.ITEM_NAME_1);
-      const secondTransformedItem = createMockTransformedItem(PAGINATION_TEST_CONSTANTS.ITEM_ID_2, PAGINATION_TEST_CONSTANTS.ITEM_NAME_2);
-
-      vi.mocked(mockServiceAccess.requestWithPagination)
-        .mockResolvedValueOnce(createMockPaginatedResponse([firstRawItem], {
-          hasNextPage: true,
-          nextCursor
-        }))
-        .mockResolvedValueOnce(createMockPaginatedResponse([secondRawItem], {
-          hasNextPage: false
-        }));
-
-      const result = await PaginationHelpers.getAllPages<
-        { pageSize?: number },
-        MockRawItem,
-        MockTransformedItem
-      >(mockConfig, {
-        pageSize: PAGINATION_TEST_CONSTANTS.PAGE_SIZE
-      });
-
-      expect(result).toEqual([firstTransformedItem, secondTransformedItem]);
-      expect(mockServiceAccess.requestWithPagination).toHaveBeenNthCalledWith(
-        1,
-        'GET',
-        PAGINATION_TEST_CONSTANTS.ENDPOINT_API_ITEMS,
-        { pageSize: PAGINATION_TEST_CONSTANTS.PAGE_SIZE },
-        expect.any(Object)
-      );
-      expect(mockServiceAccess.requestWithPagination).toHaveBeenNthCalledWith(
-        2,
-        'GET',
-        PAGINATION_TEST_CONSTANTS.ENDPOINT_API_ITEMS,
-        { pageSize: PAGINATION_TEST_CONSTANTS.PAGE_SIZE, cursor: nextCursor },
-        expect.any(Object)
-      );
-    });
-
-    it('should propagate errors from the underlying paginated call', async () => {
-      vi.mocked(mockServiceAccess.requestWithPagination)
-        .mockRejectedValue(new Error(TEST_CONSTANTS.ERROR_MESSAGE));
-
-      await expect(PaginationHelpers.getAllPages(mockConfig, {
-        pageSize: PAGINATION_TEST_CONSTANTS.PAGE_SIZE
-      })).rejects.toThrow(TEST_CONSTANTS.ERROR_MESSAGE);
-    });
-  });
 });