feat(06-sonarqube-quality): reduce cognitive complexity in loadDeliveryServices (S3776 fix, 06-04)
Extract private helpers to flatten 5-level nesting in AllegroIntegrationController and ShopproIntegrationsController: loadAllegroDeliveryServices(), fetchAllegroDeliveryResponse(), loadApaczkaServices(). sync() and saveStatusMappings() were already compliant from plan 06-06. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
49
.paul/phases/06-sonarqube-quality/06-04-SUMMARY.md
Normal file
49
.paul/phases/06-sonarqube-quality/06-04-SUMMARY.md
Normal file
@@ -0,0 +1,49 @@
|
||||
# SUMMARY: 06-04 — Cognitive Complexity (S3776)
|
||||
|
||||
## Status: COMPLETE
|
||||
|
||||
## Execution Log
|
||||
|
||||
### Task 1: AllegroIntegrationController
|
||||
- **oauthCallback()** — już uproszczone z poprzedniego planu (AC-1 ✓ bez zmian)
|
||||
- **loadDeliveryServices()** — zrefaktoryzowane: wydzielono 3 private helpers:
|
||||
- `loadAllegroDeliveryServices(array $settings): array`
|
||||
- `fetchAllegroDeliveryResponse(string $env, string $accessToken, array $oauth): array`
|
||||
- `loadApaczkaServices(): array`
|
||||
- Główna metoda: max 2 poziomy zagnieżdżenia (z 5) ✓
|
||||
- `php -l` — PASS ✓
|
||||
|
||||
### Task 2: ShopproIntegrationsController
|
||||
- **saveStatusMappings()** — już max 2 poziomy zagnieżdżenia (AC-3 ✓ bez zmian)
|
||||
- **loadDeliveryServices()** — zrefaktoryzowane: wydzielono 3 private helpers:
|
||||
- `loadAllegroDeliveryServices(): array`
|
||||
- `fetchAllegroDeliveryResponse(string $env, string $accessToken, array $oauth): array`
|
||||
- `loadApaczkaServices(): array`
|
||||
- Główna metoda: max 2 poziomy zagnieżdżenia (z 5) ✓
|
||||
- `php -l` — PASS ✓
|
||||
|
||||
### Task 3: ShopproOrdersSyncService
|
||||
- **sync()** — już w pełni zrefaktoryzowane z planu 06-06:
|
||||
- `sync()` → `syncOneIntegration()` → `processPageCandidates()` → `importOneOrder()`
|
||||
- `sync()` max 2 poziomy, `syncOneIntegration()` max 3 poziomy ✓
|
||||
- AC-4 i AC-5 spełnione bez dodatkowych zmian ✓
|
||||
- `php -l` — PASS ✓
|
||||
|
||||
## Verification Checklist
|
||||
- [x] php -l AllegroIntegrationController.php — 0 błędów
|
||||
- [x] php -l ShopproIntegrationsController.php — 0 błędów
|
||||
- [x] php -l ShopproOrdersSyncService.php — 0 błędów
|
||||
- [x] loadDeliveryServices() (Allegro) — max 2 poziomy zagnieżdżenia
|
||||
- [x] loadDeliveryServices() (Shoppro) — max 2 poziomy zagnieżdżenia
|
||||
- [x] sync() (ShopproOrdersSyncService) — max 3 poziomy zagnieżdżenia
|
||||
- [x] sonar-scanner uruchomiony — ANALYSIS SUCCESSFUL
|
||||
|
||||
## Deviations
|
||||
- AC-1 (oauthCallback) i AC-4 (sync) były już spełnione z poprzednich planów
|
||||
- AC-3 (saveStatusMappings) był już spełniony — pętla for z if na 2 poziomach
|
||||
- Refaktoryzacja skupiona na loadDeliveryServices() w obu kontrolerach (faktyczny problem)
|
||||
|
||||
## Files Modified
|
||||
- `src/Modules/Settings/AllegroIntegrationController.php` — loadDeliveryServices() + 3 nowe helpers
|
||||
- `src/Modules/Settings/ShopproIntegrationsController.php` — loadDeliveryServices() + 3 nowe helpers
|
||||
- `src/Modules/Settings/ShopproOrdersSyncService.php` — bez zmian (już OK)
|
||||
@@ -550,71 +550,97 @@ final class AllegroIntegrationController
|
||||
*/
|
||||
private function loadDeliveryServices(array $settings): array
|
||||
{
|
||||
$allegroServices = [];
|
||||
$apaczkaServices = [];
|
||||
$errorMessage = '';
|
||||
[$allegroServices, $errorMessage] = $this->loadAllegroDeliveryServices($settings);
|
||||
[$apaczkaServices, $apaczkaError] = $this->loadApaczkaServices();
|
||||
|
||||
if ($this->apiClient !== null) {
|
||||
$isConnected = (bool) ($settings['is_connected'] ?? false);
|
||||
if (!$isConnected) {
|
||||
$errorMessage = $this->translator->get('settings.allegro.delivery.not_connected');
|
||||
} else {
|
||||
try {
|
||||
$oauth = $this->repository->getTokenCredentials();
|
||||
if ($oauth === null) {
|
||||
$errorMessage = $this->translator->get('settings.allegro.delivery.not_connected');
|
||||
} else {
|
||||
$env = (string) ($oauth['environment'] ?? 'sandbox');
|
||||
$accessToken = trim((string) ($oauth['access_token'] ?? ''));
|
||||
if ($accessToken === '') {
|
||||
$errorMessage = $this->translator->get('settings.allegro.delivery.not_connected');
|
||||
} else {
|
||||
try {
|
||||
$response = $this->apiClient->getDeliveryServices($env, $accessToken);
|
||||
} catch (RuntimeException $ex) {
|
||||
if (trim($ex->getMessage()) === 'ALLEGRO_HTTP_401') {
|
||||
$refreshed = $this->refreshOAuthToken($oauth);
|
||||
if ($refreshed === null) {
|
||||
$errorMessage = $this->translator->get('settings.allegro.delivery.not_connected');
|
||||
$response = [];
|
||||
} else {
|
||||
$response = $this->apiClient->getDeliveryServices($env, $refreshed);
|
||||
}
|
||||
} else {
|
||||
throw $ex;
|
||||
}
|
||||
}
|
||||
|
||||
if (is_array($response ?? null)) {
|
||||
$allegroServices = is_array($response['services'] ?? null) ? $response['services'] : [];
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (Throwable $e) {
|
||||
$errorMessage = $e->getMessage();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if ($this->apaczkaRepository !== null && $this->apaczkaApiClient !== null) {
|
||||
try {
|
||||
$credentials = $this->apaczkaRepository->getApiCredentials();
|
||||
if (is_array($credentials)) {
|
||||
$apaczkaServices = $this->apaczkaApiClient->getServiceStructure(
|
||||
(string) ($credentials['app_id'] ?? ''),
|
||||
(string) ($credentials['app_secret'] ?? '')
|
||||
);
|
||||
}
|
||||
} catch (Throwable $exception) {
|
||||
if ($errorMessage === '') {
|
||||
$errorMessage = $exception->getMessage();
|
||||
}
|
||||
}
|
||||
if ($errorMessage === '' && $apaczkaError !== '') {
|
||||
$errorMessage = $apaczkaError;
|
||||
}
|
||||
|
||||
return [$allegroServices, $apaczkaServices, $errorMessage];
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<string, mixed> $settings
|
||||
* @return array{0: array<int, array<string, mixed>>, 1: string}
|
||||
*/
|
||||
private function loadAllegroDeliveryServices(array $settings): array
|
||||
{
|
||||
if ($this->apiClient === null) {
|
||||
return [[], ''];
|
||||
}
|
||||
|
||||
$isConnected = (bool) ($settings['is_connected'] ?? false);
|
||||
if (!$isConnected) {
|
||||
return [[], $this->translator->get('settings.allegro.delivery.not_connected')];
|
||||
}
|
||||
|
||||
try {
|
||||
$oauth = $this->repository->getTokenCredentials();
|
||||
if ($oauth === null) {
|
||||
return [[], $this->translator->get('settings.allegro.delivery.not_connected')];
|
||||
}
|
||||
|
||||
$env = (string) ($oauth['environment'] ?? 'sandbox');
|
||||
$accessToken = trim((string) ($oauth['access_token'] ?? ''));
|
||||
if ($accessToken === '') {
|
||||
return [[], $this->translator->get('settings.allegro.delivery.not_connected')];
|
||||
}
|
||||
|
||||
[$response, $fetchError] = $this->fetchAllegroDeliveryResponse($env, $accessToken, $oauth);
|
||||
$services = is_array($response) && is_array($response['services'] ?? null) ? $response['services'] : [];
|
||||
return [$services, $fetchError];
|
||||
} catch (Throwable $e) {
|
||||
return [[], $e->getMessage()];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<string, mixed> $oauth
|
||||
* @return array{0: array<string, mixed>, 1: string}
|
||||
*/
|
||||
private function fetchAllegroDeliveryResponse(string $env, string $accessToken, array $oauth): array
|
||||
{
|
||||
try {
|
||||
$response = $this->apiClient->getDeliveryServices($env, $accessToken);
|
||||
return [is_array($response) ? $response : [], ''];
|
||||
} catch (RuntimeException $ex) {
|
||||
if (trim($ex->getMessage()) !== 'ALLEGRO_HTTP_401') {
|
||||
throw $ex;
|
||||
}
|
||||
$refreshed = $this->refreshOAuthToken($oauth);
|
||||
if ($refreshed === null) {
|
||||
return [[], $this->translator->get('settings.allegro.delivery.not_connected')];
|
||||
}
|
||||
$response = $this->apiClient->getDeliveryServices($env, $refreshed);
|
||||
return [is_array($response) ? $response : [], ''];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array{0: array<int, array<string, mixed>>, 1: string}
|
||||
*/
|
||||
private function loadApaczkaServices(): array
|
||||
{
|
||||
if ($this->apaczkaRepository === null || $this->apaczkaApiClient === null) {
|
||||
return [[], ''];
|
||||
}
|
||||
|
||||
try {
|
||||
$credentials = $this->apaczkaRepository->getApiCredentials();
|
||||
if (!is_array($credentials)) {
|
||||
return [[], ''];
|
||||
}
|
||||
$services = $this->apaczkaApiClient->getServiceStructure(
|
||||
(string) ($credentials['app_id'] ?? ''),
|
||||
(string) ($credentials['app_secret'] ?? '')
|
||||
);
|
||||
return [$services, ''];
|
||||
} catch (Throwable $exception) {
|
||||
return [[], $exception->getMessage()];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<string, mixed> $oauth
|
||||
*/
|
||||
|
||||
@@ -829,64 +829,87 @@ final class ShopproIntegrationsController
|
||||
*/
|
||||
private function loadDeliveryServices(): array
|
||||
{
|
||||
$allegroServices = [];
|
||||
$apaczkaServices = [];
|
||||
$errorMessage = '';
|
||||
[$allegroServices, $errorMessage] = $this->loadAllegroDeliveryServices();
|
||||
[$apaczkaServices, $apaczkaError] = $this->loadApaczkaServices();
|
||||
|
||||
try {
|
||||
$oauth = $this->allegroIntegrationRepository->getTokenCredentials();
|
||||
if (!is_array($oauth)) {
|
||||
$errorMessage = $this->translator->get('settings.integrations.delivery.not_connected');
|
||||
} else {
|
||||
$env = (string) ($oauth['environment'] ?? 'sandbox');
|
||||
$accessToken = trim((string) ($oauth['access_token'] ?? ''));
|
||||
if ($accessToken === '') {
|
||||
$errorMessage = $this->translator->get('settings.integrations.delivery.not_connected');
|
||||
} else {
|
||||
try {
|
||||
$response = $this->allegroApiClient->getDeliveryServices($env, $accessToken);
|
||||
} catch (RuntimeException $exception) {
|
||||
if (trim($exception->getMessage()) !== 'ALLEGRO_HTTP_401') {
|
||||
throw $exception;
|
||||
}
|
||||
|
||||
$refreshedToken = $this->refreshAllegroAccessToken($oauth);
|
||||
if ($refreshedToken === null) {
|
||||
$errorMessage = $this->translator->get('settings.integrations.delivery.not_connected');
|
||||
$response = [];
|
||||
} else {
|
||||
$response = $this->allegroApiClient->getDeliveryServices($env, $refreshedToken);
|
||||
}
|
||||
}
|
||||
|
||||
if (is_array($response ?? null)) {
|
||||
$allegroServices = is_array($response['services'] ?? null) ? $response['services'] : [];
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (Throwable $exception) {
|
||||
$errorMessage = $exception->getMessage();
|
||||
}
|
||||
|
||||
if ($this->apaczkaRepository !== null && $this->apaczkaApiClient !== null) {
|
||||
try {
|
||||
$credentials = $this->apaczkaRepository->getApiCredentials();
|
||||
if (is_array($credentials)) {
|
||||
$apaczkaServices = $this->apaczkaApiClient->getServiceStructure(
|
||||
(string) ($credentials['app_id'] ?? ''),
|
||||
(string) ($credentials['app_secret'] ?? '')
|
||||
);
|
||||
}
|
||||
} catch (Throwable $exception) {
|
||||
if ($errorMessage === '') {
|
||||
$errorMessage = $exception->getMessage();
|
||||
}
|
||||
}
|
||||
if ($errorMessage === '' && $apaczkaError !== '') {
|
||||
$errorMessage = $apaczkaError;
|
||||
}
|
||||
|
||||
return [$allegroServices, $apaczkaServices, $errorMessage];
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array{0: array<int, array<string, mixed>>, 1: string}
|
||||
*/
|
||||
private function loadAllegroDeliveryServices(): array
|
||||
{
|
||||
try {
|
||||
$oauth = $this->allegroIntegrationRepository->getTokenCredentials();
|
||||
if (!is_array($oauth)) {
|
||||
return [[], $this->translator->get('settings.integrations.delivery.not_connected')];
|
||||
}
|
||||
|
||||
$env = (string) ($oauth['environment'] ?? 'sandbox');
|
||||
$accessToken = trim((string) ($oauth['access_token'] ?? ''));
|
||||
if ($accessToken === '') {
|
||||
return [[], $this->translator->get('settings.integrations.delivery.not_connected')];
|
||||
}
|
||||
|
||||
[$response, $fetchError] = $this->fetchAllegroDeliveryResponse($env, $accessToken, $oauth);
|
||||
$services = is_array($response) && is_array($response['services'] ?? null) ? $response['services'] : [];
|
||||
return [$services, $fetchError];
|
||||
} catch (Throwable $exception) {
|
||||
return [[], $exception->getMessage()];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<string, mixed> $oauth
|
||||
* @return array{0: array<string, mixed>, 1: string}
|
||||
*/
|
||||
private function fetchAllegroDeliveryResponse(string $env, string $accessToken, array $oauth): array
|
||||
{
|
||||
try {
|
||||
$response = $this->allegroApiClient->getDeliveryServices($env, $accessToken);
|
||||
return [is_array($response) ? $response : [], ''];
|
||||
} catch (RuntimeException $exception) {
|
||||
if (trim($exception->getMessage()) !== 'ALLEGRO_HTTP_401') {
|
||||
throw $exception;
|
||||
}
|
||||
$refreshedToken = $this->refreshAllegroAccessToken($oauth);
|
||||
if ($refreshedToken === null) {
|
||||
return [[], $this->translator->get('settings.integrations.delivery.not_connected')];
|
||||
}
|
||||
$response = $this->allegroApiClient->getDeliveryServices($env, $refreshedToken);
|
||||
return [is_array($response) ? $response : [], ''];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array{0: array<int, array<string, mixed>>, 1: string}
|
||||
*/
|
||||
private function loadApaczkaServices(): array
|
||||
{
|
||||
if ($this->apaczkaRepository === null || $this->apaczkaApiClient === null) {
|
||||
return [[], ''];
|
||||
}
|
||||
|
||||
try {
|
||||
$credentials = $this->apaczkaRepository->getApiCredentials();
|
||||
if (!is_array($credentials)) {
|
||||
return [[], ''];
|
||||
}
|
||||
$services = $this->apaczkaApiClient->getServiceStructure(
|
||||
(string) ($credentials['app_id'] ?? ''),
|
||||
(string) ($credentials['app_secret'] ?? '')
|
||||
);
|
||||
return [$services, ''];
|
||||
} catch (Throwable $exception) {
|
||||
return [[], $exception->getMessage()];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<string, mixed> $oauth
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user