Skip to content

Commit 8f433d3

Browse files
OHRM5X-2228: Fix pim - job contract broken issue (orangehrm#1699)
1 parent 9025ac0 commit 8f433d3

File tree

5 files changed

+94
-29
lines changed

5 files changed

+94
-29
lines changed

src/plugins/orangehrmPimPlugin/Api/EmploymentContractAPI.php

+9-4
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
use OrangeHRM\Entity\EmpContract;
3737
use OrangeHRM\Entity\EmployeeAttachment;
3838
use OrangeHRM\Pim\Api\Model\EmploymentContractModel;
39+
use OrangeHRM\Pim\Dto\PartialEmployeeAttachment;
3940
use OrangeHRM\Pim\Service\EmploymentContractService;
4041

4142
class EmploymentContractAPI extends Endpoint implements ResourceEndpoint
@@ -176,26 +177,30 @@ private function updateContractAttachment(int $empNumber): void
176177
self::PARAMETER_CURRENT_CONTRACT_ATTACHMENT
177178
);
178179

179-
$contractAttachment = $this->getEmploymentContractService()->getContractAttachment($empNumber);
180+
$partialContractAttachment = $this->getEmploymentContractService()->getContractAttachment($empNumber);
180181

181-
if (!$contractAttachment instanceof EmployeeAttachment && $currentContractAttachment) {
182+
if (!$partialContractAttachment instanceof PartialEmployeeAttachment && $currentContractAttachment) {
182183
throw $this->getBadRequestException(
183184
"`" . self::PARAMETER_CURRENT_CONTRACT_ATTACHMENT . "` should not define if there is no contract attachment"
184185
);
185-
} elseif ($contractAttachment instanceof EmployeeAttachment && !$currentContractAttachment) {
186+
} elseif ($partialContractAttachment instanceof PartialEmployeeAttachment && !$currentContractAttachment) {
186187
throw $this->getBadRequestException(
187188
"`" . self::PARAMETER_CURRENT_CONTRACT_ATTACHMENT . "` should define if there is contract attachment"
188189
);
189190
}
190191

191-
if (!$contractAttachment instanceof EmployeeAttachment && $base64Attachment) {
192+
if (!$partialContractAttachment instanceof PartialEmployeeAttachment && $base64Attachment) {
192193
$contractAttachment = new EmployeeAttachment();
193194
$contractAttachment->getDecorator()->setEmployeeByEmpNumber($empNumber);
194195
$this->setAttachmentAttributes($contractAttachment, $base64Attachment);
195196
$this->getEmploymentContractService()->saveContractAttachment($contractAttachment);
196197
} elseif ($currentContractAttachment === self::CONTRACT_ATTACHMENT_DELETE_CURRENT) {
198+
$contractAttachment = $this->getEmploymentContractService()
199+
->getContractAttachmentById($empNumber, $partialContractAttachment->getAttachId());
197200
$this->getEmploymentContractService()->deleteContractAttachment($contractAttachment);
198201
} elseif ($currentContractAttachment === self::CONTRACT_ATTACHMENT_REPLACE_CURRENT) {
202+
$contractAttachment = $this->getEmploymentContractService()
203+
->getContractAttachmentById($empNumber, $partialContractAttachment->getAttachId());
199204
$this->setAttachmentAttributes($contractAttachment, $base64Attachment);
200205
$this->getEmploymentContractService()->saveContractAttachment($contractAttachment);
201206
}

src/plugins/orangehrmPimPlugin/Api/Model/EmploymentContractModel.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ public function __construct(EmpContract $empContract)
4040
['getDecorator', 'getContractAttachment', 'getFileType'],
4141
['getDecorator', 'getContractAttachment', 'getAttachedBy'],
4242
['getDecorator', 'getContractAttachment', 'getAttachedByName'],
43-
['getDecorator', 'getContractAttachment', 'getDecorator', 'getAttachedTime'],
44-
['getDecorator', 'getContractAttachment', 'getDecorator', 'getAttachedDate'],
43+
['getDecorator', 'getContractAttachment', 'getAttachedTime'],
44+
['getDecorator', 'getContractAttachment', 'getAttachedDate'],
4545
]
4646
);
4747
$this->setAttributeNames(

src/plugins/orangehrmPimPlugin/Service/EmploymentContractService.php

+17-2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use OrangeHRM\Core\Exception\DaoException;
2323
use OrangeHRM\Entity\EmployeeAttachment;
2424
use OrangeHRM\Pim\Dao\EmploymentContractDao;
25+
use OrangeHRM\Pim\Dto\PartialEmployeeAttachment;
2526

2627
class EmploymentContractService
2728
{
@@ -59,10 +60,10 @@ public function getEmployeeAttachmentService(): EmployeeAttachmentService
5960

6061
/**
6162
* @param int $empNumber
62-
* @return EmployeeAttachment|null
63+
* @return PartialEmployeeAttachment|null
6364
* @throws DaoException
6465
*/
65-
public function getContractAttachment(int $empNumber): ?EmployeeAttachment
66+
public function getContractAttachment(int $empNumber): ?PartialEmployeeAttachment
6667
{
6768
$employeeAttachments = $this->getEmployeeAttachmentService()->getEmployeeAttachments(
6869
$empNumber,
@@ -74,6 +75,20 @@ public function getContractAttachment(int $empNumber): ?EmployeeAttachment
7475
return $employeeAttachments[0];
7576
}
7677

78+
/**
79+
* @param int $empNumber
80+
* @param int $attachId
81+
* @return EmployeeAttachment|null
82+
*/
83+
public function getContractAttachmentById(int $empNumber, int $attachId): ?EmployeeAttachment
84+
{
85+
return $this->getEmployeeAttachmentService()->getEmployeeAttachment(
86+
$empNumber,
87+
$attachId,
88+
EmployeeAttachment::SCREEN_JOB_CONTRACT
89+
);
90+
}
91+
7792
/**
7893
* @param EmployeeAttachment $employeeAttachment
7994
* @return EmployeeAttachment

src/plugins/orangehrmPimPlugin/entity/Decorator/EmpContractDecorator.php

+3-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
use OrangeHRM\Core\Traits\Service\DateTimeHelperTrait;
2424
use OrangeHRM\Entity\EmpContract;
2525
use OrangeHRM\Entity\Employee;
26-
use OrangeHRM\Entity\EmployeeAttachment;
26+
use OrangeHRM\Pim\Dto\PartialEmployeeAttachment;
2727
use OrangeHRM\Pim\Service\EmploymentContractService;
2828

2929
class EmpContractDecorator
@@ -87,9 +87,9 @@ public function getEndDate(): ?string
8787
}
8888

8989
/**
90-
* @return EmployeeAttachment|null
90+
* @return PartialEmployeeAttachment|null
9191
*/
92-
public function getContractAttachment(): ?EmployeeAttachment
92+
public function getContractAttachment(): ?PartialEmployeeAttachment
9393
{
9494
$empNumber = $this->getEmpContract()->getEmployee()->getEmpNumber();
9595
return $this->getEmploymentContractService()->getContractAttachment($empNumber);

src/plugins/orangehrmPimPlugin/test/Api/EmploymentContractAPITest.php

+63-18
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
use OrangeHRM\Framework\Services;
3535
use OrangeHRM\Pim\Api\EmploymentContractAPI;
3636
use OrangeHRM\Pim\Dao\EmploymentContractDao;
37+
use OrangeHRM\Pim\Dto\PartialEmployeeAttachment;
3738
use OrangeHRM\Pim\Service\EmploymentContractService;
3839
use OrangeHRM\Tests\Util\EndpointTestCase;
3940
use OrangeHRM\Tests\Util\MockObject;
@@ -326,13 +327,16 @@ public function testUpdateNewContractAttachment(): void
326327
->getMock();
327328

328329
$attachTime = new DateTime();
329-
$empAttachment = new EmployeeAttachment();
330-
$empAttachment->setAttachment('test');
331-
$empAttachment->setAttachId(1);
332-
$empAttachment->setFilename('attachment.txt');
333-
$empAttachment->setFileType('text/plain');
334-
$empAttachment->setSize(6);
335-
$empAttachment->setAttachedTime($attachTime);
330+
$empAttachment = new PartialEmployeeAttachment(
331+
1,
332+
null,
333+
'attachment.txt',
334+
6,
335+
'text/plain',
336+
null,
337+
null,
338+
$attachTime
339+
);
336340
$empContractDecorator = $this->getMockBuilder(EmpContractDecorator::class)
337341
->onlyMethods(['getContractAttachment'])
338342
->setConstructorArgs([$empContract])
@@ -505,6 +509,7 @@ public function testUpdateDeleteContractAttachment(): void
505509
[
506510
'getEmploymentContractDao',
507511
'getContractAttachment',
512+
'getContractAttachmentById',
508513
'saveContractAttachment',
509514
'deleteContractAttachment'
510515
]
@@ -517,19 +522,37 @@ public function testUpdateDeleteContractAttachment(): void
517522
$employee = new Employee();
518523
$employee->setEmpNumber(1);
519524
$contractAttachment = new EmployeeAttachment();
525+
$contractAttachment->setAttachId(20);
520526
$contractAttachment->setEmployee($employee);
521527
$contractAttachment->setFilename('attachment.txt');
522528
$contractAttachment->setAttachment('text');
523529
$contractAttachment->setFileType('text/plain');
524530
$contractAttachment->setSize(1024);
525531

532+
$partialContractAttachment = new PartialEmployeeAttachment(
533+
$contractAttachment->getAttachId(),
534+
null,
535+
$contractAttachment->getFilename(),
536+
$contractAttachment->getSize(),
537+
$contractAttachment->getFileType(),
538+
null,
539+
null,
540+
null
541+
);
542+
526543
$contractAttachmentMap = [
527-
[1, $contractAttachment],
544+
[1, $partialContractAttachment],
528545
[2, null],
529546
];
530547
$employmentContractService->expects($this->once())
531548
->method('getContractAttachment')
532549
->will($this->returnValueMap($contractAttachmentMap));
550+
$employmentContractService->expects($this->once())
551+
->method('getContractAttachmentById')
552+
->will($this->returnValueMap([
553+
[1, 20, $contractAttachment],
554+
[2, 1, null],
555+
]));
533556
$employmentContractService->expects($this->once())
534557
->method('deleteContractAttachment');
535558

@@ -546,7 +569,7 @@ public function testUpdateDeleteContractAttachment(): void
546569
]
547570
)->onlyMethods(['getEmploymentContractService'])
548571
->getMock();
549-
$api->expects($this->exactly(4))
572+
$api->expects($this->exactly(5))
550573
->method('getEmploymentContractService')
551574
->will($this->returnValue($employmentContractService));
552575

@@ -590,13 +613,16 @@ public function testUpdateReplaceContractAttachment(): void
590613
->getMock();
591614

592615
$attachTime = new DateTime();
593-
$empAttachment = new EmployeeAttachment();
594-
$empAttachment->setAttachment('test');
595-
$empAttachment->setAttachId(1);
596-
$empAttachment->setFilename('attachment.txt');
597-
$empAttachment->setFileType('text/plain');
598-
$empAttachment->setSize(6);
599-
$empAttachment->setAttachedTime($attachTime);
616+
$empAttachment = new PartialEmployeeAttachment(
617+
1,
618+
null,
619+
'attachment.txt',
620+
6,
621+
'text/plain',
622+
null,
623+
null,
624+
$attachTime
625+
);
600626
$empContractDecorator = $this->getMockBuilder(EmpContractDecorator::class)
601627
->onlyMethods(['getContractAttachment'])
602628
->setConstructorArgs([$empContract])
@@ -616,6 +642,7 @@ public function testUpdateReplaceContractAttachment(): void
616642
[
617643
'getEmploymentContractDao',
618644
'getContractAttachment',
645+
'getContractAttachmentById',
619646
'saveContractAttachment',
620647
'deleteContractAttachment'
621648
]
@@ -628,19 +655,37 @@ public function testUpdateReplaceContractAttachment(): void
628655
$employee = new Employee();
629656
$employee->setEmpNumber(1);
630657
$contractAttachment = new EmployeeAttachment();
658+
$contractAttachment->setAttachId(30);
631659
$contractAttachment->setEmployee($employee);
632660
$contractAttachment->setFilename('attachment.txt');
633661
$contractAttachment->setAttachment('text');
634662
$contractAttachment->setFileType('text/plain');
635663
$contractAttachment->setSize(1024);
636664

665+
$partialContractAttachment = new PartialEmployeeAttachment(
666+
$contractAttachment->getAttachId(),
667+
null,
668+
$contractAttachment->getFilename(),
669+
$contractAttachment->getSize(),
670+
$contractAttachment->getFileType(),
671+
null,
672+
null,
673+
null
674+
);
675+
637676
$contractAttachmentMap = [
638-
[1, $contractAttachment],
677+
[1, $partialContractAttachment],
639678
[2, null],
640679
];
641680
$employmentContractService->expects($this->once())
642681
->method('getContractAttachment')
643682
->will($this->returnValueMap($contractAttachmentMap));
683+
$employmentContractService->expects($this->once())
684+
->method('getContractAttachmentById')
685+
->will($this->returnValueMap([
686+
[1, 30, $contractAttachment],
687+
[2, 1, null],
688+
]));
644689
$employmentContractService->expects($this->once())
645690
->method('saveContractAttachment');
646691

@@ -674,7 +719,7 @@ public function testUpdateReplaceContractAttachment(): void
674719
]
675720
)->onlyMethods(['getEmploymentContractService', 'getUserRoleManager'])
676721
->getMock();
677-
$api->expects($this->exactly(4))
722+
$api->expects($this->exactly(5))
678723
->method('getEmploymentContractService')
679724
->will($this->returnValue($employmentContractService));
680725
$api->expects($this->exactly(2))

0 commit comments

Comments
 (0)