Skip to content

Add CloudDevice service class#1709

Open
gcatanese wants to merge 4 commits into
mainfrom
add-cloud-device-service
Open

Add CloudDevice service class#1709
gcatanese wants to merge 4 commits into
mainfrom
add-cloud-device-service

Conversation

@gcatanese

@gcatanese gcatanese commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Add CloudDeviceApi service class to support the Cloud Device API endpoints:

  • /merchants/{merchantAccount}/devices/{deviceId}/sync
  • /merchants/{merchantAccount}/devices/{deviceId}/async
  • /merchants/{merchantAccount}/connectedDevices
  • /merchants/{merchantAccount}/devices/{deviceId}/status

Added supporting models (which are not part of the OpenAPI spec), unit tests and mock data.

@gcatanese gcatanese requested a review from a team as a code owner June 24, 2026 09:13
@gcatanese gcatanese added the Feature New feature or request label Jun 24, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the CloudDeviceAPI service to support synchronous and asynchronous payment requests, retrieving connected devices, and checking device status, complete with unit tests and mock data. Feedback on the implementation highlights a potential runtime crash and type unsafety in the async method when parsing responses, as well as unintended side effects caused by mutating the requestOptions parameter directly in getConnectedDevices.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +78 to +90
const response = await getJsonResponse<CloudDeviceApiRequest, string>(
resource,
cloudDeviceApiRequest,
{ ...requestOptions, method: "POST" }
);
const result = new CloudDeviceApiAsyncResponse();
if (typeof response === "string" && response.toLowerCase().trim() === "ok") {
result.Result = response.trim();
} else {
const parsed: CloudDeviceApiAsyncResponse = typeof response === "string" ? JSON.parse(response) : response;
result.SaleToPOIRequest = parsed.SaleToPOIRequest;
}
return result;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation of the async method is prone to runtime errors and contains type unsafety:

  1. Potential Crash on Empty/Invalid Response: If the API returns an empty response (which getJsonResponse resolves to "") or an invalid JSON string, calling JSON.parse(response) will throw an unhandled SyntaxError and crash the execution.
  2. Redundant Parsing: getJsonResponse already attempts to parse the response string into a JSON object. If successful, it returns the parsed object. Thus, calling JSON.parse again on an already parsed object (or on a string that failed parsing) is redundant and error-prone.
  3. Type Lie: Typing getJsonResponse with string as the response type (R) forces TypeScript to treat the parsed object as a string, leading to manual type casting and potential runtime issues.

We can resolve all these issues cleanly by typing getJsonResponse with CloudDeviceApiAsyncResponse and checking the type of the returned response.

        const response = await getJsonResponse<CloudDeviceApiRequest, CloudDeviceApiAsyncResponse>(
            resource,
            cloudDeviceApiRequest,
            { ...requestOptions, method: "POST" }
        );
        const result = new CloudDeviceApiAsyncResponse();
        if (typeof response === "string") {
            if (response.toLowerCase().trim() === "ok") {
                result.Result = response.trim();
            }
        } else if (response && typeof response === "object") {
            result.SaleToPOIRequest = response.SaleToPOIRequest;
        }
        return result;

Comment on lines +104 to +114
const hasDefinedQueryParams = store;
if (hasDefinedQueryParams) {
if (!requestOptions) requestOptions = {};
if (!requestOptions.params) requestOptions.params = {};
if (store) (requestOptions.params as Record<string, string>)["store"] = store;
}
const response = await getJsonResponse<string, ConnectedDevicesResponse>(
resource,
"",
{ ...requestOptions, method: "GET" }
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Mutating the requestOptions argument passed by the caller is a side effect that can lead to unexpected behavior if the caller reuses the same options object for subsequent requests.

Instead of mutating requestOptions directly, we should create a shallow copy of the options and append the query parameters safely.

        const options: IRequest.Options = { ...requestOptions, method: "GET" };
        if (store) {
            options.params = { ...options.params, store };
        }
        const response = await getJsonResponse<string, ConnectedDevicesResponse>(
            resource,
            "",
            options
        );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant