feat(queues): add queue item operations [PLT-104203] - #643
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| /** Timestamp when processing started (set once a transaction begins) */ | ||
| startProcessing: string | null; | ||
| /** Timestamp when processing ended */ | ||
| endProcessing: string | null; |
There was a problem hiding this comment.
Convention violation: all timestamp fields in SDK response types must use the *Time suffix (rule: "NEVER introduce *At suffixes … *Time suffix").
startProcessing and endProcessing are timestamps but don't carry that suffix. They need to be renamed and a mapping entry added to QueueItemMap.
| /** Timestamp when processing started (set once a transaction begins) */ | |
| startProcessing: string | null; | |
| /** Timestamp when processing ended */ | |
| endProcessing: string | null; | |
| /** Timestamp when processing started (set once a transaction begins) */ | |
| startProcessingTime: string | null; | |
| /** Timestamp when processing ended */ | |
| endProcessingTime: string | null; |
In queues.constants.ts, add the renames so the pipeline produces the right names:
startProcessing: 'startProcessingTime',
endProcessing: 'endProcessingTime',The TransactionItemResponse JSDoc (line 210) references startProcessing by name — update that too.
There was a problem hiding this comment.
check what's better and consistent with the codebase. The above one or
startProcessing: 'processingStartTime',
endProcessing: 'processingEndTime'
| * (the item's data cannot be processed — not retried) from | ||
| * `ApplicationException` (a transient system error — eligible for retry). | ||
| */ | ||
| type?: string; |
There was a problem hiding this comment.
Convention violation: "Use enums for fixed value sets — NEVER leave raw strings/numbers."
The JSDoc documents exactly two values (BusinessException / ApplicationException). That's a fixed set — it needs an enum.
| type?: string; | |
| type?: QueueExceptionType; |
Add the enum near the other queue enums:
/**
* Exception category for a queue processing failure.
*/
export enum QueueExceptionType {
/** The item's data cannot be processed and will not be retried. */
BusinessException = 'BusinessException',
/** A transient system error; the item is eligible for retry. */
ApplicationException = 'ApplicationException'
}There was a problem hiding this comment.
@shivendra6720 check if this can be made an enum.
shivendra6720
changed the title
| * @returns Promise resolving to either an array of queues NonPaginatedResponse<QueueGetResponse> or a PaginatedResponse<QueueGetResponse> when pagination options are used. | ||
| * {@link QueueGetResponse} | ||
| * @returns Promise resolving to either an array of queues NonPaginatedResponse<QueueWithMethods> or a PaginatedResponse<QueueWithMethods> when pagination options are used. Each queue has methods attached for operating on its items. | ||
| * {@link QueueWithMethods} |
There was a problem hiding this comment.
Convention violation: "{@link} placed on a standalone line after @returns renders as stray text in TypeDoc, not a clickable link." The link must be embedded inline within the sentence.
Same pattern appears on lines 79, 101, 140, 190, 215, and 252 — all need fixing.
| * {@link QueueWithMethods} | |
| * @returns Promise resolving to either a {@link QueueWithMethods} array (`NonPaginatedResponse`) or a `PaginatedResponse<QueueWithMethods>` when pagination options are provided. Each queue has methods attached for operating on its items. |
There was a problem hiding this comment.
@shivendra6720 let's keep the link inline. Check this elsewhere in the file as well.
| export interface TransactionCompletionOptions { | ||
| /** | ||
| * True when the item was processed successfully; false records a failure | ||
| * (provide `processingException` with the failure details). | ||
| */ | ||
| isSuccessful: boolean; |
There was a problem hiding this comment.
Convention violation: "{Entity}{Operation}Options — bag of optional fields … Contains only optional fields." isSuccessful: boolean is required, which disqualifies the Options suffix.
Per the rules, 1–3 required params stay positional. Since only isSuccessful is required, extract it as a positional argument and rename the bag to hold only optional fields:
// queues.types.ts
export interface TransactionCompletionRequest {
processingException?: QueueProcessingException;
deferDate?: Date;
dueDate?: Date;
outputData?: Record<string, unknown>;
analytics?: Record<string, unknown>;
progress?: string;
operationId?: string;
}The service/model signatures then become:
completeTransaction(itemId: number, folderId: number, isSuccessful: boolean, options?: TransactionCompletionRequest): Promise<OperationResponse<TransactionCompletionRequest>>;
// bound variant
completeTransaction(itemId: number, isSuccessful: boolean, options?: TransactionCompletionRequest): Promise<OperationResponse<TransactionCompletionRequest>>;Update the service implementation, model factory delegate, JSDoc examples, and tests accordingly.
Review findingsFour CLAUDE.md convention violations across the new queue-item types: New inline comments this run
|
Adds getAllItems and insertItemByName, and attaches queue-bound equivalents to the objects returned by getAll/getById following the Data Fabric entities pattern. User-defined payload keys in specificData/outputData are preserved exactly; the JSON-string wire forms surface as specificDataJson / outputDataJson. Transaction operations (startTransaction / completeTransaction) follow in a separate PR stacked on this one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
158caa6 to
baa9c5b
Compare
shivendra6720
changed the title
|
| /** Current processing status */ | ||
| status: QueueItemStatus; | ||
| /** Review status for failed items */ | ||
| reviewStatus: string; |
There was a problem hiding this comment.
Convention violation: "Use enums for fixed value sets — NEVER leave raw strings/numbers."
reviewStatus has a fixed set of values in Orchestrator (None, Approved, Rejected, RetryPending). It must be an enum, the same way QueueItemStatus was correctly enumerated in this same PR.
| reviewStatus: string; | |
| reviewStatus: QueueItemReviewStatus; |
Add the enum alongside QueueItemStatus:
/**
* Review status of a failed queue item.
*/
export enum QueueItemReviewStatus {
/** No review has been assigned */
None = 'None',
/** Item approved after review */
Approved = 'Approved',
/** Item rejected after review */
Rejected = 'Rejected',
/** Item marked for retry after review */
RetryPending = 'RetryPending'
}Also update createBasicQueueItem() in the mock and ITEM_REVIEW_STATUS in test constants to use the enum value.
| // arbitrary queues. | ||
| describe('Queue items and transactions', () => { | ||
| /** Resolves the dedicated test queue, throwing when preconditions are unmet. */ | ||
| async function getTestQueue() { |
There was a problem hiding this comment.
Convention violation: "Extract shared data lookups to beforeAll in integration tests — when multiple tests in a describe block need the same setup data … fetch it once in beforeAll and store in a let variable!: Type variable. Repeating getAll or equivalent calls inside each it block wastes API quota and slows the suite."
getTestQueue() is called on lines 156, 194, 223, and 239 — four redundant API calls per run. The queue doesn't change between tests; fetch it once:
describe('Queue items and transactions', () => {
let testQueue!: QueueWithMethods;
beforeAll(async () => {
const { queues } = getServices();
const config = getTestConfig();
if (!config.folderId) {
throw new Error('INTEGRATION_TEST_FOLDER_ID must be configured for queue item tests');
}
if (!config.queuesTestQueueName) {
throw new Error('QUEUES_TEST_QUEUE_NAME must be configured for queue item tests');
}
const folderId = Number(config.folderId);
const result = await queues.getAll({
folderId,
filter: `name eq '${config.queuesTestQueueName}'`,
});
if (result.items.length === 0) {
throw new Error(
`Queue "${config.queuesTestQueueName}" was not found in folder ${folderId} — create it before running queue item tests`
);
}
testQueue = result.items[0];
});
it('should insert an item …', async () => {
const item = await testQueue.insertItem(payload, options);
// …
});
// … other tests use testQueue directlyRemove the async function getTestQueue() helper entirely.
Review findingsTwo new CLAUDE.md convention violations in addition to the four already flagged in open threads:
|
| * `Output`) are handled separately by the service — they are excluded from | ||
| * case conversion entirely (their keys are user-defined) and reattached as | ||
| * `specificData` / `outputData`. The JSON-string wire fields are renamed to | ||
| * explicit `*Json` names so both representations stay available. |
There was a problem hiding this comment.
Keep inline code comments concise.
| /** | ||
| * Queue metadata combined with queue-bound helper methods. | ||
| */ | ||
| export type QueueWithMethods = QueueGetResponse & QueueMethods; |
There was a problem hiding this comment.
The compose type should be called QueueGetResponse, not QueueWithMethods
We can rename the raw shape in queues.types.ts to RawQueueGetResponse and define QueueGetResponse = RawQueueGetResponse & QueueMethods here.
Examples -
| * @returns Promise resolving to either an array of queues NonPaginatedResponse<QueueGetResponse> or a PaginatedResponse<QueueGetResponse> when pagination options are used. | ||
| * {@link QueueGetResponse} | ||
| * @returns Promise resolving to either an array of queues NonPaginatedResponse<QueueWithMethods> or a PaginatedResponse<QueueWithMethods> when pagination options are used. Each queue has methods attached for operating on its items. | ||
| * {@link QueueWithMethods} |
There was a problem hiding this comment.
@shivendra6720 let's keep the link inline. Check this elsewhere in the file as well.
| * const queue = await queues.getById(<queueId>, <folderId>); | ||
| * | ||
| * // Operate on the queue directly via the attached methods | ||
| * const items = await queue.getAllItems(); | ||
| * ``` | ||
| */ |
There was a problem hiding this comment.
can we add a insertItem example as well?
| * // First, get queues with queues.getAll() | ||
| * const items = await queues.getAllItems(<queueId>, <folderId>); |
There was a problem hiding this comment.
Users won't use queues.getAllItems(<queueId>, <folderId>); on an array of queues returned. hence the comment above(first, get queues with queues.getAll()) is misleading.
Keep two examples, the one above after dropping the ask to fetch all queues and one which shows this method bounded to a queue returned by getById or getAll.
(keep the failed items example as is - thats fine)
| queueName: string, | ||
| folderId: number, | ||
| specificData: Record<string, unknown>, | ||
| options: QueueInsertItemOptions = {} |
There was a problem hiding this comment.
check if we put validation errors for required fields. if we do, add them for both getAllItems and insertItemByName
| const apiOptions = addPrefixToKeys(apiFieldOptions, ODATA_PREFIX, Object.keys(apiFieldOptions)); | ||
|
|
||
| const response = await this.get<QueueGetResponse>( |
There was a problem hiding this comment.
once we define RawQueueGetResponse, this would change to this.get<RawQueueGetResponse>
| private transformQueue(queue: object): QueueWithMethods { | ||
| const transformedQueue = transformData( | ||
| pascalToCamelCaseKeys(queue) as QueueGetResponse, | ||
| QueueMap | ||
| ) as QueueGetResponse; | ||
|
|
||
| return createQueueWithMethods(transformedQueue, this); | ||
| } |
There was a problem hiding this comment.
do we need a separate method for this. It's a 2 line pipeline that I think other services write inline.
| const itemData = camelToPascalCaseKeys({ | ||
| priority: options.priority ?? QueuePriority.Normal, | ||
| reference: options.reference, | ||
| progress: options.progress, | ||
| deferDate: toIsoString(options.deferDate), | ||
| dueDate: toIsoString(options.dueDate), | ||
| riskSlaDate: toIsoString(options.riskSlaDate), | ||
| name: queueName | ||
| }); |
There was a problem hiding this comment.
if its just specific fields we want to case, wouldn't it be better to
const response = await this.post<RawQueueItem>(
QUEUE_ENDPOINTS.ADD_ITEM,
{
itemData: {
Name: queueName,
Priority: options.priority ?? QueuePriority.Normal,
Reference: options.reference,
Progress: options.progress,
DeferDate: toIsoString(options.deferDate),
DueDate: toIsoString(options.dueDate),
RiskSlaDate: toIsoString(options.riskSlaDate),
// User-defined keys — sent exactly as provided, no case conversion
SpecificContent: specificData,
},
},
{ headers: createHeaders({ [FOLDER_ID]: folderId }) },
);
| name: queueName | ||
| }); | ||
|
|
||
| const response = await this.post<object>( |
There was a problem hiding this comment.
check if we can have something like RawQueueItem. If not then even Record<string, unknown> is better than passing object.
Reworks the queue-item surface per review:
- rename QueueWithMethods -> QueueGetResponse (= RawQueueGetResponse &
QueueMethods, the jobs/entities compose pattern) and
QueueItemResponse -> QueueItem; the released QueueGetResponse name and
its fields are preserved for existing consumers
- enums for fixed value sets: QueueItemReviewStatus (None/InReview/
Verified/Retried) and QueueExceptionType (ApplicationException/
BusinessException), both verified against the live swagger
- processingException -> processingError (QueueProcessingError), with
the nested creationTime renamed to createdTime via
QueueProcessingErrorMap; startProcessing/endProcessing ->
processingStartTime/processingEndTime; folderId/folderName required
- drop the specificDataJson/outputDataJson duplicates; the payload
objects (SpecificContent/Output) remain the single representation
- enforce flat insert payloads in the type system via QueueItemValue
(string | number | boolean | Date | null | undefined)
- ValidationError guards on getAllItems/insertItemByName required params
- inline the queue transform (jobs pattern); module-level toQueueItem
shared by list + insert; explicit PascalCase itemData body
- JSDoc: inline {@link} in @returns, bound-usage examples, insertItem
example on getById, backend wording removed; factory delegates are
async with explicit return types
- integration tests resolve the test queue once in beforeAll; unit
tests cover the new enums, dropped fields, nested error rename, and
validation guards
Verified: typecheck, lint, 2171 unit tests, 20/20 live integration
against alpha.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
||
| await expect(queue.getAllItems()).rejects.toThrow('Queue ID is undefined'); | ||
| expect(mockService.getAllItems).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
Missing test for the symmetric folderId === undefined guard. createQueueMethods.getAllItems has two pre-condition checks (id === undefined and folderId === undefined), but only the first is exercised here — a regression removing the second check would go undetected.
Convention: "Test all meaningful branches for boolean-like conditions — when a method branches on param === false vs other values, add explicit tests for true, false, and undefined."
| }); | |
| await expect(queue.getAllItems()).rejects.toThrow('Queue ID is undefined'); | |
| expect(mockService.getAllItems).not.toHaveBeenCalled(); | |
| }); | |
| it('should reject when the folder ID is undefined', async () => { | |
| const queueData = createBasicQueue({ folderId: undefined as unknown as number }); | |
| const queue = createQueueWithMethods(queueData, mockService); | |
| await expect(queue.getAllItems()).rejects.toThrow('Folder ID is undefined'); | |
| expect(mockService.getAllItems).not.toHaveBeenCalled(); | |
| }); |
Review findingsNew inline comment this runMissing error test for
|
Live browser E2E against alpha showed AddQueueItem returns OrganizationUnitFullyQualifiedName as null — only the listing endpoint populates it. QueueItem.folderName is therefore string | null, and the insert integration test pins the null so a future API change surfaces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
2 participants
Method Added
queues.getAllItems()getAllItems<T extends QueueGetAllItemsOptions>(queueId: number, folderId: number, options?: T): Promise<T extends HasPaginationOptions<T> ? PaginatedResponse<QueueItem> : NonPaginatedResponse<QueueItem>>queues.insertItemByName()insertItemByName(queueName: string, folderId: number, specificData: Record<string, QueueItemValue>, options?: QueueInsertItemOptions): Promise<QueueItem>queue.getAllItems()getAllItems<T extends QueueGetAllItemsOptions>(options?: T): Promise<T extends HasPaginationOptions<T> ? PaginatedResponse<QueueItem> : NonPaginatedResponse<QueueItem>>queue.insertItem()insertItem(specificData: Record<string, QueueItemValue>, options?: QueueInsertItemOptions): Promise<QueueItem>Queues now attach bound methods (
QueueGetResponse = RawQueueGetResponse & QueueMethods, the Jobs/Entities compose pattern). The releasedQueueGetResponsename and all of its fields are preserved — existing consumers keep compiling; queue objects additionally carry the two bound methods.Endpoint Called
getAllItems()/orchestrator_/odata/QueueItemsOR.QueuesorOR.Queues.ReadinsertItemByName()/orchestrator_/odata/Queues/UiPathODataSvc.AddQueueItemOR.QueuesorOR.Queues.WriteFolderScopedService— sets theX-UIPATH-OrganizationUnitIdheader fromfolderIdgetAllItemssupports OData pagination ($top,$skip,$count), filtering, and ordering; the queue scoping filter (queueId eq N) is merged with any caller filter, and SDK field names infilter/orderby/selectare rewritten to API names before the requestRecord<string, QueueItemValue>whereQueueItemValue = string | number | boolean | Date | null | undefined— nested objects/arrays are rejected at compile time;Datevalues are serialized to ISO-8601ValidationErrorbefore any HTTP callExample Usage
API Response vs SDK Response
Transform pipeline
extract user-payload objects (
SpecificContent/Output) → drop their JSON-string duplicates (SpecificData/OutputData) →pascalToCamelCaseKeys→transformData(QueueItemMap)→transformData(QueueProcessingErrorMap)on the nested failure object → reattach payloads verbatim asspecificData/outputDataField mapping
QueueDefinitionIdqueueIdCreationTimecreatedTimeOrganizationUnitIdfolderIdOrganizationUnitFullyQualifiedNamefolderNameStartProcessingprocessingStartTime*Timesuffix convention for timestampsEndProcessingprocessingEndTime*Timesuffix convention for timestampsProcessingExceptionprocessingErrorCreationTime→createdTimeSpecificContentspecificDataOutputoutputDataSpecificData/OutputDataStatus,ReviewStatus,Prioritystatus,reviewStatus,priorityQueueItemStatus/QueueItemReviewStatus/QueuePriorityenums (values verified against live swagger)Sample SDK Response
insertItemByName()/queue.insertItem(){ "id": 22905684, "key": "62625516-227e-41cd-8e9b-43e6be405d5d", "status": "New", "reviewStatus": "None", "priority": "High", "queueId": 346714, "specificData": { "InvoiceId": "INV-1001", "amountDue": 1520, "Vendor_Name": "Acme" }, "outputData": null, "processingError": null, "progress": null, "reference": "SDK-REF-001", "createdTime": "2026-08-03T16:20:00.603Z", "deferDate": "2026-08-03T00:00:00Z", "dueDate": "2026-08-15T00:00:00Z", "riskSlaDate": null, "processingStartTime": null, "processingEndTime": null, "retryNumber": 0, "folderId": 756377, "folderName": null }From live E2E on alpha — note the payload keys (
InvoiceId,amountDue,Vendor_Name) returned exactly as provided, with no case conversion, andfolderName: null— the insert response does not populate it (listing does), hencestring | null.getAllItems()/queue.getAllItems(){ "items": [ { "id": 22905684, "key": "62625516-227e-41cd-8e9b-43e6be405d5d", "status": "New", "reviewStatus": "None", "priority": "High", "queueId": 346714, "specificData": { "InvoiceId": "INV-1001", "amountDue": 1520, "Vendor_Name": "Acme" }, "outputData": null, "processingError": null, "reference": "SDK-REF-001", "createdTime": "2026-08-03T16:20:00.603Z", "processingStartTime": null, "processingEndTime": null, "retryNumber": 0, "folderId": 756377, "folderName": "APPS_TestPass_Folder" } ], "totalCount": 1 }Truncated to 1 item — full response pages through the queue's items.
Files
src/utils/constants/endpoints/orchestrator.ts(GET_ITEMS,ADD_ITEM)src/models/orchestrator/queues.types.ts(RawQueueGetResponse,QueueItem,QueueItemValue,QueueInsertItemOptions,QueueProcessingError,QueuePriority/QueueItemStatus/QueueItemReviewStatus/QueueExceptionTypeenums)src/models/orchestrator/queues.constants.ts(QueueItemMap,QueueProcessingErrorMap)src/models/orchestrator/queues.models.ts(QueueGetResponsecompose type,QueueServiceModel,QueueMethods, factories)src/services/orchestrator/queues/queues.ts(getAllItems,insertItemByName, module-leveltoQueueItem)tests/unit/services/orchestrator/queues.test.ts(24),tests/unit/models/orchestrator/queues.test.ts(6)tests/integration/shared/orchestrator/queues.integration.test.ts(10 tests × v0/v1 modes; 20/20 passing live against alpha)tests/utils/mocks/queues.ts,tests/utils/constants/queues.ts.github/workflows/coverage.yml(QUEUES_TEST_QUEUE_NAMEwiring),tests/integration/config/test-config.ts,tests/.env.integration.exampledocs/oauth-scopes.md,docs/pagination.md,agent_docs/conventions.mdRefs PLT-104203
🤖 Auto-generated using onboarding skills