From d7c996de0ee689ad5d0005b738c150dd141ffd0e Mon Sep 17 00:00:00 2001 From: marsal Date: Wed, 8 Jul 2026 15:33:10 +0800 Subject: [PATCH] fix(channel): address Task 1 spec review issues for business-field preservation - Strengthen WeComSyncTest personType assertion by seeding a value (3) that differs from what resolvePersonType(user001/INTERNAL) would assign (1), so the test actually detects an overwrite. - Remove redundant personMapper/conversationMapper selectById calls in upsertContact and upsertConversation; preserve original business fields by reading them from the already-loaded entity before mutation. - Delete the manual regression shell script; the unit tests are the primary verification and the script required manual DB edits and only covered conversation fields. --- .../service/ChannelAccountService.java | 27 ++-- .../mci/channel/controller/WeComSyncTest.java | 5 +- .../wecom-sync-protect-business-fields.sh | 140 ------------------ 3 files changed, 17 insertions(+), 155 deletions(-) delete mode 100644 scripts/regression/wecom-sync-protect-business-fields.sh diff --git a/backend/mci-server/src/main/java/com/sino/mci/channel/service/ChannelAccountService.java b/backend/mci-server/src/main/java/com/sino/mci/channel/service/ChannelAccountService.java index 66276ed..488195e 100644 --- a/backend/mci-server/src/main/java/com/sino/mci/channel/service/ChannelAccountService.java +++ b/backend/mci-server/src/main/java/com/sino/mci/channel/service/ChannelAccountService.java @@ -232,14 +232,14 @@ public class ChannelAccountService { Person person; if (identity != null) { person = personMapper.selectById(identity.getPersonId()); + Long originalInstitutionId = person.getInstitutionId(); + Long originalSupplierId = person.getSupplierId(); + Integer originalPersonType = person.getPersonType(); person.setPersonName(contact.getName()); person.setPhone(contact.getMobile()); - Person existing = personMapper.selectById(person.getId()); - if (existing != null) { - person.setInstitutionId(existing.getInstitutionId()); - person.setSupplierId(existing.getSupplierId()); - person.setPersonType(existing.getPersonType()); - } + person.setInstitutionId(originalInstitutionId); + person.setSupplierId(originalSupplierId); + person.setPersonType(originalPersonType); personMapper.updateById(person); identity.setChannelUserName(contact.getName()); @@ -276,16 +276,17 @@ public class ChannelAccountService { .eq(Conversation::getChannelConversationId, conv.getChatId())); if (conversation != null) { + Long originalInstitutionId = conversation.getInstitutionId(); + Long originalSupplierId = conversation.getSupplierId(); + Long originalContractId = conversation.getContractId(); + String originalChannelCode = conversation.getChannelCode(); conversation.setConversationName(conv.getChatName()); conversation.setOwnerAccountId(account.getId()); conversation.setSubjectId(subjectId); - Conversation existing = conversationMapper.selectById(conversation.getId()); - if (existing != null) { - conversation.setInstitutionId(existing.getInstitutionId()); - conversation.setSupplierId(existing.getSupplierId()); - conversation.setContractId(existing.getContractId()); - conversation.setChannelCode(existing.getChannelCode()); - } + conversation.setInstitutionId(originalInstitutionId); + conversation.setSupplierId(originalSupplierId); + conversation.setContractId(originalContractId); + conversation.setChannelCode(originalChannelCode); conversationMapper.updateById(conversation); return conversation; } diff --git a/backend/mci-server/src/test/java/com/sino/mci/channel/controller/WeComSyncTest.java b/backend/mci-server/src/test/java/com/sino/mci/channel/controller/WeComSyncTest.java index 3846943..1cfe7db 100644 --- a/backend/mci-server/src/test/java/com/sino/mci/channel/controller/WeComSyncTest.java +++ b/backend/mci-server/src/test/java/com/sino/mci/channel/controller/WeComSyncTest.java @@ -189,7 +189,8 @@ class WeComSyncTest { person.setPersonCode("user001"); person.setPersonName("原始姓名"); person.setPhone("13800138001"); - person.setPersonType(1); + // Seed with a personType that differs from what resolvePersonType(user001/INTERNAL) would assign (1). + person.setPersonType(3); person.setInstitutionId(100L); person.setSupplierId(200L); person.setStatus(1); @@ -217,7 +218,7 @@ class WeComSyncTest { assertThat(updated.getPersonName()).isEqualTo("张三"); assertThat(updated.getInstitutionId()).isEqualTo(100L); assertThat(updated.getSupplierId()).isEqualTo(200L); - assertThat(updated.getPersonType()).isEqualTo(1); + assertThat(updated.getPersonType()).isEqualTo(3); } @Test diff --git a/scripts/regression/wecom-sync-protect-business-fields.sh b/scripts/regression/wecom-sync-protect-business-fields.sh deleted file mode 100644 index 308748b..0000000 --- a/scripts/regression/wecom-sync-protect-business-fields.sh +++ /dev/null @@ -1,140 +0,0 @@ -#!/bin/bash -set -e - -# Regression script for Task 1: business fields must survive WeCom sync. -# -# Pre-requisites: -# - mci-server is running (default http://localhost/msg-platform) -# - WeCom sync client is mocked or the real client returns a conversation -# whose channelConversationId matches the one created below. -# - The conversation API currently does NOT expose contractId/institutionId/ -# supplierId/channelCode, so after creating the conversation via API you -# must set contract_id = 100 directly in the DB (or extend the DTO). -# - jq and curl are installed. - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -cd "$SCRIPT_DIR/../.." - -BASE_URL="${BASE_URL:-http://localhost/msg-platform}" - -command -v curl >/dev/null 2>&1 || { echo "curl is required but not installed."; exit 1; } -command -v jq >/dev/null 2>&1 || { echo "jq is required but not installed."; exit 1; } - -TENANT_CODE="wecom-boundary-$(date +%s)" - -make_request() { - local method=$1 - local url=$2 - local payload=$3 - shift 3 - local headers=("$@") - - if [ "$method" = "GET" ]; then - curl -s -S -X GET "${headers[@]}" "$url" - elif [ -n "$payload" ]; then - curl -s -S -X "$method" "${headers[@]}" -H "Content-Type: application/json" -d "$payload" "$url" - else - curl -s -S -X "$method" "${headers[@]}" -H "Content-Type: application/json" "$url" - fi -} - -assert_code_zero() { - local name=$1 - local response=$2 - local code - code=$(echo "$response" | jq -r '.code // .errcode // empty') - - if [ "$code" = "0" ]; then - echo "[PASS] $name" - return 0 - else - echo "[FAIL] $name (code=$code)" - echo " response: $response" - exit 1 - fi -} - -# 1. Create tenant -echo "==> 1. Create tenant" -TENANT_RESPONSE=$(make_request POST "$BASE_URL/api/v1/account/tenant" \ - "{\"tenantCode\":\"$TENANT_CODE\",\"tenantName\":\"WeCom Boundary Test\"}") -assert_code_zero "Create tenant" "$TENANT_RESPONSE" - -# 2. Create admin user and login -echo "==> 2. Create admin user and login" -make_request POST "$BASE_URL/api/v1/account/user" \ - '{"username":"admin","password":"admin123","realName":"Admin","roleCodes":["tenant_admin"]}' \ - -H "X-Tenant-Code: $TENANT_CODE" >/dev/null - -LOGIN_RESPONSE=$(make_request POST "$BASE_URL/api/v1/account/login" \ - '{"username":"admin","password":"admin123"}' \ - -H "X-Tenant-Code: $TENANT_CODE") -assert_code_zero "Login" "$LOGIN_RESPONSE" -TOKEN=$(echo "$LOGIN_RESPONSE" | jq -r '.data.token') -AUTH_HEADER="Authorization: Bearer $TOKEN" -COMMON_HEADERS=(-H "$AUTH_HEADER" -H "X-Tenant-Code: $TENANT_CODE") - -# 3. Create channel subject (corp credentials are required for sync) -echo "==> 3. Create channel subject" -SUBJECT_RESPONSE=$(make_request POST "$BASE_URL/api/v1/channel-subject" \ - '{"channelType":"wecom","subjectCode":"sino","subjectName":"Sino Boundary","corpId":"ww-boundary","corpSecret":"secret","agentId":"1000002"}' \ - "${COMMON_HEADERS[@]}") -assert_code_zero "Create channel subject" "$SUBJECT_RESPONSE" -SUBJECT_ID=$(echo "$SUBJECT_RESPONSE" | jq -r '.data.id') - -# 4. Create channel account -echo "==> 4. Create channel account" -ACCOUNT_RESPONSE=$(make_request POST "$BASE_URL/api/v1/channel-account" \ - "{\"accountType\":1,\"channelType\":\"wecom\",\"accountId\":\"boundary-account\",\"accountName\":\"Boundary Account\",\"subjectId\":$SUBJECT_ID}" \ - "${COMMON_HEADERS[@]}") -assert_code_zero "Create channel account" "$ACCOUNT_RESPONSE" -ACCOUNT_ID=$(echo "$ACCOUNT_RESPONSE" | jq -r '.data.id') - -# 5. Create conversation via API -echo "==> 5. Create conversation" -CONVERSATION_RESPONSE=$(make_request POST "$BASE_URL/api/v1/conversation" \ - '{"conversationType":2,"channelType":"wecom","channelConversationId":"chat-boundary-001","conversationName":"Boundary Group"}' \ - "${COMMON_HEADERS[@]}") -assert_code_zero "Create conversation" "$CONVERSATION_RESPONSE" -CONVERSATION_ID=$(echo "$CONVERSATION_RESPONSE" | jq -r '.data.id') - -# 6. Set business fields in DB (contract_id=100). -# The public API does not expose these fields yet, so this step is performed -# directly in the database. Example: -# -# UPDATE mci_conversation -# SET contract_id = 100, -# institution_id = 1000, -# supplier_id = 2000, -# channel_code = 'ORIGINAL' -# WHERE id = ; -# -echo "==> 6. Set contractId=100 in DB for conversation $CONVERSATION_ID" -echo " (execute the SQL shown in the script comments, then press any key)" -read -r -n 1 -s || true - -# 7. Call sync-conversations -echo "==> 7. Call sync-conversations" -SYNC_RESPONSE=$(make_request POST "$BASE_URL/api/v1/channel-account/$ACCOUNT_ID/sync-conversations" \ - '' "${COMMON_HEADERS[@]}") -assert_code_zero "Sync conversations" "$SYNC_RESPONSE" - -# 8. Query conversation and assert contractId is still 100 -echo "==> 8. Query conversation and assert business fields" -QUERY_RESPONSE=$(make_request GET "$BASE_URL/api/v1/conversation/$CONVERSATION_ID" \ - '' "${COMMON_HEADERS[@]}") -assert_code_zero "Query conversation" "$QUERY_RESPONSE" - -CONTRACT_ID=$(echo "$QUERY_RESPONSE" | jq -r '.data.contractId // empty') -if [ "$CONTRACT_ID" = "100" ]; then - echo "[PASS] contractId preserved (100) after sync" -else - echo "[FAIL] contractId was overwritten or not set (got '$CONTRACT_ID')" - echo " response: $QUERY_RESPONSE" - exit 1 -fi - -echo "" -echo "=========================" -echo "All regression checks passed." -echo "========================="