Skip to content

Commit

Permalink
♻️ refactoring representation bundles (#439)
Browse files Browse the repository at this point in the history
* ♻️ refactoring - add logging and change error messages
* ♻️ add error logging and use fix error messages instead of exception message
* ♻️ uuid generation: use event behavior instead of service
  • Loading branch information
julianzimmermann committed Sep 22, 2023
1 parent 9f0c36c commit 26aadc2
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,36 @@ interface RepresentativeCompanyUserTradeFairRestApiConstants
*/
public const HTTP_CODE_VALIDATION_ERRORS = 422;

/**
* @var string
*/
public const HTTP_CODE_ADD_ERRORS = '1000';

/**
* @var string
*/
public const HTTP_CODE_UPDATE_ERRORS = '2000';

/**
* @var string
*/
public const HTTP_CODE_DELETE_ERRORS = '3000';

/**
* @var string
*/
public const HTTP_MESSAGE_ADD_ERROR = 'Could not create trade fair representation!';

/**
* @var string
*/
public const HTTP_MESSAGE_UPDATE_ERROR = 'Could not update trade fair representation!';

/**
* @var string
*/
public const HTTP_MESSAGE_DELETE_ERROR = 'Could not delete trade fair representation!';

/**
* @var string
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Business\Model;

use FondOfOryx\Shared\RepresentativeCompanyUserTradeFairRestApi\RepresentativeCompanyUserTradeFairRestApiConstants;
use FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Business\Model\Mapper\RestDataMapperInterface;
use FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Business\Validator\DurationValidatorInterface;
use FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Communication\Plugin\PermissionExtension\CanManageRepresentationOnTradeFairPermissionPlugin;
Expand All @@ -18,6 +19,7 @@
use Generated\Shared\Transfer\RestRepresentativeCompanyUserTradeFairAttributesTransfer;
use Generated\Shared\Transfer\RestRepresentativeCompanyUserTradeFairRequestTransfer;
use Generated\Shared\Transfer\RestRepresentativeCompanyUserTradeFairResponseTransfer;
use Psr\Log\LoggerInterface;
use Throwable;

class TradeFairRepresentationManager implements TradeFairRepresentationManagerInterface
Expand All @@ -42,6 +44,11 @@ class TradeFairRepresentationManager implements TradeFairRepresentationManagerIn
*/
protected RepresentativeCompanyUserTradeFairRestApiRepositoryInterface $repository;

/**
* @var \Psr\Log\LoggerInterface
*/
protected LoggerInterface $logger;

/**
* @var \FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Business\Model\Mapper\RestDataMapperInterface
*/
Expand All @@ -53,19 +60,22 @@ class TradeFairRepresentationManager implements TradeFairRepresentationManagerIn
* @param \FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Business\Validator\DurationValidatorInterface $durationValidator
* @param \FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Persistence\RepresentativeCompanyUserTradeFairRestApiRepositoryInterface $repository
* @param \FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Business\Model\Mapper\RestDataMapperInterface $restDataMapper
* @param \Psr\Log\LoggerInterface $logger
*/
public function __construct(
RepresentativeCompanyUserTradeFairRestApiToRepresentativeCompanyUserTradeFairFacadeInterface $representativeCompanyUserTradeFairFacade,
RepresentativeCompanyUserTradeFairRestApiToCompanyTypeFacadeInterface $companyTypeFacade,
DurationValidatorInterface $durationValidator,
RepresentativeCompanyUserTradeFairRestApiRepositoryInterface $repository,
RestDataMapperInterface $restDataMapper
RestDataMapperInterface $restDataMapper,
LoggerInterface $logger
) {
$this->representativeCompanyUserTradeFairFacade = $representativeCompanyUserTradeFairFacade;
$this->companyTypeFacade = $companyTypeFacade;
$this->durationValidator = $durationValidator;
$this->repository = $repository;
$this->restDataMapper = $restDataMapper;
$this->logger = $logger;
}

/**
Expand Down Expand Up @@ -99,7 +109,9 @@ public function addTradeFairRepresentation(

$response = $this->representativeCompanyUserTradeFairFacade->addRepresentativeCompanyUserTradeFair($representationTransfer);
} catch (Throwable $throwable) {
return $restRepresentativeCompanyUserTradeFairResponse->setError($this->createError($throwable->getMessage(), '500', 500));
$this->logger->error($throwable->getMessage(), $throwable->getTrace());

return $restRepresentativeCompanyUserTradeFairResponse->setError($this->createError(RepresentativeCompanyUserTradeFairRestApiConstants::HTTP_MESSAGE_ADD_ERROR, RepresentativeCompanyUserTradeFairRestApiConstants::HTTP_CODE_ADD_ERRORS));
}

return $restRepresentativeCompanyUserTradeFairResponse
Expand Down Expand Up @@ -138,12 +150,15 @@ public function updateTradeFairRepresentation(
}

$representationTransfer
->setName($restRepresentativeCompanyUserTradeFairAttributesTransfer->getTradeFairName())
->setActive($this->getStatus($representationTransfer, $restRepresentativeCompanyUserTradeFairAttributesTransfer))
->setEndAt($restRepresentativeCompanyUserTradeFairAttributesTransfer->getEndAt())
->setStartAt($restRepresentativeCompanyUserTradeFairAttributesTransfer->getStartAt());
$response = $this->representativeCompanyUserTradeFairFacade->updateRepresentativeCompanyUserTradeFair($representationTransfer);
} catch (Throwable $throwable) {
return $restResponse->setError($this->createError($throwable->getMessage(), '500', 500));
$this->logger->error($throwable->getMessage(), $throwable->getTrace());

return $restResponse->setError($this->createError(RepresentativeCompanyUserTradeFairRestApiConstants::HTTP_MESSAGE_UPDATE_ERROR, RepresentativeCompanyUserTradeFairRestApiConstants::HTTP_CODE_UPDATE_ERRORS));
}

return $restResponse
Expand All @@ -166,8 +181,9 @@ public function deleteTradeFairRepresentation(
$representation = $this->representativeCompanyUserTradeFairFacade->deleteRepresentativeCompanyUserTradeFair($attributes->getUuid());
$response = $this->restDataMapper->mapResponse($representation);
} catch (Throwable $throwable) {
//ToDo Handle/Log
return $restResponse->setError($this->createError($throwable->getMessage(), '500', 500));
$this->logger->error($throwable->getMessage(), $throwable->getTrace());

return $restResponse->setError($this->createError(RepresentativeCompanyUserTradeFairRestApiConstants::HTTP_MESSAGE_DELETE_ERROR, RepresentativeCompanyUserTradeFairRestApiConstants::HTTP_CODE_DELETE_ERRORS));
}

return $restResponse->setRepresentation($response)->setIsSuccessful(true);
Expand Down Expand Up @@ -228,13 +244,13 @@ protected function validate(
$companyTypeManufacturer->getIdCompanyType(),
)
) {
return $this->createError(RepresentativeCompanyUserTradeFairRestApiConfig::ERROR_MESSAGE_USER_IS_NOT_ALLOWED_TO_ADD_TRADE_FAIR_REPRESENTATION, '422');
return $this->createError(RepresentativeCompanyUserTradeFairRestApiConfig::ERROR_MESSAGE_USER_IS_NOT_ALLOWED_TO_ADD_TRADE_FAIR_REPRESENTATION, (string)RepresentativeCompanyUserTradeFairRestApiConstants::HTTP_CODE_VALIDATION_ERRORS, RepresentativeCompanyUserTradeFairRestApiConstants::HTTP_CODE_VALIDATION_ERRORS);
}

if (
!$this->durationValidator->validate($restRepresentativeCompanyUserTradeFairRequestTransfer->getAttributes())
) {
return $this->createError(RepresentativeCompanyUserTradeFairRestApiConfig::ERROR_MESSAGE_REPRESENTATION_DURATION_EXCEEDED, '422');
return $this->createError(RepresentativeCompanyUserTradeFairRestApiConfig::ERROR_MESSAGE_REPRESENTATION_DURATION_EXCEEDED, (string)RepresentativeCompanyUserTradeFairRestApiConstants::HTTP_CODE_VALIDATION_ERRORS, RepresentativeCompanyUserTradeFairRestApiConstants::HTTP_CODE_VALIDATION_ERRORS);
}

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Dependency\Facade\RepresentativeCompanyUserTradeFairRestApiToCompanyTypeFacadeInterface;
use FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Dependency\Facade\RepresentativeCompanyUserTradeFairRestApiToRepresentativeCompanyUserTradeFairFacadeInterface;
use FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\RepresentativeCompanyUserTradeFairRestApiDependencyProvider;
use Spryker\Shared\Log\LoggerTrait;
use Spryker\Zed\Kernel\Business\AbstractBusinessFactory;

/**
Expand All @@ -19,6 +20,8 @@
*/
class RepresentativeCompanyUserTradeFairRestApiBusinessFactory extends AbstractBusinessFactory
{
use LoggerTrait;

/**
* @return \FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Business\Model\TradeFairRepresentationManagerInterface
*/
Expand All @@ -30,6 +33,7 @@ public function createTradeFairRepresentationManager(): TradeFairRepresentationM
$this->createDurationValidator(),
$this->getRepository(),
$this->createRestDataMapper(),
$this->getLogger(),
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Generated\Shared\Transfer\RestRepresentativeCompanyUserTradeFairResponseTransfer;
use Generated\Shared\Transfer\RestRepresentativeCompanyUserTradeFairTransfer;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;

class TradeFairRepresentationManagerTest extends Unit
{
Expand Down Expand Up @@ -87,6 +88,11 @@ class TradeFairRepresentationManagerTest extends Unit
*/
protected RestDataMapperInterface|MockObject $restDataMapperMock;

/**
* @var \Psr\Log\LoggerInterface|\PHPUnit\Framework\MockObject\MockObject
*/
protected LoggerInterface|MockObject $loggerMock;

/**
* @var \FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Business\Model\TradeFairRepresentationManager
*/
Expand Down Expand Up @@ -134,6 +140,11 @@ protected function _before(): void
->disableOriginalConstructor()
->getMock();

$this->loggerMock = $this
->getMockBuilder(LoggerInterface::class)
->disableOriginalConstructor()
->getMock();

$this->restRepresentativeCompanyUserTradeFairRequestTransferMock = $this
->getMockBuilder(RestRepresentativeCompanyUserTradeFairRequestTransfer::class)
->disableOriginalConstructor()
Expand Down Expand Up @@ -170,6 +181,7 @@ protected function _before(): void
$this->durationValidatorMock,
$this->repositoryMock,
$this->restDataMapperMock,
$this->loggerMock,
);
}

Expand Down Expand Up @@ -316,6 +328,7 @@ public function testUpdateTradeFairRepresentation(): void
$date1 = '1970-01-01';
$date2 = '1970-01-05';
$companyTypeId = 1;
$name = 'name';
$uuid = 'xxxx-xxxxx-xxxx-xxxx';

$this->restRepresentativeCompanyUserTradeFairRequestTransferMock->expects(static::atLeastOnce())
Expand Down Expand Up @@ -389,11 +402,20 @@ public function testUpdateTradeFairRepresentation(): void
->method('getEndAt')
->willReturn($date2);

$this->restRepresentativeCompanyUserTradeFairAttributesTransferMock->expects(static::atLeastOnce())
->method('getTradeFairName')
->willReturn($name);

$this->representativeCompanyUserTradeFairTransferMock->expects(static::atLeastOnce())
->method('setEndAt')
->with($date2)
->willReturnSelf();

$this->representativeCompanyUserTradeFairTransferMock->expects(static::atLeastOnce())
->method('setName')
->with($name)
->willReturnSelf();

$this->representativeCompanyUserTradeFairTransferMock->expects(static::atLeastOnce())
->method('setStartAt')
->with($date2)
Expand Down Expand Up @@ -722,9 +744,8 @@ public function testUpdateTradeFairRepresentationWithStatusFalse(): void
$startAt2 = '1970-01-05';
$uuid = 'xxxx-xxxxx-xxxx-xxxx';
$customerReference = 'customer-reference';
$originatorId = 1;
$representationId = 2;
$companyTypeId = 1;
$name = 'name';

$this->restRepresentativeCompanyUserTradeFairRequestTransferMock->expects(static::atLeastOnce())
->method('getAttributes')
Expand Down Expand Up @@ -785,6 +806,10 @@ public function testUpdateTradeFairRepresentationWithStatusFalse(): void
->method('setActive')
->willReturnSelf();

$this->restRepresentativeCompanyUserTradeFairAttributesTransferMock->expects(static::atLeastOnce())
->method('getTradeFairName')
->willReturn($name);

$this->restRepresentativeCompanyUserTradeFairAttributesTransferMock->expects(static::atLeastOnce())
->method('getStartAt')
->willReturn($startAt1);
Expand All @@ -801,6 +826,10 @@ public function testUpdateTradeFairRepresentationWithStatusFalse(): void
->method('setStartAt')
->willReturnSelf();

$this->representativeCompanyUserTradeFairTransferMock->expects(static::atLeastOnce())
->method('setName')->with($name)
->willReturnSelf();

$this->representativeCompanyUserTradeFairFacadeMock->expects(static::atLeastOnce())
->method('updateRepresentativeCompanyUserTradeFair')
->with($this->representativeCompanyUserTradeFairTransferMock)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
use FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\RepresentativeCompanyUserTradeFairRestApiConfig;
use FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\RepresentativeCompanyUserTradeFairRestApiDependencyProvider;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Spryker\Shared\Log\Config\LoggerConfigInterface;
use Spryker\Zed\Kernel\Container;

class RepresentativeCompanyUserTradeFairRestApiBusinessFactoryTest extends Unit
Expand All @@ -35,6 +37,11 @@ class RepresentativeCompanyUserTradeFairRestApiBusinessFactoryTest extends Unit
*/
protected MockObject|RepresentativeCompanyUserTradeFairRestApiRepository $repositoryMock;

/**
* @var \PHPUnit\Framework\MockObject\MockObject|\Psr\Log\LoggerInterface
*/
protected MockObject|LoggerInterface $loggerMock;

/**
* @var \PHPUnit\Framework\MockObject\MockObject|\FondOfOryx\Zed\RepresentativeCompanyUserTradeFairRestApi\Dependency\Facade\RepresentativeCompanyUserTradeFairRestApiToRepresentativeCompanyUserTradeFairFacadeInterface
*/
Expand Down Expand Up @@ -77,12 +84,40 @@ protected function _before(): void
->disableOriginalConstructor()
->getMock();

$this->loggerMock = $this
->getMockBuilder(LoggerInterface::class)
->disableOriginalConstructor()
->getMock();

$this->representativeCompanyUserTradeFairRestApiToCompanyTypeFacadeMock = $this
->getMockBuilder(RepresentativeCompanyUserTradeFairRestApiToCompanyTypeFacadeInterface::class)
->disableOriginalConstructor()
->getMock();

$this->factory = new RepresentativeCompanyUserTradeFairRestApiBusinessFactory();
$this->factory = new class ($this->loggerMock) extends RepresentativeCompanyUserTradeFairRestApiBusinessFactory {
/**
* @var \Psr\Log\LoggerInterface
*/
protected LoggerInterface $loggerMock;

/**
* @param \Psr\Log\LoggerInterface $loggerMock
*/
public function __construct(LoggerInterface $loggerMock)
{
$this->loggerMock = $loggerMock;
}

/**
* @param \Spryker\Shared\Log\Config\LoggerConfigInterface|null $loggerConfig
*
* @return \Psr\Log\LoggerInterface
*/
public function getLogger(?LoggerConfigInterface $loggerConfig = null): LoggerInterface
{
return $this->loggerMock;
}
};
$this->factory->setContainer($this->containerMock);
$this->factory->setConfig($this->configMock);
$this->factory->setRepository($this->repositoryMock);
Expand Down

0 comments on commit 26aadc2

Please sign in to comment.