Add inspector_image_url support to createInspection

[apotema]

Summary

• Add optional inspectorImageUrl parameter to the createInspection method
• Update InspectionRequest interface to include inspector_image_url field
• Include inspector_image_url in API request payload when provided

Changes

• src/client.ts: Updated method signature and request building logic
• tests/client.test.ts: Added tests for both with and without inspector image URL

Related Issues

Closes #1
Closes #2

Test plan

• Unit tests pass for createInspection without inspectorImageUrl
• Unit tests pass for createInspection with inspectorImageUrl
• Backwards compatible - existing calls continue to work

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR adds support for an optional inspectorImageUrl parameter to the createInspection method, allowing the SDK to pass inspector profile images to the Fixle API. This change enables the gcp-fixle Cloud Function to use the SDK natively instead of making custom HTTP requests to include the inspector image URL received from Spectora webhooks.

Key changes:

• Extended createInspection method to accept an optional inspectorImageUrl parameter
• Updated InspectionRequest interface to include the inspector_image_url field
• Added comprehensive test coverage for both scenarios (with and without the image URL)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/client.ts	Added optional inspectorImageUrl parameter to createInspection method signature, updated InspectionRequest interface, and implemented conditional payload inclusion using spread operator
tests/client.test.ts	Added two test cases covering createInspection behavior both with and without the inspectorImageUrl parameter

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

There is significant code duplication between the two test cases. The mock setup code (lines 52-69 and 79-96) is nearly identical. Consider extracting the common mock setup into a helper function or a beforeEach block to improve maintainability and reduce duplication.

[apotema]

🤖 Claude Code Review

PR Review: Add Inspector Image URL Support

✅ What's Good

• Clean API Design: Optional parameter with sensible default behavior
• Proper TypeScript: Interface updated correctly with optional field
• Good Documentation: JSDoc updated with examples and parameter description
• Comprehensive Testing: Both scenarios (with/without image URL) are tested
• Backward Compatibility: Existing code continues to work unchanged

🔧 Potential Improvements

1. URL Validation

Consider adding basic URL validation for the inspectorImageUrl parameter:

if (inspectorImageUrl && !isValidUrl(inspectorImageUrl)) {
  throw new Error('Invalid inspector image URL provided');
}

2. Test Duplication

The test setup is duplicated across both test cases. Consider extracting to a helper function:

const setupMockHttp = () => {
  const mockResponse = {
    statusCode: 201,
    on: jest.fn((event, handler) => {
      if (event === 'data') handler('{"data":{"id":"456"}}');
      if (event === 'end') handler();
    }),
  };
  
  let capturedBody = '';
  const http = require('http');
  http.request = jest.fn((options, callback) => {
    callback(mockResponse);
    return {
      on: jest.fn(),
      write: jest.fn((data: string) => { capturedBody = data; }),
      end: jest.fn(),
    };
  });
  
  return { capturedBody: () => capturedBody };
};

3. Documentation Enhancement

Consider adding validation details to the JSDoc:

* @param inspectorImageUrl - Optional URL to the inspector's profile image (must be valid HTTP/HTTPS URL)

4. Edge Case Testing

Consider adding tests for edge cases:

• Empty string URL
• Invalid URL format
• Very long URLs

📝 Minor Notes

• Nice use of spread operator with conditional inclusion
• Good choice making the parameter optional rather than creating overloaded methods
• Test assertions are clear and focused

🎯 Overall Assessment

This is a solid, well-implemented feature addition. The code follows good practices and maintains backward compatibility. The main areas for improvement are around input validation and test organization, but these are minor enhancements to an already good implementation.

Recommendation: ✅ Approve with minor suggestions

---

Automated review by Claude Code (Sonnet 4)

[apotema]

🤖 Cursor Agent Review

• ...(inspectorImageUrl && { inspector_image_url: inspectorImageUrl }) drops the field when a consumer purposely passes an empty string (common when clearing images). Consider checking for inspectorImageUrl !== undefined instead so falsy-but-intentional values still flow to the API.```246:251:src/client.ts
  const inspectionData: InspectionRequest = {
  inspection: {
  external_id: inspectionId.toString(),
  ...(inspectorImageUrl && { inspector_image_url: inspectorImageUrl }),
  },
  };

- The new `createInspection` tests monkey-patch `http.request` but never restore it. Because this happens inside the shared `describe`, the stub leaks into subsequent tests in the file. Capture the original function in a `beforeEach`/`afterEach` or use `jest.spyOn(http, 'request')` with `mockRestore()` to prevent cross-test interference.```49:93:tests/client.test.ts
  const http = require('http');
  let capturedBody = '';
  http.request = jest.fn((options, callback) => {
    callback(mockResponse);
    return {
      on: jest.fn(),
      write: jest.fn((data: string) => { capturedBody = data; }),
      end: jest.fn(),
    };
  });

• Nice job extending the JSDoc with a concrete example and covering both presence/absence of inspectorImageUrl in unit tests—makes the API change discoverable and protects the regression.

---

Automated review by cursor-agent (GPT-5.1 Codex)

[apotema]

🤖 Cursor Agent Review (GPT-5.2)

High-signal feedback

✅ What’s good

• Backwards-compatible API change: Adding an optional inspectorImageUrl?: string keeps existing callers working.
• Clean request shaping: The conditional spread ...(inspectorImageUrl && { inspector_image_url: inspectorImageUrl }) is concise and avoids sending null/undefined.
• Docs improved: JSDoc now includes a clear example for the new optional parameter.

⚠️ Potential issues / improvements

• Validation: Consider validating inspectorImageUrl (at least non-empty + basic URL format). Right now any string gets sent (including whitespace).
• Empty string behavior: '' will be treated as “not provided” due to truthiness. If empty string should be rejected or sent explicitly, handle it intentionally.
• Test isolation / mocking risk:
  • These tests mutate http.request globally and don’t restore it. This can leak into other tests and cause flaky failures.
  • Prefer jest.spyOn(http, 'request') with mockImplementation, and restore in afterEach(() => jest.restoreAllMocks()).
• Mock response shape: Your response on('data') emits a string once; real streams can emit multiple chunks. If the client code concatenates chunks, you’re fine—but if not, tests might not reflect reality. Consider emitting multiple chunks to be resilient.
• Coverage gap: Add a test for URL included but invalid (if you add validation), and potentially for inspection_date/inspector_name interplay if the request supports multiple optional fields.

🧼 Minor style nits

• JSDoc formatting: Multiple @example blocks are fine, but keep them tight; also ensure the signature in docs matches the function (it does now).

---

Automated review by cursor-agent (GPT-5.2)