[jsandfordhughes]

Summary

• Rewrite the SDK as a Laravel package built on Saloon
• Add OAuth password-grant authentication, encrypted token caching and refresh handling
• Provide typed data objects, resources, enums and facade-based access to incidents, posts, locations, software assets and releases
• Add attachment uploads and downloads with mapped API exceptions
• Document installation, configuration, usage, testing and migration from v2
• Add Laravel package configuration, service provider discovery and verification tooling

Usage

Very simple fluent API

$incident = ServiceDeskify::incidents()->create(
    new CreateIncidentData(
        title: 'Till 2 is offline',
        details: 'The till remains offline after restarting.',
        storeNumber: 722,
        priority: IncidentPriority::P1,
    ),
);

$incidentId = $incident->id;

Testing

• Added Pest feature coverage for authentication, package configuration and resource groups

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1216949317449306

[jsandfordhughes]

@codex review.

Keep reviewing until all comments are made, do not stop at 2 comments.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e9aabd0ea5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jsandfordhughes]

@codex review.

Keep reviewing until all comments are made, do not stop at 2 comments.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 73192f893e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[AndreVME]

Reviewed this against the current servicedeskify-service API (routes/api.php, the Api\* controllers, the form requests, ModelFilters/* and Http/Resources/*) rather than just reading the diff, so most of the "is this still compatible?" questions are now settled rather than open.

Headline: the contract modelling is accurate. I checked every endpoint and the SDK matches the service on all of them:

Endpoint	Service returns	SDK reads
GET /api/incidents	IncidentResource::collection(paginate(50)) — data+links+meta	json() → PaginatedData	OK
GET/POST /api/incidents{,/{id}}	data-wrapped	json('data')	OK
GET /api/incidents/{id}/posts	PostResource::collection(paginate(request('per_page', 15)))	json() → PaginatedData	OK
POST .../posts	data-wrapped, 201	json('data')	OK
POST .../close	response('', 200), empty	void	OK
POST .../closure-details	204, empty	void	OK
GET /api/locations	raw LengthAwarePaginator (flat shape)	PaginatedData fallback path	OK
GET /api/software-assets	bare root array, no pagination	root json(), plain array	OK
GET /api/software-assets/{id}	raw model at root (currentVersion, current_release)	current_release_id ?? currentVersion etc.	OK
POST .../{id}/releases	root {software_asset_version_id, master_software_asset_version_id}, 201	root json()	OK
GET /api/releases	bare root array, no pagination	root json(), plain array	OK
PUT /api/releases/{id}	204	void	OK

Three things worth calling out explicitly:

1. The integer priority is correct, and it fixes a live bug. IncidentRequest::rules() is 'priority' => 'integer|min:1|max:5' — the Rule::in(['Low','Moderate','High','Critical']) line is commented out directly beneath it. Incident::PRIORITIES is [1 => Critical, 2 => High, 3 => Moderate, 4 => Low, 5 => Very Low], which is byte-for-byte the mapping in IncidentPriority::fromApiValue(). That means v2's ServiceDeskifyConstants::PRIORITY_HIGH ('High') fails validation against the API as it stands today — this PR repairs that. Worth stating in the PR description. (The @bodyParam priority string ... Low, Moderate, High, Critical docblock on IncidentController@store is stale and should get its own PR on the service repo.)
2. The query DTOs are complete, not a narrowing. IncidentFilter implements exactly one method (status, and only the value open) with paginate(50) hard-coded; LocationFilter is exactly store/name/customer; PostFilter is exactly since/sortDirection with per_page honoured by the controller. IncidentQueryData, LocationQueryData and IncidentPostQueryData mirror all three precisely. Nothing is missing.
3. The DTOs are 1:1 with the API resources. IncidentResource::toArray() returns exactly the 15 fields IncidentData models; PostResource exactly the 6 in IncidentPostData; HelpDeskUser::getFromCache() exactly username/avatar/email; Location::getFromCache() exactly name/customer/store_number; the PostAttachment resource exactly file_name/download_url. No fields are dropped today.

Also confirmed: users/locations genuinely no longer exists in routes/api.php, so removing it is right.

The comments below are therefore mostly hardening rather than contract problems. Two I'd fix before merge (the priority 500 and the timezone parse); the rest are judgement calls.

---

Not blocking, but worth deciding on

No escape hatch to the raw response. v2 returned a PSR-7 object, so callers had the status code, headers and the raw JSON string. v3 exposes none of it, and there is no way to reach an endpoint the SDK does not model (e.g. /api/user, or /me/shifts). Consider exposing the Saloon\Http\Response, or making ServiceDeskifyClient::send() reachable so consumers can send their own Saloon requests through the authenticated connector.

Single credential set. TokenManager, ServiceDeskifyClient and ServiceDeskifyManager are all container singletons bound from config, so an app cannot authenticate as more than one ServiceDeskify user or hit test and prod in the same process. v2 could. Probably fine for current consumers, but it is a deliberate capability drop and worth recording as such.

Migration section. "Migrating from v2" currently lists deleted classes, which tells a reader what broke but not what to type instead. A method-by-method mapping table (old call → new call → new return type), including the priority constant mapping (PRIORITY_CRITICAL → P1 ... PRIORITY_LOW → P4, which inverts against intuition) and the replacement for getBodyAsJson(), would carry most of the upgrade cost.

[AndreVME]

Re-reviewed against d195274. I pulled the branch, ran composer install and the suite locally — 43 passed, 139 assertions, and all four CI matrix jobs are green.

The two things I'd flagged as fix-before-merge are properly resolved, with tests that cover the actual regression rather than the happy path: priority defaulting to P3 closes the 500, and CarbonImmutable::parse($value, 'UTC') is verified by a test that sets app.timezone to Europe/London first. The identity guards, Countable/IteratorAggregate, lazy()/each(), the SoftwareAssetSummaryData split for all(), and the attachment-handle unwind all landed cleanly too. Declining the storeNumber/locationId and summary-is-HTML docblocks is fair — both were documentation-only.

Three of the fixes introduced small follow-ups, and a few items were skipped without comment. Everything below I verified against this head rather than assuming; the two one-line ones are the only things I'd actively press for.

Also still open from the first pass, not re-raised inline to keep this short: ServiceDeskifyClient still builds a fresh connector (and Guzzle client) per call with no timeout or middleware configuration, and IncidentPostData::$detail vs CreateIncidentPostData::$details is still unaliased and undocumented. Both remain judgement calls.