Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [X.X.X] - Unreleased

### Fixed

- Fixed all `sharing` methods (`listAssetShares`, `getAssetShare`, `shareAsset`, `updateAssetShare`, `deleteAssetShare`) failing with `404 Not Found` because query parameters (`assetType`/`assetId`) were baked into the request URL and then re-applied by the HTTP layer, producing a duplicated, malformed query string. Query parameters are now applied once. ([#195](https://github.com/smartsheet/smartsheet-javascript-sdk/issues/195))
- Fixed `sharing.updateAssetShare` throwing at runtime because the HTTP requestor did not expose a `patch` method.

## [5.2.0] - 2026-07-09

### Added
Expand Down
53 changes: 12 additions & 41 deletions lib/sharing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ export const createSharing = (options: CreateOptions): SharingApi => {
};

// Base URL for shares endpoints
const baseUrl = '/shares';
const baseUrl = 'shares';

/**
* List all shares for a specified asset
Expand All @@ -214,27 +214,9 @@ export const createSharing = (options: CreateOptions): SharingApi => {
options: ListSharesOptions,
callback?: RequestCallback<ListSharesResponse>
): Promise<ListSharesResponse> => {
const { queryParameters } = options;

// Build the base URL with required parameters
const urlParams = new URLSearchParams();
urlParams.append('assetType', queryParameters.assetType.toString());
urlParams.append('assetId', queryParameters.assetId.toString());

// Add optional query parameters
if (queryParameters.maxItems) {
urlParams.append('maxItems', queryParameters.maxItems.toString());
}
if (queryParameters.lastKey) {
urlParams.append('lastKey', queryParameters.lastKey);
}
if (queryParameters.sharingInclude) {
urlParams.append('sharingInclude', queryParameters.sharingInclude);
}

const url = `${baseUrl}?${urlParams.toString()}`;

return requestor.get({ ...optionsToSend, url, ...options }, callback);
const urlOptions = { url: baseUrl };

return requestor.get({ ...optionsToSend, ...urlOptions, ...options }, callback);
};

/**
Expand All @@ -247,11 +229,9 @@ export const createSharing = (options: CreateOptions): SharingApi => {
options: GetShareOptions,
callback?: RequestCallback<ShareResponse>
): Promise<ShareResponse> => {
const { shareId, queryParameters } = options;

const url = `${baseUrl}/${shareId}?assetType=${queryParameters.assetType}&assetId=${queryParameters.assetId}`;
const urlOptions = { url: `${baseUrl}/${options.shareId}` };

return requestor.get({ ...optionsToSend, url, ...options }, callback);
return requestor.get({ ...optionsToSend, ...urlOptions, ...options }, callback);
};

/**
Expand All @@ -261,14 +241,9 @@ export const createSharing = (options: CreateOptions): SharingApi => {
* @returns Promise with shares success result
*/
const shareAsset = (options: ShareAssetOptions, callback?: RequestCallback<SharesResult>): Promise<SharesResult> => {
const { body, queryParameters } = options;

let url = `${baseUrl}?assetType=${queryParameters.assetType}&assetId=${queryParameters.assetId}`;
if (queryParameters.sendEmail) {
url += '&sendEmail=true';
}
const urlOptions = { url: baseUrl };

return requestor.post({ ...optionsToSend, url, body, ...options }, callback);
return requestor.post({ ...optionsToSend, ...urlOptions, ...options }, callback);
};

/**
Expand All @@ -281,11 +256,9 @@ export const createSharing = (options: CreateOptions): SharingApi => {
options: UpdateShareOptions,
callback?: RequestCallback<ShareResponse>
): Promise<ShareResponse> => {
const { shareId, queryParameters, body } = options;
const urlOptions = { url: `${baseUrl}/${options.shareId}` };

const url = `${baseUrl}/${shareId}?assetType=${queryParameters.assetType}&assetId=${queryParameters.assetId}`;

return requestor.patch({ ...optionsToSend, url, body, ...options }, callback);
return requestor.patch({ ...optionsToSend, ...urlOptions, ...options }, callback);
};

/**
Expand All @@ -295,11 +268,9 @@ export const createSharing = (options: CreateOptions): SharingApi => {
* @returns Promise with success result
*/
const deleteAssetShare = (options: DeleteShareOptions, callback?: RequestCallback<any>): Promise<any> => {
const { shareId, queryParameters } = options;

const url = `${baseUrl}/${shareId}?assetType=${queryParameters.assetType}&assetId=${queryParameters.assetId}`;
const urlOptions = { url: `${baseUrl}/${options.shareId}` };

return requestor.delete({ ...optionsToSend, url, ...options }, callback);
return requestor.delete({ ...optionsToSend, ...urlOptions, ...options }, callback);
};

return {
Expand Down
3 changes: 3 additions & 0 deletions lib/utils/httpRequestor.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ export function create(requestorConfig) {

const put = (options, callback) => methodRequest(options, request.put, 'PUT', callback, options.body);

const patch = (options, callback) => methodRequest(options, request.patch, 'PATCH', callback, options.body);

const methodRequest = (options, method, methodName, callback, body) => {
const baseRequestOptions = {
url: buildUrl(options),
Expand Down Expand Up @@ -191,6 +193,7 @@ export function create(requestorConfig) {
return {
get: get,
put: put,
patch: patch,
post: post,
postFile: postFile,
delete: deleteFunc,
Expand Down
63 changes: 63 additions & 0 deletions test/mock-api/sharing/common_test_constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { AccessLevel, AssetType, ShareScope, ShareType } from '@smartsheet/sharing';

// Common share identifiers
export const TEST_SHARE_ID = 'AAAMCmYGFOeE';
export const TEST_SHARE_EMAIL = 'test.email@smartsheet.com';
// userId and groupId are returned as strings in the API JSON payload
export const TEST_SHARE_USER_ID = '9876543210';
export const TEST_SHARE_GROUP_ID = '1234567890';
export const TEST_SHARE_NAME = 'Example Name';
export const TEST_LAST_KEY = 'abcDefGhIjKlMnOpQrStUvWxYz';

// Asset targeted by share operations
export const TEST_ASSET_ID = 9876543210;

// Pagination / include query parameters
export const TEST_MAX_ITEMS = 25;
export const TEST_SHARING_INCLUDE = 'email';

// Common success response values
export const TEST_SUCCESS_MESSAGE = 'SUCCESS';
export const TEST_SUCCESS_RESULT_CODE = 0;

// Share object with all properties present (matches all-response-body-properties mappings)
export const EXPECTED_SHARE_ALL_PROPERTIES = {
id: TEST_SHARE_ID,
email: TEST_SHARE_EMAIL,
userId: TEST_SHARE_USER_ID,
groupId: TEST_SHARE_GROUP_ID,
name: TEST_SHARE_NAME,
type: ShareType.USER,
accessLevel: AccessLevel.ADMIN,
scope: ShareScope.ITEM,
};

// Share object with only required properties (matches required-response-body-properties mappings)
export const EXPECTED_SHARE_REQUIRED_PROPERTIES = {
id: TEST_SHARE_ID,
email: TEST_SHARE_EMAIL,
userId: TEST_SHARE_USER_ID,
groupId: TEST_SHARE_GROUP_ID,
type: ShareType.USER,
accessLevel: AccessLevel.ADMIN,
scope: ShareScope.ITEM,
};

// Reusable query parameters targeting a sheet asset
export const TEST_SHEET_ASSET_QUERY = {
assetType: AssetType.SHEET,
assetId: TEST_ASSET_ID,
};

// Expected query string for a sheet asset, built as URLSearchParams so assertions
// verify key cardinality (a duplicated assetType/assetId would fail the comparison).
export const EXPECTED_SHEET_ASSET_QUERY = new URLSearchParams({
assetType: AssetType.SHEET,
assetId: TEST_ASSET_ID.toString(),
});

// Common error status codes and messages
export const ERROR_500_STATUS_CODE = 500;
export const ERROR_500_MESSAGE = 'Internal Server Error';
export const ERROR_400_STATUS_CODE = 400;
export const ERROR_400_MESSAGE = 'Malformed Request';
111 changes: 111 additions & 0 deletions test/mock-api/sharing/delete_asset_share.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import crypto from 'crypto';
import { createClient, findWireMockRequest } from '../utils/utils';
import { expect } from '@jest/globals';
import { AssetType } from '@smartsheet/sharing';
import {
TEST_SHARE_ID,
TEST_ASSET_ID,
EXPECTED_SHEET_ASSET_QUERY,
TEST_SUCCESS_MESSAGE,
TEST_SUCCESS_RESULT_CODE,
ERROR_500_STATUS_CODE,
ERROR_500_MESSAGE,
ERROR_400_STATUS_CODE,
ERROR_400_MESSAGE,
} from './common_test_constants';

describe('Sharing - deleteAssetShare endpoint tests', () => {
const client = createClient();

const expectedAllResponseProperties = {
message: TEST_SUCCESS_MESSAGE,
resultCode: TEST_SUCCESS_RESULT_CODE,
};

it('deleteAssetShare generated url is correct', async () => {
const requestId = crypto.randomUUID();
const options = {
shareId: TEST_SHARE_ID,
queryParameters: {
assetType: AssetType.SHEET,
assetId: TEST_ASSET_ID,
},
customProperties: {
'x-request-id': requestId,
'x-test-name': '/sharing/delete-asset-share/all-response-body-properties',
},
};
await client.sharing.deleteAssetShare(options);
const matchedRequest = await findWireMockRequest(requestId);

const parsedUrl = new URL(matchedRequest.absoluteUrl);
expect(parsedUrl.pathname).toEqual(`/2.0/shares/${TEST_SHARE_ID}`);
expect(matchedRequest.method).toEqual('DELETE');

expect(parsedUrl.searchParams).toEqual(EXPECTED_SHEET_ASSET_QUERY);
});

it('deleteAssetShare all response body properties', async () => {
const requestId = crypto.randomUUID();
const options = {
shareId: TEST_SHARE_ID,
queryParameters: {
assetType: AssetType.SHEET,
assetId: TEST_ASSET_ID,
},
customProperties: {
'x-request-id': requestId,
'x-test-name': '/sharing/delete-asset-share/all-response-body-properties',
},
};
const response = await client.sharing.deleteAssetShare(options);
const matchedRequest = await findWireMockRequest(requestId);

expect(matchedRequest.body).toEqual('');
expect(response).toEqual(expectedAllResponseProperties);
});

it('deleteAssetShare error 400 response', async () => {
const requestId = crypto.randomUUID();
const options = {
shareId: TEST_SHARE_ID,
queryParameters: {
assetType: AssetType.SHEET,
assetId: TEST_ASSET_ID,
},
customProperties: {
'x-request-id': requestId,
'x-test-name': '/errors/400-response',
},
};
try {
await client.sharing.deleteAssetShare(options);
expect(true).toBe(false); // Expected an error to be thrown
} catch (error: any) {
expect(error.statusCode).toBe(ERROR_400_STATUS_CODE);
expect(error.message).toBe(ERROR_400_MESSAGE);
}
});

it('deleteAssetShare error 500 response', async () => {
const requestId = crypto.randomUUID();
const options = {
shareId: TEST_SHARE_ID,
queryParameters: {
assetType: AssetType.SHEET,
assetId: TEST_ASSET_ID,
},
customProperties: {
'x-request-id': requestId,
'x-test-name': '/errors/500-response',
},
};
try {
await client.sharing.deleteAssetShare(options);
expect(true).toBe(false); // Expected an error to be thrown
} catch (error: any) {
expect(error.statusCode).toBe(ERROR_500_STATUS_CODE);
expect(error.message).toBe(ERROR_500_MESSAGE);
}
});
});
Loading
Loading