Fix customer registration duplicate recovery

This commit is contained in:
Jeppe Bundgaard
2026-06-11 12:04:56 +02:00
parent bdb1a0074b
commit d06c78119b
3 changed files with 241 additions and 56 deletions
+39 -32
View File
@@ -127,40 +127,47 @@ class users_o extends db
private function importCustomerFromExternalSource(int $customer_number): object|bool
{
global $db;
// Get the customer data from the external source
$economic = new economicCustomers();
$customer_data = $economic->getCustomerId($customer_number);
// DEBUG: Return the customer data
// Check if the customer exists
if ($customer_data) {
// Avoid SQL injection
$customer_number = $db->escape_string($customer_data->customerNumber);
// Double check if the customer exists
$sql = "SELECT * FROM $this->table WHERE customer_number = '$customer_number'";
$result = $db->query($sql);
if ($result->num_rows > 0) {
$this->id = $result->fetch_assoc()['id'];
$this->getObjectProperties();
} else {
// Import the customer
$this->add($customer_number, '', 0);
// Nullify the password
$this->password->nullify();
// If the customer has an email address, save it
if (isset($customer_data->email)) {
$this->email->set($customer_data->email);
}
// If the customer has a name, save it as the display name
if (isset($customer_data->name)) {
$this->display_name->set($customer_data->name);
}
}
return $this->importCustomerFromEconomicCustomerData($customer_data);
}
// Else return false
return false;
}
public function importCustomerFromEconomicCustomerData(object $customer_data): users_o|bool
{
global $db;
if (!isset($customer_data->customerNumber) || !is_numeric($customer_data->customerNumber)) {
return false;
}
$customer_number = $db->escape_string((string)$customer_data->customerNumber);
$sql = "SELECT * FROM $this->table WHERE customer_number = '$customer_number'";
$result = $db->query($sql);
if ($result->num_rows > 0) {
$this->id = (int)$result->fetch_assoc()['id'];
$this->getObjectProperties();
return $this;
}
$this->add($customer_number, '', 0);
$this->password->nullify();
if (isset($customer_data->email)) {
$this->email->set($customer_data->email);
}
if (isset($customer_data->name)) {
$this->display_name->set($customer_data->name);
}
return $this;
}
/**
* @throws Exception
*/
@@ -258,7 +265,7 @@ class users_o extends db
* @param int|null $user_id The user id to add the attribute to
* @throws Exception If the user is not selected, and the user_id is null
*/
public function addAttribute(string $attribute, int $user_id = null): void
public function addAttribute(string $attribute, ?int $user_id = null): void
{
global $db;
if ($user_id === null) {
@@ -272,7 +279,7 @@ class users_o extends db
$db->query($sql);
}
public function deleteAttribute(string $attribute, int $user_id = null): void
public function deleteAttribute(string $attribute, ?int $user_id = null): void
{
global $db;
if ($user_id === null) {
@@ -304,7 +311,7 @@ class users_o extends db
$db->query($sql);
}
public function doesUserHaveAttribute(string $attribute, int $user_id = null): bool
public function doesUserHaveAttribute(string $attribute, ?int $user_id = null): bool
{
global $db;
if ($user_id === null) {
@@ -518,7 +525,7 @@ class users_o extends db
return customer_name_cache_payload_builder::build($cached_name, $fallback_name);
}
public function getCustomerEcocomicData(int $customer_number = null): users_o
public function getCustomerEcocomicData(?int $customer_number = null): users_o
{
// Check if the customer number is set
if (!isset($this->customer_number) && $customer_number === null) {
@@ -717,7 +724,7 @@ class users_o extends db
$this->permissions = $perms;
}
public function getUserAttributes(int $user_id = null): array
public function getUserAttributes(?int $user_id = null): array
{
global $db;
if ($user_id === null) {
@@ -1106,7 +1113,7 @@ class users_o extends db
* Set the password for the user
* @throws Exception If the user is not selected
*/
public function setPassword(string $password = null): void
public function setPassword(?string $password = null): void
{
self::requireSelected();
global $db;
+119 -23
View File
@@ -409,15 +409,7 @@ class authRoute
* Check if the cvr already exists
*/
$economic = new economic();
$economic_response = ($economic->customers->customers->search([
'corporateIdentificationNumber' => (string)$cvr,
], [
'skipPages' => 0,
'pageSize' => 1, // Since the limit is 1000, we need to set the page size to 1000.
])->collection);
if (!is_array($economic_response)) {
$economic_response = [];
}
$economic_response = $this->searchEconomicCustomersByCvr($economic, (string)$cvr);
$localUserExists = $this->localCustomerNumberExists($companyPhone);
$matchingEconomicCustomer = $this->findEconomicCustomerByNumber($economic_response, $companyPhone);
@@ -455,15 +447,37 @@ class authRoute
// Get the CVR company information used for the e-conomic customer payload.
$companyInformation = (new virkdata())->getCompanyInformation($cvr, '', []);
$name = (string)($companyInformation->name ?? '');
$result = $economic->createCustomer(
(int)$companyPhone,
$name,
(int)$cvr,
(string)$invoiceEmail,
(int)$companyPhone,
(int)$contactPhone,
$companyInformation,
);
try {
$result = $economic->createCustomer(
(int)$companyPhone,
$name,
(int)$cvr,
(string)$invoiceEmail,
(int)$companyPhone,
(int)$contactPhone,
$companyInformation,
);
} catch (Exception $exception) {
$recoveredCustomer = $this->recoverRegistrationAfterCreateFailure(
$economic,
(string)$cvr,
$companyPhone,
(string)$invoiceEmail,
$exception
);
if ($recoveredCustomer !== null) {
$response->success($recoveredCustomer, 200);
}
$this->logRegisterCvrIssue('AUTH_REGISTER_CVR_CREATE_FAILED', [
'phase' => 'create',
'cvr' => (string)$cvr,
'requestedCustomerNumber' => $companyPhone,
'message' => $exception->getMessage(),
]);
$response->error('Failed to create customer in e-conomic.', 502);
}
if (!isset($result->customerNumber) || !is_numeric($result->customerNumber)) {
$this->logRegisterCvrIssue('AUTH_REGISTER_CVR_INVALID_CREATE_RESPONSE', [
@@ -495,7 +509,7 @@ class authRoute
);
}
$this->bootstrapLocalCustomerOrFail($companyPhone);
$this->bootstrapLocalCustomerOrFail($companyPhone, $result);
$this->sendRegistrationWelcomeEmails($companyPhone, (string)$invoiceEmail);
$response->success($result, 201);
});
@@ -758,6 +772,61 @@ class authRoute
return count($rows) > 0;
}
private function searchEconomicCustomersByCvr(economic $economic, string $cvr): array
{
$economic_response = ($economic->customers->customers->search([
'corporateIdentificationNumber' => $cvr,
], [
'skipPages' => 0,
'pageSize' => 1,
])->collection);
return is_array($economic_response) ? $economic_response : [];
}
private function recoverRegistrationAfterCreateFailure(
economic $economic,
string $cvr,
int $customerNumber,
string $invoiceEmail,
Exception $exception
): ?object {
if (!$this->isRecoverableEconomicDuplicateError($exception)) {
return null;
}
try {
$economic_response = $this->searchEconomicCustomersByCvr($economic, $cvr);
} catch (Exception $searchException) {
$this->logRegisterCvrIssue('AUTH_REGISTER_CVR_CREATE_RECOVERY_SEARCH_FAILED', [
'phase' => 'create_recovery',
'cvr' => $cvr,
'requestedCustomerNumber' => $customerNumber,
'message' => $searchException->getMessage(),
]);
return null;
}
$matchingEconomicCustomer = $this->findEconomicCustomerByNumber($economic_response, $customerNumber);
if ($matchingEconomicCustomer === null || $this->localCustomerNumberExists($customerNumber)) {
return null;
}
$this->bootstrapLocalCustomerOrFail($customerNumber, $matchingEconomicCustomer);
$this->sendRegistrationWelcomeEmails($customerNumber, $invoiceEmail);
return $matchingEconomicCustomer;
}
private function isRecoverableEconomicDuplicateError(Exception $exception): bool
{
$message = strtolower($exception->getMessage());
return str_contains($message, 'already exists')
|| str_contains($message, 'already exist')
|| str_contains($message, 'duplicate');
}
private function findEconomicCustomerByNumber(array $customers, int $customerNumber): ?object
{
foreach ($customers as $customer) {
@@ -785,19 +854,46 @@ class authRoute
/**
* @throws Exception
*/
private function bootstrapLocalCustomerOrFail(int $customerNumber): users_o
private function bootstrapLocalCustomerOrFail(int $customerNumber, ?object $economicCustomer = null): users_o
{
global $response;
$customer = (new users_o())->getUserByCustomerNumber($customerNumber);
if (method_exists($customer, 'exists') && $customer->exists()) {
return $customer;
$customer = new users_o();
try {
$customer = $customer->getUserByCustomerNumber($customerNumber);
if (method_exists($customer, 'exists') && $customer->exists()) {
return $customer;
}
} catch (Exception $exception) {
$this->logRegisterCvrIssue('AUTH_REGISTER_CVR_LOCAL_BOOTSTRAP_LOOKUP_FAILED', [
'customerNumber' => $customerNumber,
'message' => $exception->getMessage(),
]);
}
if (
$economicCustomer !== null
&& $this->extractEconomicCustomerNumber($economicCustomer) === $customerNumber
&& method_exists($customer, 'importCustomerFromEconomicCustomerData')
) {
try {
$importedCustomer = $customer->importCustomerFromEconomicCustomerData($economicCustomer);
if (is_object($importedCustomer) && method_exists($importedCustomer, 'exists') && $importedCustomer->exists()) {
return $importedCustomer;
}
} catch (Exception $exception) {
$this->logRegisterCvrIssue('AUTH_REGISTER_CVR_LOCAL_SNAPSHOT_BOOTSTRAP_FAILED', [
'customerNumber' => $customerNumber,
'message' => $exception->getMessage(),
]);
}
}
$this->logRegisterCvrIssue('AUTH_REGISTER_CVR_LOCAL_BOOTSTRAP_FAILED', [
'customerNumber' => $customerNumber,
]);
$response->error('Customer was created in e-conomic but could not be imported locally.', 500);
throw new Exception('Customer was created in e-conomic but could not be imported locally.');
}
/**
@@ -69,6 +69,8 @@ namespace classes {
{
public static array $mock_collection = [];
public static ?object $mock_create_response = null;
public static ?\RuntimeException $mock_create_exception = null;
public static array $mock_collection_after_create_exception = [];
public static array $search_calls = [];
public static array $create_calls = [];
@@ -78,6 +80,8 @@ namespace classes {
{
self::$mock_collection = [];
self::$mock_create_response = null;
self::$mock_create_exception = null;
self::$mock_collection_after_create_exception = [];
self::$search_calls = [];
self::$create_calls = [];
}
@@ -113,6 +117,11 @@ namespace classes {
'company_information' => $companyInformation,
];
if (self::$mock_create_exception !== null) {
self::$mock_collection = self::$mock_collection_after_create_exception;
throw self::$mock_create_exception;
}
$response = self::$mock_create_response ?? (object)[
'customerNumber' => (int)$number,
];
@@ -214,6 +223,7 @@ namespace objects {
{
public static array $mock_existing_customer_numbers = [];
public static array $mock_importable_customer_numbers = [];
public static bool $mock_external_lookup_enabled = true;
public static array $interaction_log = [];
public int $id = 0;
@@ -223,6 +233,7 @@ namespace objects {
{
self::$mock_existing_customer_numbers = [];
self::$mock_importable_customer_numbers = [];
self::$mock_external_lookup_enabled = true;
self::$interaction_log = [];
}
@@ -242,7 +253,8 @@ namespace objects {
self::$interaction_log[] = 'bootstrap:' . $customerNumber;
$existsLocally = in_array($customerNumber, self::$mock_existing_customer_numbers, true);
$canImport = in_array($customerNumber, self::$mock_importable_customer_numbers, true);
$canImport = self::$mock_external_lookup_enabled
&& in_array($customerNumber, self::$mock_importable_customer_numbers, true);
if ($existsLocally || $canImport) {
$this->id = $customerNumber;
@@ -257,6 +269,22 @@ namespace objects {
return $this;
}
public function importCustomerFromEconomicCustomerData(object $customerData): self|bool
{
$customerNumber = (int)($customerData->customerNumber ?? 0);
if ($customerNumber <= 0) {
return false;
}
self::$interaction_log[] = 'snapshot-import:' . $customerNumber;
$this->id = $customerNumber;
$this->exists = true;
self::$mock_existing_customer_numbers[] = $customerNumber;
self::$mock_existing_customer_numbers = array_values(array_unique(self::$mock_existing_customer_numbers));
return $this;
}
public function exists(): bool
{
return $this->exists;
@@ -509,6 +537,60 @@ namespace {
assert_true(\classes\slack::$customer_registration_notifications[0]['customer_number'] === 12345678, 'Fresh registration Slack notification must use the created customer number.');
},
],
[
'name' => 'Successful registration falls back to the create response when immediate import lookup misses',
'params' => array_merge($baseParams, ['contactPhone' => 87654320]),
'setup' => static function (): void {
\classes\economic::$mock_create_response = (object)[
'customerNumber' => 12345678,
'name' => 'Mock Company',
'email' => 'test@test.com',
];
\objects\users_o::$mock_external_lookup_enabled = false;
},
'expected_success' => (object)[
'customerNumber' => 12345678,
'name' => 'Mock Company',
'email' => 'test@test.com',
],
'expected_status' => 201,
'assert' => static function (): void {
assert_true(count(\classes\economic::$create_calls) === 1, 'Fresh registration must call create exactly once.');
assert_true(\objects\users_o::$interaction_log[0] === 'bootstrap:12345678', 'Fresh registration must try the standard local bootstrap first.');
assert_true(\objects\users_o::$interaction_log[1] === 'snapshot-import:12345678', 'Fresh registration must import from the create response when the immediate lookup misses.');
assert_true(count(\classes\email::$sent) === 2, 'Snapshot fallback registration must send two welcome emails.');
assert_true(count(\classes\email::$superuser_notifications) === 1, 'Snapshot fallback registration must notify opted-in superusers once.');
assert_true(count(\classes\slack::$customer_registration_notifications) === 1, 'Snapshot fallback registration must notify Slack once.');
},
],
[
'name' => 'Duplicate create response recovers a just-created e-conomic customer and sends notifications',
'params' => $baseParams,
'setup' => static function (): void {
\classes\economic::$mock_create_exception = new \RuntimeException('e-conomic request failed with HTTP 400: Customer already exists');
\classes\economic::$mock_collection_after_create_exception = [
(object)[
'customerNumber' => 12345678,
'name' => 'Recovered After Create',
'email' => 'test@test.com',
],
];
\objects\users_o::$mock_importable_customer_numbers = [12345678];
},
'expected_success' => (object)[
'customerNumber' => 12345678,
'name' => 'Recovered After Create',
'email' => 'test@test.com',
],
'expected_status' => 200,
'assert' => static function (): void {
assert_true(count(\classes\economic::$create_calls) === 1, 'Recovery must still record the attempted create call.');
assert_true(count(\classes\economic::$search_calls) === 2, 'Recovery must verify the duplicate by searching e-conomic again.');
assert_true(count(\classes\email::$sent) === 2, 'Duplicate create recovery must send two welcome emails.');
assert_true(count(\classes\email::$superuser_notifications) === 1, 'Duplicate create recovery must notify opted-in superusers once.');
assert_true(count(\classes\slack::$customer_registration_notifications) === 1, 'Duplicate create recovery must notify Slack once.');
},
],
[
'name' => 'Fresh create mismatch returns conflict without local bootstrap or email',
'params' => $baseParams,