fix(psa): resource assignment targets CW owner, status PATCH verifies apply
Previous `resources`-string PATCH was silently ignored by CW — the `resources` field is server-derived from the ticket's owner + schedule entries, not freely writable. Status PATCH could also silently no-op when a cross-board status id was sent. - add_resource: when the ticket is unassigned, set the `owner` MemberReference (the canonical writable primary-assignee field). If already owned by someone else, append the identifier to the `resources` co-assignee string best-effort. - remove_resource: clear `owner` (with remove→replace:null fallback) if the target is the current owner, otherwise strip from `resources`. - list_resources: merge owner + resources string, deduped by member id, so the UI reflects both single-owner and multi-resource assignments. - update_ticket_status: verify CW applied the status by comparing the response body's status.id — raises PSAError with a clear message when CW silently rejects the change (e.g., status invalid for ticket's board), instead of reporting spurious success. - Frontend: surface the backend error detail in the toast so users see the real reason instead of a generic "Failed to update" message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -266,13 +266,30 @@ class ConnectWiseProvider(PSAProvider):
|
|||||||
async def update_ticket_status(
|
async def update_ticket_status(
|
||||||
self, ticket_id: str, status_id: int
|
self, ticket_id: str, status_id: int
|
||||||
) -> PSATicket:
|
) -> PSATicket:
|
||||||
"""Update a CW ticket's status using JSON Patch format."""
|
"""Update a CW ticket's status using JSON Patch format.
|
||||||
|
|
||||||
|
Verifies CW actually applied the change — CW silently returns 200 when
|
||||||
|
a status id is invalid for the ticket's board. We check the response
|
||||||
|
body's status.id matches what we sent, and raise PSAError if not.
|
||||||
|
"""
|
||||||
patch_body = [
|
patch_body = [
|
||||||
{"op": "replace", "path": "status", "value": {"id": status_id}}
|
{"op": "replace", "path": "status", "value": {"id": status_id}}
|
||||||
]
|
]
|
||||||
data = await self.client.patch(
|
data = await self.client.patch(
|
||||||
f"/service/tickets/{ticket_id}", json_body=patch_body
|
f"/service/tickets/{ticket_id}", json_body=patch_body
|
||||||
)
|
)
|
||||||
|
applied = (data.get("status") or {}) if isinstance(data, dict) else {}
|
||||||
|
applied_id = applied.get("id")
|
||||||
|
if applied_id != status_id:
|
||||||
|
logger.warning(
|
||||||
|
"CW status PATCH for ticket %s returned status id=%s instead of %s",
|
||||||
|
ticket_id, applied_id, status_id,
|
||||||
|
)
|
||||||
|
raise PSAError(
|
||||||
|
f"ConnectWise did not apply status {status_id} "
|
||||||
|
f"(still {applied.get('name') or applied_id}). "
|
||||||
|
"The status may not be valid for this ticket's board."
|
||||||
|
)
|
||||||
return self._map_ticket(data)
|
return self._map_ticket(data)
|
||||||
|
|
||||||
async def list_members(self) -> list[PSAMember]:
|
async def list_members(self) -> list[PSAMember]:
|
||||||
@@ -631,58 +648,107 @@ class ConnectWiseProvider(PSAProvider):
|
|||||||
|
|
||||||
# ── Resource management ───────────────────────────────────────────
|
# ── Resource management ───────────────────────────────────────────
|
||||||
|
|
||||||
async def _get_ticket_resource_identifiers(self, ticket_id: int) -> list[str]:
|
async def _get_ticket_assignment(self, ticket_id: int) -> tuple[dict | None, str | None]:
|
||||||
"""Fetch the ticket's `resources` field (comma-separated identifiers)."""
|
"""Fetch the ticket's current owner (MemberReference) and resources string.
|
||||||
|
|
||||||
|
Returns (owner_dict, resources_string). owner_dict has keys like
|
||||||
|
{id, identifier, name} when set; None when unassigned.
|
||||||
|
"""
|
||||||
data = await self.client.get(
|
data = await self.client.get(
|
||||||
f"/service/tickets/{ticket_id}",
|
f"/service/tickets/{ticket_id}",
|
||||||
params={"fields": "id,resources"},
|
params={"fields": "id,owner,resources"},
|
||||||
)
|
)
|
||||||
raw = data.get("resources") if isinstance(data, dict) else None
|
if not isinstance(data, dict):
|
||||||
if not raw:
|
return None, None
|
||||||
return []
|
owner_raw = data.get("owner")
|
||||||
return [p.strip() for p in str(raw).split(",") if p.strip()]
|
owner = owner_raw if isinstance(owner_raw, dict) and owner_raw.get("id") else None
|
||||||
|
resources = data.get("resources")
|
||||||
|
return owner, (str(resources) if resources else None)
|
||||||
|
|
||||||
async def list_resources(self, ticket_id: int) -> list[PSAResource]:
|
async def list_resources(self, ticket_id: int) -> list[PSAResource]:
|
||||||
"""List members assigned to a CW ticket via the `resources` string field."""
|
"""List members assigned to a CW ticket.
|
||||||
identifiers = await self._get_ticket_resource_identifiers(ticket_id)
|
|
||||||
if not identifiers:
|
CW exposes assignment in two places: the `owner` MemberReference (primary
|
||||||
return []
|
assignee) and a derived `resources` string of identifiers. We merge both
|
||||||
|
so the UI shows everyone CW considers assigned.
|
||||||
|
"""
|
||||||
|
owner, resources_str = await self._get_ticket_assignment(ticket_id)
|
||||||
members = await self.list_members()
|
members = await self.list_members()
|
||||||
by_identifier = {m.identifier: m for m in members if m.identifier}
|
by_identifier = {m.identifier: m for m in members if m.identifier}
|
||||||
|
by_id = {str(m.id): m for m in members}
|
||||||
|
|
||||||
|
seen_ids: set[str] = set()
|
||||||
results: list[PSAResource] = []
|
results: list[PSAResource] = []
|
||||||
for ident in identifiers:
|
|
||||||
m = by_identifier.get(ident)
|
if owner is not None:
|
||||||
|
owner_id = str(owner.get("id"))
|
||||||
|
m = by_id.get(owner_id)
|
||||||
if m:
|
if m:
|
||||||
results.append(PSAResource(
|
results.append(PSAResource(
|
||||||
member_id=int(m.id),
|
member_id=int(m.id),
|
||||||
member_name=m.name,
|
member_name=m.name,
|
||||||
member_identifier=m.identifier,
|
member_identifier=m.identifier,
|
||||||
))
|
))
|
||||||
|
seen_ids.add(owner_id)
|
||||||
else:
|
else:
|
||||||
# Unknown identifier — surface it so the UI doesn't silently drop it
|
|
||||||
results.append(PSAResource(
|
results.append(PSAResource(
|
||||||
member_id=0,
|
member_id=int(owner.get("id") or 0),
|
||||||
member_name=ident,
|
member_name=str(owner.get("name") or ""),
|
||||||
member_identifier=ident,
|
member_identifier=str(owner.get("identifier") or ""),
|
||||||
))
|
))
|
||||||
|
seen_ids.add(owner_id)
|
||||||
|
|
||||||
|
if resources_str:
|
||||||
|
for ident in (p.strip() for p in resources_str.split(",") if p.strip()):
|
||||||
|
m = by_identifier.get(ident)
|
||||||
|
if m and str(m.id) not in seen_ids:
|
||||||
|
results.append(PSAResource(
|
||||||
|
member_id=int(m.id),
|
||||||
|
member_name=m.name,
|
||||||
|
member_identifier=m.identifier,
|
||||||
|
))
|
||||||
|
seen_ids.add(str(m.id))
|
||||||
|
elif not m:
|
||||||
|
results.append(PSAResource(
|
||||||
|
member_id=0,
|
||||||
|
member_name=ident,
|
||||||
|
member_identifier=ident,
|
||||||
|
))
|
||||||
|
|
||||||
return results
|
return results
|
||||||
|
|
||||||
async def add_resource(self, ticket_id: int, member_id: int) -> PSAResource:
|
async def add_resource(self, ticket_id: int, member_id: int) -> PSAResource:
|
||||||
"""Assign a member to a CW ticket by updating the ticket's `resources` field."""
|
"""Assign a member to a CW ticket.
|
||||||
|
|
||||||
|
CW's `resources` string is derived server-side and PATCHing it is unreliable.
|
||||||
|
The canonical writable field is `owner`. If the ticket is unassigned we set
|
||||||
|
the owner; if it already has a different owner, we append to `resources`
|
||||||
|
(best-effort — CW may not surface it in every view).
|
||||||
|
"""
|
||||||
members = await self.list_members()
|
members = await self.list_members()
|
||||||
target = next((m for m in members if str(m.id) == str(member_id)), None)
|
target = next((m for m in members if str(m.id) == str(member_id)), None)
|
||||||
if target is None or not target.identifier:
|
if target is None:
|
||||||
raise PSAError(f"Member {member_id} not found or has no identifier")
|
raise PSAError(f"Member {member_id} not found")
|
||||||
|
|
||||||
current = await self._get_ticket_resource_identifiers(ticket_id)
|
current_owner, resources_str = await self._get_ticket_assignment(ticket_id)
|
||||||
if target.identifier not in current:
|
|
||||||
current.append(target.identifier)
|
if current_owner is None:
|
||||||
new_value = ",".join(current)
|
# Primary assign — set owner
|
||||||
|
await self.client.patch(
|
||||||
|
f"/service/tickets/{ticket_id}",
|
||||||
|
json_body=[{"op": "replace", "path": "owner", "value": {"id": int(target.id)}}],
|
||||||
|
)
|
||||||
|
elif str(current_owner.get("id")) != str(target.id):
|
||||||
|
# Ticket already owned by someone else — add to resources string as co-assignee.
|
||||||
|
identifiers = [p.strip() for p in (resources_str or "").split(",") if p.strip()]
|
||||||
|
if target.identifier and target.identifier not in identifiers:
|
||||||
|
identifiers.append(target.identifier)
|
||||||
|
await self.client.patch(
|
||||||
|
f"/service/tickets/{ticket_id}",
|
||||||
|
json_body=[{"op": "replace", "path": "resources", "value": ",".join(identifiers)}],
|
||||||
|
)
|
||||||
|
# else: already the owner — idempotent no-op
|
||||||
|
|
||||||
await self.client.patch(
|
|
||||||
f"/service/tickets/{ticket_id}",
|
|
||||||
json_body=[{"op": "replace", "path": "resources", "value": new_value}],
|
|
||||||
)
|
|
||||||
return PSAResource(
|
return PSAResource(
|
||||||
member_id=int(target.id),
|
member_id=int(target.id),
|
||||||
member_name=target.name,
|
member_name=target.name,
|
||||||
@@ -690,22 +756,41 @@ class ConnectWiseProvider(PSAProvider):
|
|||||||
)
|
)
|
||||||
|
|
||||||
async def remove_resource(self, ticket_id: int, member_id: int) -> None:
|
async def remove_resource(self, ticket_id: int, member_id: int) -> None:
|
||||||
"""Remove a member from a CW ticket by updating the `resources` field (idempotent)."""
|
"""Remove a member from a CW ticket (idempotent).
|
||||||
|
|
||||||
|
If the target is the current owner, clear the owner. Otherwise remove them
|
||||||
|
from the `resources` string.
|
||||||
|
"""
|
||||||
members = await self.list_members()
|
members = await self.list_members()
|
||||||
target = next((m for m in members if str(m.id) == str(member_id)), None)
|
target = next((m for m in members if str(m.id) == str(member_id)), None)
|
||||||
if target is None or not target.identifier:
|
if target is None:
|
||||||
return # Unknown member — treat as idempotent
|
return
|
||||||
|
|
||||||
current = await self._get_ticket_resource_identifiers(ticket_id)
|
current_owner, resources_str = await self._get_ticket_assignment(ticket_id)
|
||||||
if target.identifier not in current:
|
|
||||||
return # Already not assigned — idempotent
|
|
||||||
new_list = [i for i in current if i != target.identifier]
|
|
||||||
new_value = ",".join(new_list)
|
|
||||||
|
|
||||||
await self.client.patch(
|
if current_owner is not None and str(current_owner.get("id")) == str(target.id):
|
||||||
f"/service/tickets/{ticket_id}",
|
# Unassign the owner. Try RFC 6902 "remove" first; fall back to
|
||||||
json_body=[{"op": "replace", "path": "resources", "value": new_value}],
|
# "replace" with null if CW rejects it.
|
||||||
)
|
try:
|
||||||
|
await self.client.patch(
|
||||||
|
f"/service/tickets/{ticket_id}",
|
||||||
|
json_body=[{"op": "remove", "path": "owner"}],
|
||||||
|
)
|
||||||
|
except PSAError:
|
||||||
|
await self.client.patch(
|
||||||
|
f"/service/tickets/{ticket_id}",
|
||||||
|
json_body=[{"op": "replace", "path": "owner", "value": None}],
|
||||||
|
)
|
||||||
|
return
|
||||||
|
|
||||||
|
if target.identifier and resources_str:
|
||||||
|
identifiers = [p.strip() for p in resources_str.split(",") if p.strip()]
|
||||||
|
if target.identifier in identifiers:
|
||||||
|
new_list = [i for i in identifiers if i != target.identifier]
|
||||||
|
await self.client.patch(
|
||||||
|
f"/service/tickets/{ticket_id}",
|
||||||
|
json_body=[{"op": "replace", "path": "resources", "value": ",".join(new_list)}],
|
||||||
|
)
|
||||||
|
|
||||||
# ── Ticket creation ───────────────────────────────────────────────
|
# ── Ticket creation ───────────────────────────────────────────────
|
||||||
|
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import { useState } from 'react'
|
import { useState } from 'react'
|
||||||
|
import axios from 'axios'
|
||||||
import { ticketsApi } from '@/api/tickets'
|
import { ticketsApi } from '@/api/tickets'
|
||||||
import { toast } from '@/lib/toast'
|
import { toast } from '@/lib/toast'
|
||||||
import type { PSATicketSearchResult, PSATicketStatusItem } from '@/types/integrations'
|
import type { PSATicketSearchResult, PSATicketStatusItem } from '@/types/integrations'
|
||||||
@@ -22,8 +23,11 @@ export function TicketDetailHeader({ ticket, currentStatusId, currentStatusName,
|
|||||||
const result: PSATicketStatusUpdate = await ticketsApi.updateStatus(Number(ticket.id), statusId)
|
const result: PSATicketStatusUpdate = await ticketsApi.updateStatus(Number(ticket.id), statusId)
|
||||||
onStatusUpdated(result.ticket_id, result.new_status, result.new_status_id)
|
onStatusUpdated(result.ticket_id, result.new_status, result.new_status_id)
|
||||||
toast.success(`Status updated to ${result.new_status}`)
|
toast.success(`Status updated to ${result.new_status}`)
|
||||||
} catch {
|
} catch (err) {
|
||||||
toast.error('Failed to update status')
|
const detail = axios.isAxiosError(err)
|
||||||
|
? (err.response?.data as { detail?: string })?.detail
|
||||||
|
: undefined
|
||||||
|
toast.error(detail || 'Failed to update status')
|
||||||
} finally {
|
} finally {
|
||||||
setUpdating(false)
|
setUpdating(false)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import { useState } from 'react'
|
import { useState } from 'react'
|
||||||
|
import axios from 'axios'
|
||||||
import { UserPlus, X, User } from 'lucide-react'
|
import { UserPlus, X, User } from 'lucide-react'
|
||||||
import { ticketsApi } from '@/api/tickets'
|
import { ticketsApi } from '@/api/tickets'
|
||||||
import { toast } from '@/lib/toast'
|
import { toast } from '@/lib/toast'
|
||||||
@@ -27,8 +28,11 @@ export function TicketResourceManager({ ticketId, resources, allMembers, onChang
|
|||||||
setAdding(false)
|
setAdding(false)
|
||||||
setSelectedMemberId('')
|
setSelectedMemberId('')
|
||||||
onChanged()
|
onChanged()
|
||||||
} catch {
|
} catch (err) {
|
||||||
toast.error('Failed to add resource')
|
const detail = axios.isAxiosError(err)
|
||||||
|
? (err.response?.data as { detail?: string })?.detail
|
||||||
|
: undefined
|
||||||
|
toast.error(detail || 'Failed to add resource')
|
||||||
} finally {
|
} finally {
|
||||||
setBusy(null)
|
setBusy(null)
|
||||||
}
|
}
|
||||||
@@ -40,8 +44,11 @@ export function TicketResourceManager({ ticketId, resources, allMembers, onChang
|
|||||||
await ticketsApi.removeResource(ticketId, memberId)
|
await ticketsApi.removeResource(ticketId, memberId)
|
||||||
toast.success('Resource removed')
|
toast.success('Resource removed')
|
||||||
onChanged()
|
onChanged()
|
||||||
} catch {
|
} catch (err) {
|
||||||
toast.error('Failed to remove resource')
|
const detail = axios.isAxiosError(err)
|
||||||
|
? (err.response?.data as { detail?: string })?.detail
|
||||||
|
: undefined
|
||||||
|
toast.error(detail || 'Failed to remove resource')
|
||||||
} finally {
|
} finally {
|
||||||
setBusy(null)
|
setBusy(null)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user