mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 06:17:41 +00:00
fix(contacts): merge contact into existing tenant user
- user_search: return tenant_user UUID in new "uuid" field so callers
forwarding a tenant_user foreign key receive the primary key instead
of the user_id string (root cause of HTTP 400 "invalid JSON" on
/v1/contacts/merge when linking to an existing tenant user)
- use-user-picker: add opt-in valueMode prop ("user_id" default | "uuid")
so only merge-contacts-dialog commits UUID; other pickers (tenant
add-user, MCP/CLI credentials) keep returning user_id string
- pg.MigrateUserDataOnMerge: split tClauseIns/tClauseDel with correct
placeholder offsets (N+2 for INSERT/UPDATE, N+1 for DELETE) — reusing
one clause caused "no parameter \$N+2" errors that silently skipped
every cleanup DELETE, leaving orphan per-user rows after a merge
This commit is contained in:
1 parent
f2325f6d98
commit
3c39b7d2a0
5 files changed
+62
-25
No files matched your search
@@ -12,15 +12,19 @@ import (
|
||||
)
|
||||
|
||||
// UserSearchResult is a unified result from contacts + tenant_users.
|
||||
// ID is the user_id string (human-facing identifier). UUID is the tenant_user
|
||||
// primary key (only populated when Source == "tenant_user"); callers that need
|
||||
// to reference a tenant_user by foreign key (e.g. contact merge) must use UUID.
|
||||
type UserSearchResult struct {
|
||||
ID string `json:"id"`
|
||||
DisplayName *string `json:"display_name,omitempty"`
|
||||
Username *string `json:"username,omitempty"`
|
||||
Source string `json:"source"` // "contact" or "tenant_user"
|
||||
ChannelType *string `json:"channel_type,omitempty"`
|
||||
PeerKind *string `json:"peer_kind,omitempty"`
|
||||
MergedTenantUserID *string `json:"merged_tenant_user_id,omitempty"`
|
||||
Role *string `json:"role,omitempty"`
|
||||
ID string `json:"id"`
|
||||
UUID string `json:"uuid,omitempty"`
|
||||
DisplayName *string `json:"display_name,omitempty"`
|
||||
Username *string `json:"username,omitempty"`
|
||||
Source string `json:"source"` // "contact" or "tenant_user"
|
||||
ChannelType *string `json:"channel_type,omitempty"`
|
||||
PeerKind *string `json:"peer_kind,omitempty"`
|
||||
MergedTenantUserID *string `json:"merged_tenant_user_id,omitempty"`
|
||||
Role *string `json:"role,omitempty"`
|
||||
}
|
||||
|
||||
// handleSearchUsers returns unified results from channel_contacts + tenant_users.
|
||||
@@ -91,6 +95,7 @@ func (h *ChannelInstancesHandler) handleSearchUsers(w http.ResponseWriter, r *ht
|
||||
role := u.Role
|
||||
results = append(results, UserSearchResult{
|
||||
ID: u.UserID,
|
||||
UUID: u.ID.String(),
|
||||
DisplayName: u.DisplayName,
|
||||
Source: "tenant_user",
|
||||
Role: &role,
|
||||
|
||||
@@ -126,7 +126,16 @@ func (s *PGAgentStore) MigrateUserDataOnMerge(ctx context.Context, oldUserIDs []
|
||||
if len(oldUserIDs) == 0 {
|
||||
return nil
|
||||
}
|
||||
tClause, tArgs, _, err := scopeClause(ctx, len(oldUserIDs)+2)
|
||||
// INSERT / UPDATE queries pass args in order [oldIDs..., newUserID, tArgs...],
|
||||
// so tenant-scope placeholders start at position N+2.
|
||||
tClauseIns, tArgs, _, err := scopeClause(ctx, len(oldUserIDs)+2)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
// DELETE queries drop the newUserID placeholder and pass [oldIDs..., tArgs...],
|
||||
// so tenant-scope placeholders must start at N+1. Reusing tClauseIns here
|
||||
// produces "there is no parameter $N+2" errors that silently corrupt cleanup.
|
||||
tClauseDel, _, _, err := scopeClause(ctx, len(oldUserIDs)+1)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -143,8 +152,8 @@ func (s *PGAgentStore) MigrateUserDataOnMerge(ctx context.Context, oldUserIDs []
|
||||
baseArgs = append(baseArgs, tArgs...)
|
||||
|
||||
// Args for delete (no newUserID param needed).
|
||||
delArgs := make([]any, len(oldUserIDs))
|
||||
copy(delArgs, baseArgs[:len(oldUserIDs)])
|
||||
delArgs := make([]any, 0, len(oldUserIDs)+len(tArgs))
|
||||
delArgs = append(delArgs, baseArgs[:len(oldUserIDs)]...)
|
||||
delArgs = append(delArgs, tArgs...)
|
||||
|
||||
// Helper: migrate + delete for one table. DO NOTHING on conflict —
|
||||
@@ -163,8 +172,8 @@ func (s *PGAgentStore) MigrateUserDataOnMerge(ctx context.Context, oldUserIDs []
|
||||
fmt.Sprintf(`INSERT INTO user_context_files (id, agent_id, user_id, file_name, content, updated_at, tenant_id)
|
||||
SELECT gen_random_uuid(), agent_id, %s, file_name, content, updated_at, tenant_id
|
||||
FROM user_context_files WHERE user_id IN (%s)%s
|
||||
ON CONFLICT (agent_id, user_id, file_name) DO NOTHING`, newP, inClause, tClause),
|
||||
fmt.Sprintf(`DELETE FROM user_context_files WHERE user_id IN (%s)%s`, inClause, tClause),
|
||||
ON CONFLICT (agent_id, user_id, file_name) DO NOTHING`, newP, inClause, tClauseIns),
|
||||
fmt.Sprintf(`DELETE FROM user_context_files WHERE user_id IN (%s)%s`, inClause, tClauseDel),
|
||||
)
|
||||
|
||||
// 2. user_agent_overrides: UNIQUE(agent_id, user_id)
|
||||
@@ -172,8 +181,8 @@ func (s *PGAgentStore) MigrateUserDataOnMerge(ctx context.Context, oldUserIDs []
|
||||
fmt.Sprintf(`INSERT INTO user_agent_overrides (id, agent_id, user_id, provider, model, settings, created_at, updated_at, tenant_id)
|
||||
SELECT gen_random_uuid(), agent_id, %s, provider, model, settings, created_at, updated_at, tenant_id
|
||||
FROM user_agent_overrides WHERE user_id IN (%s)%s
|
||||
ON CONFLICT (agent_id, user_id) DO NOTHING`, newP, inClause, tClause),
|
||||
fmt.Sprintf(`DELETE FROM user_agent_overrides WHERE user_id IN (%s)%s`, inClause, tClause),
|
||||
ON CONFLICT (agent_id, user_id) DO NOTHING`, newP, inClause, tClauseIns),
|
||||
fmt.Sprintf(`DELETE FROM user_agent_overrides WHERE user_id IN (%s)%s`, inClause, tClauseDel),
|
||||
)
|
||||
|
||||
// 3. user_agent_profiles: PK(agent_id, user_id)
|
||||
@@ -181,8 +190,8 @@ func (s *PGAgentStore) MigrateUserDataOnMerge(ctx context.Context, oldUserIDs []
|
||||
fmt.Sprintf(`INSERT INTO user_agent_profiles (agent_id, user_id, workspace, first_seen_at, last_seen_at, metadata, tenant_id)
|
||||
SELECT agent_id, %s, workspace, first_seen_at, last_seen_at, metadata, tenant_id
|
||||
FROM user_agent_profiles WHERE user_id IN (%s)%s
|
||||
ON CONFLICT (agent_id, user_id) DO NOTHING`, newP, inClause, tClause),
|
||||
fmt.Sprintf(`DELETE FROM user_agent_profiles WHERE user_id IN (%s)%s`, inClause, tClause),
|
||||
ON CONFLICT (agent_id, user_id) DO NOTHING`, newP, inClause, tClauseIns),
|
||||
fmt.Sprintf(`DELETE FROM user_agent_profiles WHERE user_id IN (%s)%s`, inClause, tClauseDel),
|
||||
)
|
||||
|
||||
// 4. memory_documents: UNIQUE(agent_id, COALESCE(user_id,''), path)
|
||||
@@ -190,14 +199,15 @@ func (s *PGAgentStore) MigrateUserDataOnMerge(ctx context.Context, oldUserIDs []
|
||||
fmt.Sprintf(`INSERT INTO memory_documents (id, agent_id, user_id, path, content, hash, updated_at, created_at, tenant_id)
|
||||
SELECT gen_random_uuid(), agent_id, %s, path, content, hash, updated_at, created_at, tenant_id
|
||||
FROM memory_documents WHERE user_id IN (%s)%s
|
||||
ON CONFLICT (agent_id, COALESCE(user_id,''), path) DO NOTHING`, newP, inClause, tClause),
|
||||
fmt.Sprintf(`DELETE FROM memory_documents WHERE user_id IN (%s)%s`, inClause, tClause),
|
||||
ON CONFLICT (agent_id, COALESCE(user_id,''), path) DO NOTHING`, newP, inClause, tClauseIns),
|
||||
fmt.Sprintf(`DELETE FROM memory_documents WHERE user_id IN (%s)%s`, inClause, tClauseDel),
|
||||
)
|
||||
|
||||
// 5. memory_chunks: FK on document_id — cascade from memory_documents delete handles this.
|
||||
// But orphan chunks (where document was already migrated) need cleanup.
|
||||
// Simply re-point remaining chunks whose document still has old user_id.
|
||||
repoint := fmt.Sprintf(`UPDATE memory_chunks SET user_id = %s WHERE user_id IN (%s)%s`, newP, inClause, tClause)
|
||||
// Uses INSERT-style arg layout (newUserID at N+1, tenant at N+2).
|
||||
repoint := fmt.Sprintf(`UPDATE memory_chunks SET user_id = %s WHERE user_id IN (%s)%s`, newP, inClause, tClauseIns)
|
||||
if _, err := s.db.ExecContext(ctx, repoint, baseArgs...); err != nil {
|
||||
slog.Warn("merge.migrate_chunks", "error", err)
|
||||
}
|
||||
|
||||
@@ -13,6 +13,11 @@ interface UserPickerComboboxProps {
|
||||
/** Filter by source: "contact" | "tenant_user" | undefined (both).
|
||||
* Use "tenant_user" for merge dialogs and tenant user pickers. */
|
||||
source?: "contact" | "tenant_user";
|
||||
/** Committed value shape. "user_id" (default) returns the human-facing user_id
|
||||
* string; "uuid" returns the tenant_user primary key UUID and is only useful
|
||||
* when the consumer forwards the value to a backend expecting a tenant_user
|
||||
* foreign key (e.g. contact merge's `tenant_user_id`). Requires `source="tenant_user"`. */
|
||||
valueMode?: "user_id" | "uuid";
|
||||
/** Allow typing custom values not in the list. Default true. */
|
||||
allowCustom?: boolean;
|
||||
/** Render dropdown into a portal container (useful inside dialogs). */
|
||||
@@ -38,10 +43,11 @@ export function UserPickerCombobox({
|
||||
className,
|
||||
peerKind,
|
||||
source,
|
||||
valueMode,
|
||||
allowCustom = true,
|
||||
portalContainer,
|
||||
}: UserPickerComboboxProps) {
|
||||
const { options } = useUserPicker(value, peerKind, source);
|
||||
const { options } = useUserPicker(value, peerKind, source, valueMode);
|
||||
|
||||
return (
|
||||
<Combobox
|
||||
|
||||
@@ -7,6 +7,8 @@ import type { ComboboxOption } from "@/components/ui/combobox";
|
||||
/** Unified search result from contacts + tenant_users. */
|
||||
export interface UserPickerItem {
|
||||
id: string;
|
||||
/** Tenant_user primary key (UUID). Only present when source === "tenant_user". */
|
||||
uuid?: string;
|
||||
display_name?: string;
|
||||
username?: string;
|
||||
source: "contact" | "tenant_user";
|
||||
@@ -25,8 +27,17 @@ export interface UserPickerItem {
|
||||
/**
|
||||
* @param source - Filter by source: "contact" | "tenant_user" | undefined (both).
|
||||
* Use "tenant_user" for merge dialog / add tenant user (contacts excluded).
|
||||
* @param valueMode - "user_id" (default) or "uuid". When "uuid", the combobox
|
||||
* commits the tenant_user primary key instead of the user_id string. Only
|
||||
* callers forwarding the value as a tenant_user foreign key (contact merge)
|
||||
* should opt in.
|
||||
*/
|
||||
export function useUserPicker(search: string, peerKind?: string, source?: "contact" | "tenant_user") {
|
||||
export function useUserPicker(
|
||||
search: string,
|
||||
peerKind?: string,
|
||||
source?: "contact" | "tenant_user",
|
||||
valueMode?: "user_id" | "uuid",
|
||||
) {
|
||||
const http = useHttp();
|
||||
const [debouncedSearch, setDebouncedSearch] = useState("");
|
||||
|
||||
@@ -51,7 +62,10 @@ export function useUserPicker(search: string, peerKind?: string, source?: "conta
|
||||
|
||||
const results = data ?? [];
|
||||
|
||||
/** Format results as ComboboxOptions with source badges. */
|
||||
/** Format results as ComboboxOptions with source badges.
|
||||
* Committed value = tenant_user UUID only when valueMode === "uuid" AND the
|
||||
* result is a tenant_user row. All other callers (allow/deny lists, MCP/CLI
|
||||
* credentials, add-tenant-user dialog) still receive the user_id string. */
|
||||
const options: ComboboxOption[] = useMemo(() =>
|
||||
results.map((r) => {
|
||||
const parts: string[] = [];
|
||||
@@ -61,9 +75,10 @@ export function useUserPicker(search: string, peerKind?: string, source?: "conta
|
||||
if (r.source === "contact" && r.channel_type) parts.push(`[${r.channel_type}]`);
|
||||
if (r.source === "tenant_user") parts.push("[tenant]");
|
||||
if (r.merged_tenant_user_id) parts.push(`→ ${r.merged_tenant_user_id}`);
|
||||
return { value: r.id, label: parts.join(" ") };
|
||||
const value = valueMode === "uuid" && r.source === "tenant_user" && r.uuid ? r.uuid : r.id;
|
||||
return { value, label: parts.join(" ") };
|
||||
}),
|
||||
[results],
|
||||
[results, valueMode],
|
||||
);
|
||||
|
||||
return { results, options, loading: isLoading };
|
||||
|
||||
@@ -117,6 +117,7 @@ export function MergeContactsDialog({
|
||||
onChange={setSelectedUserId}
|
||||
placeholder={t("merge.selectUser")}
|
||||
source="tenant_user"
|
||||
valueMode="uuid"
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
|
||||
Reference in new issue
Block a user