From d56dfb097efafc5a81b5add898640311a574a7f9 Mon Sep 17 00:00:00 2001 From: Tunbola Ogunwande Date: Tue, 31 Jul 2018 15:50:38 +0100 Subject: [PATCH 1/6] PCHR-4051: Add the canCancelForAbsenceType method in LeaveRequestRights Service. Add tests. --- .../Service/LeaveRequestRights.php | 31 +++++++++++ .../Service/LeaveRequestRightsTest.php | 55 +++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php b/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php index 648d8dabab0..6fe8db1752b 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php @@ -209,4 +209,35 @@ public function getLeaveContactsCurrentUserHasAccessTo() { return $results; } + + /** + * Checks if the current user can cancel the leave request for the given absence type, + * leave contact and leave from date. + * + * @param int $absenceTypeId + * @param int $contactId + * @param \DateTime $leaveFromDate + * + * @return bool + */ + public function canCancelForAbsenceType($absenceTypeId, $contactId, DateTime $leaveFromDate) { + if ($this->currentUserIsAdmin() || $this->currentUserIsLeaveManagerOf($contactId)) { + return TRUE; + } + + $absenceType = AbsenceType::findById($absenceTypeId); + if ($absenceType->allow_request_cancelation == AbsenceType::REQUEST_CANCELATION_ALWAYS) { + return TRUE; + } + + $today = new DateTime('today'); + + if ($absenceType->allow_request_cancelation != AbsenceType::REQUEST_CANCELATION_NO && + $leaveFromDate > $today + ) { + return TRUE; + } + + return FALSE; + } } diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestRightsTest.php b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestRightsTest.php index 8d959f2a5ff..dd77e1f7662 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestRightsTest.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestRightsTest.php @@ -4,6 +4,7 @@ use CRM_HRLeaveAndAbsences_BAO_LeaveRequest as LeaveRequest; use CRM_HRLeaveAndAbsences_Test_Fabricator_AbsenceType as AbsenceTypeFabricator; use CRM_HRCore_Test_Fabricator_Contact as ContactFabricator; +use CRM_HRLeaveAndAbsences_BAO_AbsenceType as AbsenceType; /** * Class CRM_HRLeaveAndAbsences_Service_LeaveRequestRightsTest @@ -323,6 +324,60 @@ public function testAdminShouldHaveAccessToAllContacts() { $this->assertEquals([], $accessibleContacts); } + public function testCanCancelForAbsenceTypeReturnsTrueWhenUserIsAdmin() { + $typeId = 1; + $contactID = 2; + $leaveDate = new DateTime(); + $leaveRequestRightsService = $this->getLeaveRequestRightsForAdminAsCurrentUser(); + $result = $leaveRequestRightsService->canCancelForAbsenceType($typeId, $contactID, $leaveDate); + $this->assertTrue($result); + } + + public function testCanCancelForAbsenceTypeReturnsTrueWhenUserIsLeaveManager() { + $typeId = 1; + $contactID = 2; + $leaveDate = new DateTime(); + $leaveRequestRightsService = $this->getLeaveRequestRightsForLeaveManagerAsCurrentUser(); + $result = $leaveRequestRightsService->canCancelForAbsenceType($typeId, $contactID, $leaveDate); + $this->assertTrue($result); + } + + public function testCanCancelForAbsenceTypeReturnsTrueWhenAbsenceTypeAllowsCancellationForStaff() { + $absenceType = AbsenceTypeFabricator::fabricate(['allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_ALWAYS]); + $contactID = 2; + $leaveDate = new DateTime(); + $leaveRequestRightsService = $this->getLeaveRightsService(); + $result = $leaveRequestRightsService->canCancelForAbsenceType($absenceType->id, $contactID, $leaveDate); + $this->assertTrue($result); + } + + public function testCanCancelForAbsenceTypeReturnsFalseWhenAbsenceTypeDoesNotAllowCancellationForStaff() { + $absenceType = AbsenceTypeFabricator::fabricate(['allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_NO]); + $contactID = 2; + $leaveDate = new DateTime(); + $leaveRequestRightsService = $this->getLeaveRightsService(); + $result = $leaveRequestRightsService->canCancelForAbsenceType($absenceType->id, $contactID, $leaveDate); + $this->assertFalse($result); + } + + public function testCanCancelForAbsenceTypeReturnsFalseWhenAbsenceTypeAllowsCancellationForFutureDateButLeaveDateIsPast() { + $absenceType = AbsenceTypeFabricator::fabricate(['allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_IN_ADVANCE_OF_START_DATE]); + $contactID = 2; + $leaveDate = new DateTime('yesterday'); + $leaveRequestRightsService = $this->getLeaveRightsService(); + $result = $leaveRequestRightsService->canCancelForAbsenceType($absenceType->id, $contactID, $leaveDate); + $this->assertFalse($result); + } + + public function testCanCancelForAbsenceTypeReturnsTrueWhenAbsenceTypeAllowsCancellationForFutureDateAndLeaveDateIsInFuture() { + $absenceType = AbsenceTypeFabricator::fabricate(['allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_IN_ADVANCE_OF_START_DATE]); + $contactID = 2; + $leaveDate = new DateTime('tomorrow'); + $leaveRequestRightsService = $this->getLeaveRightsService(); + $result = $leaveRequestRightsService->canCancelForAbsenceType($absenceType->id, $contactID, $leaveDate); + $this->assertTrue($result); + } + private function getLeaveRightsService($isAdmin = FALSE, $isManager = FALSE) { $leaveManagerService = $this->createLeaveManagerServiceMock($isAdmin, $isManager); return new LeaveRequestRightsService($leaveManagerService); From 3cf2fc4d4df8d1379633f84aef3eca3d323f15cc Mon Sep 17 00:00:00 2001 From: Tunbola Ogunwande Date: Tue, 31 Jul 2018 15:55:35 +0100 Subject: [PATCH 2/6] PCHR-4051: Use the canCancelForAbsenceType method in LeaveRequest Service. Add tests. Remove validation for absence type cancellation in Leave Request BAO. --- .../HRLeaveAndAbsences/BAO/LeaveRequest.php | 41 ------------------- .../Service/LeaveRequest.php | 27 +++++++++++- .../Service/LeaveRequestTest.php | 32 +++++++++++++++ 3 files changed, 58 insertions(+), 42 deletions(-) diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/BAO/LeaveRequest.php b/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/BAO/LeaveRequest.php index b4731ab25d9..91591e6a00a 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/BAO/LeaveRequest.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/BAO/LeaveRequest.php @@ -101,7 +101,6 @@ public static function validateParams($params, $validationMode = self::VALIDATIO self::validateAbsenceTypeIsActiveAndValid($params, $absenceType); self::validateTOILRequest($params, $absenceType, $absencePeriod); self::validateLeaveDaysAgainstAbsenceTypeMaxConsecutiveLeaveDays($params, $absenceType); - self::validateAbsenceTypeAllowRequestCancellationForLeaveRequestCancellation($params, $absenceType); self::validateAbsencePeriod($params, $absencePeriod); if($validationMode != self::IMPORT_VALIDATION) { @@ -731,46 +730,6 @@ private static function validateLeaveDaysAgainstAbsenceTypeMaxConsecutiveLeaveDa } } - /** - * This method checks if the absence type allows cancellation in advance of start date and that the leave request from_date - * should not be in the past in the event of a leave request cancellation by a user. - * - * Also checks that a user's leave request should not be cancelled if the absence type does not - * allow leave request cancellation - * - * @param array $params - * The params array received by the create method - * @param AbsenceType $absenceType - * - * @throws \CRM_HRLeaveAndAbsences_Exception_InvalidLeaveRequestException - */ - private static function validateAbsenceTypeAllowRequestCancellationForLeaveRequestCancellation($params, $absenceType) { - $leaveRequestStatuses = self::getStatuses(); - $leaveRequestIsForCurrentUser = CRM_Core_Session::getLoggedInContactID() == $params['contact_id']; - $isACancellationRequest = ($params['status_id'] == $leaveRequestStatuses['cancelled']); - - if($leaveRequestIsForCurrentUser && $isACancellationRequest) { - $today = new DateTime('today'); - $fromDate = new DateTime($params['from_date']); - - if($absenceType->allow_request_cancelation == AbsenceType::REQUEST_CANCELATION_IN_ADVANCE_OF_START_DATE && $fromDate < $today) { - throw new InvalidLeaveRequestException( - 'Leave Request with past days cannot be cancelled', - 'leave_request_past_days_cannot_be_cancelled', - 'type_id' - ); - } - - if($absenceType->allow_request_cancelation == AbsenceType::REQUEST_CANCELATION_NO) { - throw new InvalidLeaveRequestException( - 'Absence Type does not allow leave request cancellation', - 'leave_request_absence_type_disallows_cancellation', - 'type_id' - ); - } - } - } - /** * This method validates that the absence type is active and is valid for the * type of request. That is, if this is a Sickness Request, then only absence diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequest.php b/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequest.php index 20805846edf..c2cd51fe064 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequest.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequest.php @@ -115,8 +115,17 @@ protected function update($params) { $this->getErrorMessageForInvalidStatusTransition($params)); } - $isTOILWithPastDates = LeaveRequest::isTOILWithPastDates($params); $isCancelledStatus = $this->isCancelledStatus($params); + + if ($isCancelledStatus) { + if(!$this->canCancelForAbsenceType($params)) { + throw new RuntimeException( + 'You cannot cancel leave requests for this Absence type' + ); + } + } + + $isTOILWithPastDates = LeaveRequest::isTOILWithPastDates($params); $canCanCancelTOILWithPastDates = $this->canCanCancelTOILWithPastDates($params); if ($isTOILWithPastDates && $isCancelledStatus && !$canCanCancelTOILWithPastDates) { @@ -128,6 +137,22 @@ protected function update($params) { return $this->createRequestWithBalanceChanges($params); } + /** + * Whether the current user can cancel a leave request for the absence + * type. + * + * @param array $params + * + * @return bool + */ + private function canCancelForAbsenceType($params) { + return $this->leaveRequestRightsService->canCancelForAbsenceType( + $params['type_id'], + $params['contact_id'], + new DateTime($params['from_date']) + ); + } + /** * Return an error message either for specific invalid status transition cases * or a default generic message diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php index 8a6c538aff3..99e010cbae0 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php @@ -499,6 +499,18 @@ private function getLeaveRequestService($isAdmin = false, $isManager = false, $a ); } + private function getLeaveRequestServiceForWhenAbsenceTypeCannotBeCancelled($typeId, $contactID, $leaveFromDate) { + $leaveRightsService = $this->prophesize(LeaveRequestRightsService::class); + $leaveRightsService->canCreateAndUpdateFor($contactID)->willReturn(TRUE); + $leaveRightsService->canCancelForAbsenceType($typeId, $contactID, new DateTime($leaveFromDate))->willReturn(FALSE); + + return new LeaveRequestService( + $this->leaveBalanceChangeService, + $this->createLeaveRequestStatusMatrixServiceMock(TRUE), + $leaveRightsService->reveal() + ); + } + public function testLeaveRequestServiceCallsRecalculateExpiredBalanceChangesForLeaveRequestPastDatesMethodWhenALeaveRequestHasPastDates() { $params = $this->getDefaultParams([ 'from_date' => CRM_Utils_Date::processDate('-2 days'), @@ -1233,6 +1245,26 @@ public function testToilRequestWithPastDatesCanBeCancelledWhenAbsenceTypeAllowsP $this->assertEquals($toilRequest->status_id, $leaveStatuses['cancelled']); } + public function testCreateThrowsAnExceptionWhenUserIsNotAllowedToCancelAbsenceType() { + $leaveRequestStatuses = LeaveRequest::getStatuses(); + $this->registerCurrentLoggedInContactInSession($this->leaveContact); + $typeId = 1; + $params = $this->getDefaultParams([ + 'contact_id' => $this->leaveContact, + 'type_id' => $typeId, + ]); + + $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation($params); + $params['status_id'] = $leaveRequestStatuses['cancelled']; + $params['id'] = $leaveRequest->id; + + $leaveRequestService = $this->getLeaveRequestServiceForWhenAbsenceTypeCannotBeCancelled( + $params['type_id'], $params['contact_id'], $params['from_date']); + + $this->setExpectedException(RuntimeException::class, 'You cannot cancel leave requests for this Absence type'); + $leaveRequestService->create($params, false); + } + private function getExpectedBreakdownForLeaveRequest(LeaveRequest $leaveRequest, $amount = false) { $leaveRequestDayTypes = LeaveRequest::buildOptions('from_date_type'); From c22b77e56a8d52a3d3e7c0d3041853100c2b0851 Mon Sep 17 00:00:00 2001 From: Tunbola Ogunwande Date: Tue, 31 Jul 2018 16:12:29 +0100 Subject: [PATCH 3/6] PCHR-4051: Remove obsolete tests. Fix broken tests. --- .../BAO/LeaveRequestTest.php | 66 --------------- .../Service/LeaveRequestTest.php | 7 +- .../tests/phpunit/api/v3/LeaveRequestTest.php | 80 ------------------- 3 files changed, 5 insertions(+), 148 deletions(-) diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/BAO/LeaveRequestTest.php b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/BAO/LeaveRequestTest.php index fdcd2be3a13..449369760e5 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/BAO/LeaveRequestTest.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/BAO/LeaveRequestTest.php @@ -1000,72 +1000,6 @@ public function testLeaveRequestCanBeCreatedWhenNumberOfLeaveWorkingDaysNotGreat ]); } - public function testAUserCannotCancelOwnLeaveRequestWhenAbsenceTypeDoesNotAllowIt() { - $contactID = 5; - $this->registerCurrentLoggedInContactInSession($contactID); - - $absenceType = AbsenceTypeFabricator::fabricate([ - 'allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_NO - ]); - - $leaveRequestStatuses = array_flip(LeaveRequest::buildOptions('status_id', 'validate')); - - $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation([ - 'type_id' => $absenceType->id, - 'contact_id' => $contactID, - 'status_id' => $leaveRequestStatuses['awaiting_approval'], - 'from_date' => CRM_Utils_Date::processDate('now'), - 'to_date' => CRM_Utils_Date::processDate('+4 days') - ]); - - $this->setExpectedException('CRM_HRLeaveAndAbsences_Exception_InvalidLeaveRequestException', 'Absence Type does not allow leave request cancellation'); - //cancel leave request - LeaveRequest::create([ - 'id' => $leaveRequest->id, - 'type_id' => $absenceType->id, - 'contact_id' => $contactID, - 'status_id' => $leaveRequestStatuses['cancelled'], - 'from_date' => CRM_Utils_Date::processDate('now'), - 'from_date_type' => 1, - 'to_date' => CRM_Utils_Date::processDate('+4 days'), - 'to_date_type' => 1, - 'request_type' => LeaveRequest::REQUEST_TYPE_LEAVE - ]); - } - - public function testAUserCannotCancelOwnLeaveRequestWhenAbsenceTypeAllowsItInAdvanceOfStartDateAndFromDateIsLessThanToday() { - $contactID = 5; - $this->registerCurrentLoggedInContactInSession($contactID); - - $absenceType = AbsenceTypeFabricator::fabricate([ - 'allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_IN_ADVANCE_OF_START_DATE - ]); - - $leaveRequestStatuses = array_flip(LeaveRequest::buildOptions('status_id', 'validate')); - - $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation([ - 'type_id' => $absenceType->id, - 'contact_id' => $contactID, - 'status_id' => $leaveRequestStatuses['awaiting_approval'], - 'from_date' => CRM_Utils_Date::processDate('-1 day'), - 'to_date' => CRM_Utils_Date::processDate('+4 days') - ]); - - $this->setExpectedException('CRM_HRLeaveAndAbsences_Exception_InvalidLeaveRequestException', 'Leave Request with past days cannot be cancelled'); - //cancel leave request - LeaveRequest::create([ - 'id' => $leaveRequest->id, - 'type_id' => $absenceType->id, - 'contact_id' => $contactID, - 'status_id' => $leaveRequestStatuses['cancelled'], - 'from_date' => CRM_Utils_Date::processDate('-1 day'), - 'from_date_type' => 1, - 'to_date' => CRM_Utils_Date::processDate('+4 days'), - 'to_date_type' => 1, - 'request_type' => LeaveRequest::REQUEST_TYPE_LEAVE - ]); - } - public function testFindOverlappingLeaveRequestsForOneOverlappingLeaveRequest() { $contactID = 1; $fromDate1 = new DateTime('2016-11-02'); diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php index 99e010cbae0..2d3b2c70da7 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php @@ -3,6 +3,7 @@ use CRM_Hrjobcontract_Test_Fabricator_HRJobContract as HRJobContractFabricator; use CRM_HRLeaveAndAbsences_BAO_LeaveBalanceChange as LeaveBalanceChange; use CRM_HRLeaveAndAbsences_BAO_LeaveRequest as LeaveRequest; +use CRM_HRLeaveAndAbsences_BAO_AbsenceType as AbsenceType; use CRM_HRLeaveAndAbsences_BAO_PublicHoliday as PublicHoliday; use CRM_HRLeaveAndAbsences_Service_LeaveBalanceChange as LeaveBalanceChangeService; use CRM_HRLeaveAndAbsences_Service_LeaveRequest as LeaveRequestService; @@ -1128,7 +1129,8 @@ public function testGetBreakdownIncludeOnlyTheLeaveBalanceChangesOfTheLeaveReque public function testToilRequestWithPastDatesCanNotBeCancelledWhenUserIsLeaveContactAndAbsenceTypeDoesNotAllowPastAccrual() { $absenceType = AbsenceTypeFabricator::fabricate([ 'allow_accruals_request' => true, - 'allow_accrue_in_the_past' => false + 'allow_accrue_in_the_past' => false, + 'allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_ALWAYS ]); $leaveStatuses = LeaveRequest::getStatuses(); @@ -1216,7 +1218,8 @@ public function testToilRequestWithPastDatesCanBeCancelledWhenUserIsManagerAndAb public function testToilRequestWithPastDatesCanBeCancelledWhenAbsenceTypeAllowsPastAccrual() { $absenceType = AbsenceTypeFabricator::fabricate([ 'allow_accruals_request' => true, - 'allow_accrue_in_the_past' => true + 'allow_accrue_in_the_past' => true, + 'allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_ALWAYS ]); $leaveStatuses = LeaveRequest::getStatuses(); diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/api/v3/LeaveRequestTest.php b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/api/v3/LeaveRequestTest.php index d75026e6ee9..f9f844cdf6d 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/api/v3/LeaveRequestTest.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/api/v3/LeaveRequestTest.php @@ -2856,86 +2856,6 @@ public function testLeaveRequestIsValidShouldReturnErrorWhenLeaveDaysIsGreaterTh $this->assertEquals($expectedResult, $result); } - public function testLeaveRequestIsValidShouldReturnErrorWhenUserCancelsOwnLeaveRequestAndAbsenceTypeDoesNotAllowIt() { - $contactID = 5; - $this->registerCurrentLoggedInContactInSession($contactID); - - $absenceType = AbsenceTypeFabricator::fabricate([ - 'allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_NO - ]); - - $fromDate = new DateTime(); - $toDate = new DateTime('+4 days'); - $leaveRequestStatuses = array_flip(LeaveRequest::buildOptions('status_id', 'validate')); - - $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation([ - 'type_id' => $absenceType->id, - 'contact_id' => $contactID, - 'status_id' => $leaveRequestStatuses['awaiting_approval'], - 'from_date' => $fromDate->format('YmdHis'), - 'from_date_type' => 1, - 'to_date' => $toDate->format('YmdHis'), - 'to_date_type' => 1 - ]); - - //cancel leave request - $result = civicrm_api3('LeaveRequest', 'isvalid', [ - 'id' => $leaveRequest->id, - 'type_id' => $absenceType->id, - 'contact_id' => $contactID, - 'status_id' => $leaveRequestStatuses['cancelled'], - 'from_date' => $fromDate->format('YmdHis'), - 'from_date_type' => 1, - 'to_date' => $toDate->format('YmdHis'), - 'to_date_type' => 1, - 'request_type' => LeaveRequest::REQUEST_TYPE_LEAVE - ]); - - $errorMessage = 'Absence Type does not allow leave request cancellation'; - $expectedResult = $this->getExpectedArrayForIsValidError('type_id', $errorMessage); - $this->assertEquals($expectedResult, $result); - } - - public function testLeaveRequestIsValidShouldReturnErrorWhenUserCancelsOwnLeaveRequestAndAbsenceTypeAllowsItInAdvanceOfStartDateAndLeaveRequestFromDateIsLessThanToday() { - $contactID = 5; - $this->registerCurrentLoggedInContactInSession($contactID); - - $absenceType = AbsenceTypeFabricator::fabricate([ - 'allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_IN_ADVANCE_OF_START_DATE - ]); - - $fromDate = new DateTime('-1 day'); - $toDate = new DateTime('+4 days'); - $leaveRequestStatuses = array_flip(LeaveRequest::buildOptions('status_id', 'validate')); - - $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation([ - 'type_id' => $absenceType->id, - 'contact_id' => $contactID, - 'status_id' => $leaveRequestStatuses['awaiting_approval'], - 'from_date' => $fromDate->format('YmdHis'), - 'from_date_type' => 1, - 'to_date' => $toDate->format('YmdHis'), - 'to_date_type' => 1 - ]); - - //cancel leave request - $result = civicrm_api3('LeaveRequest', 'isvalid', [ - 'id' => $leaveRequest->id, - 'type_id' => $absenceType->id, - 'contact_id' => $contactID, - 'status_id' => $leaveRequestStatuses['cancelled'], - 'from_date' => $fromDate->format('YmdHis'), - 'from_date_type' => 1, - 'to_date' => $toDate->format('YmdHis'), - 'to_date_type' => 1, - 'request_type' => LeaveRequest::REQUEST_TYPE_LEAVE - ]); - - $errorMessage = 'Leave Request with past days cannot be cancelled'; - $expectedResult = $this->getExpectedArrayForIsValidError('type_id', $errorMessage); - $this->assertEquals($expectedResult, $result); - } - public function testLeaveRequestIsValidShouldReturnAnErrorWhenTheToilToAccrueDoesNotHaveAValidValue() { AbsencePeriodFabricator::fabricate([ 'start_date' => CRM_Utils_Date::processDate('2015-01-01'), From 618dec101359919760a439a767522b2d596f4563 Mon Sep 17 00:00:00 2001 From: Tunbola Ogunwande Date: Tue, 31 Jul 2018 16:43:11 +0100 Subject: [PATCH 4/6] PCHR-4051: Code formatting fixes --- .../HRLeaveAndAbsences/Service/LeaveRequest.php | 2 +- .../Service/LeaveRequestRights.php | 6 +++--- .../Service/LeaveRequestRightsTest.php | 4 +++- .../Service/LeaveRequestTest.php | 16 +++++++++++++--- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequest.php b/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequest.php index c2cd51fe064..b3a75707294 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequest.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequest.php @@ -118,7 +118,7 @@ protected function update($params) { $isCancelledStatus = $this->isCancelledStatus($params); if ($isCancelledStatus) { - if(!$this->canCancelForAbsenceType($params)) { + if (!$this->canCancelForAbsenceType($params)) { throw new RuntimeException( 'You cannot cancel leave requests for this Absence type' ); diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php b/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php index 6fe8db1752b..1e7d29c83ac 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php @@ -232,9 +232,9 @@ public function canCancelForAbsenceType($absenceTypeId, $contactId, DateTime $le $today = new DateTime('today'); - if ($absenceType->allow_request_cancelation != AbsenceType::REQUEST_CANCELATION_NO && - $leaveFromDate > $today - ) { + $absenceTypeRequestCancellationNotNo = + $absenceType->allow_request_cancelation != AbsenceType::REQUEST_CANCELATION_NO; + if ($absenceTypeRequestCancellationNotNo && $leaveFromDate > $today) { return TRUE; } diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestRightsTest.php b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestRightsTest.php index dd77e1f7662..5ad77f8fec1 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestRightsTest.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestRightsTest.php @@ -343,7 +343,9 @@ public function testCanCancelForAbsenceTypeReturnsTrueWhenUserIsLeaveManager() { } public function testCanCancelForAbsenceTypeReturnsTrueWhenAbsenceTypeAllowsCancellationForStaff() { - $absenceType = AbsenceTypeFabricator::fabricate(['allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_ALWAYS]); + $absenceType = AbsenceTypeFabricator::fabricate([ + 'allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_ALWAYS + ]); $contactID = 2; $leaveDate = new DateTime(); $leaveRequestRightsService = $this->getLeaveRightsService(); diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php index 2d3b2c70da7..72a2f388d04 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php @@ -500,10 +500,17 @@ private function getLeaveRequestService($isAdmin = false, $isManager = false, $a ); } - private function getLeaveRequestServiceForWhenAbsenceTypeCannotBeCancelled($typeId, $contactID, $leaveFromDate) { + private function getLeaveRequestServiceForWhenAbsenceTypeCannotBeCancelled( + $typeId, + $contactID, + $leaveFromDate + ) { $leaveRightsService = $this->prophesize(LeaveRequestRightsService::class); $leaveRightsService->canCreateAndUpdateFor($contactID)->willReturn(TRUE); - $leaveRightsService->canCancelForAbsenceType($typeId, $contactID, new DateTime($leaveFromDate))->willReturn(FALSE); + $leaveRightsService->canCancelForAbsenceType( + $typeId, + $contactID, + new DateTime($leaveFromDate))->willReturn(FALSE); return new LeaveRequestService( $this->leaveBalanceChangeService, @@ -1264,7 +1271,10 @@ public function testCreateThrowsAnExceptionWhenUserIsNotAllowedToCancelAbsenceTy $leaveRequestService = $this->getLeaveRequestServiceForWhenAbsenceTypeCannotBeCancelled( $params['type_id'], $params['contact_id'], $params['from_date']); - $this->setExpectedException(RuntimeException::class, 'You cannot cancel leave requests for this Absence type'); + $this->setExpectedException( + RuntimeException::class, + 'You cannot cancel leave requests for this Absence type' + ); $leaveRequestService->create($params, false); } From bbd885613de9e0ca5557aab517bb2178a06478e2 Mon Sep 17 00:00:00 2001 From: Tunbola Ogunwande Date: Tue, 31 Jul 2018 16:48:25 +0100 Subject: [PATCH 5/6] PCHR-4051: Fix boolean case in tests. --- .../Service/LeaveRequestTest.php | 152 +++++++++--------- 1 file changed, 76 insertions(+), 76 deletions(-) diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php index 72a2f388d04..27487c88c53 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/tests/phpunit/CRM/HRLeaveAndAbsences/Service/LeaveRequestTest.php @@ -50,7 +50,7 @@ public function testCreateAlsoCreateTheLeaveRequestBalanceChanges() { ['period_start_date' => '2016-01-01'] ); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); // a 7 days leave request, from monday to sunday $leaveRequest = $this->getleaveRequestService()->create([ @@ -62,7 +62,7 @@ public function testCreateAlsoCreateTheLeaveRequestBalanceChanges() { 'to_date' => CRM_Utils_Date::processDate('2016-01-10'), 'to_date_type' => $this->getLeaveRequestDayTypes()['all_day']['value'], 'request_type' => LeaveRequest::REQUEST_TYPE_LEAVE - ], false); + ], FALSE); $balance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($leaveRequest); // Since the 40 hours work pattern was used, and it this is a week long @@ -81,7 +81,7 @@ public function testCreateAlsoCreateTheLeaveRequestBalanceChangesForLeaveInHours ['period_start_date' => '2016-01-01'] ); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); // a 5 days leave request, from monday to sunday $absenceType = AbsenceTypeFabricator::fabricate(['calculation_unit' => 2]); @@ -94,7 +94,7 @@ public function testCreateAlsoCreateTheLeaveRequestBalanceChangesForLeaveInHours 'to_date' => CRM_Utils_Date::processDate('2016-01-08 16:45'), 'to_date_amount' => 2.4, 'request_type' => LeaveRequest::REQUEST_TYPE_LEAVE - ], false); + ], FALSE); $balance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($leaveRequest); // Since the 40 hours work pattern was used, and it this is a week long @@ -113,7 +113,7 @@ public function testCreateAlsoCreatesTheBalanceChangesForTheLeaveDatesCorrectlyF ['period_start_date' => '2016-01-01'] ); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); // a 5 days leave request, from monday to friday $absenceType = AbsenceTypeFabricator::fabricate(['calculation_unit' => 2]); @@ -126,7 +126,7 @@ public function testCreateAlsoCreatesTheBalanceChangesForTheLeaveDatesCorrectlyF 'to_date' => CRM_Utils_Date::processDate('2016-01-08 16:45'), 'to_date_amount' => 2.4, 'request_type' => LeaveRequest::REQUEST_TYPE_LEAVE - ], false); + ], FALSE); $amountInHours = 8.0; $expectedBreakdown = $this->getExpectedBreakdownForLeaveRequest($leaveRequest, $amountInHours); @@ -143,7 +143,7 @@ public function testCreateAlsoCreateTheLeaveRequestBalanceChangesProperlyForLeav ['period_start_date' => '2016-01-01'] ); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); // a 6 days leave request, from monday to saturday $absenceType = AbsenceTypeFabricator::fabricate(['calculation_unit' => 2]); @@ -156,7 +156,7 @@ public function testCreateAlsoCreateTheLeaveRequestBalanceChangesProperlyForLeav 'to_date' => CRM_Utils_Date::processDate('2016-01-09 16:45'), 'to_date_amount' => 2.4, 'request_type' => LeaveRequest::REQUEST_TYPE_LEAVE - ], false); + ], FALSE); $balance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($leaveRequest); // Since the 40 hours work pattern was used, and it this is a week long @@ -176,7 +176,7 @@ public function testCreateDoesNotDuplicateLeaveBalanceChangesOnUpdate() { ['period_start_date' => '2016-01-01'] ); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); // a 7 days leave request, from friday to thursday $params = [ @@ -190,7 +190,7 @@ public function testCreateDoesNotDuplicateLeaveBalanceChangesOnUpdate() { 'request_type' => LeaveRequest::REQUEST_TYPE_LEAVE ]; - $leaveRequest = $this->getleaveRequestService()->create($params, false); + $leaveRequest = $this->getleaveRequestService()->create($params, FALSE); $balance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($leaveRequest); // Since the 40 hours work pattern was used, and it this is a week long @@ -205,7 +205,7 @@ public function testCreateDoesNotDuplicateLeaveBalanceChangesOnUpdate() { // Increase the Leave Request period by 4 days (2 weekend + 2 working days) $params['id'] = $leaveRequest->id; $params['to_date'] = CRM_Utils_Date::processDate('2016-01-11'); - $this->getleaveRequestService()->create($params, false); + $this->getleaveRequestService()->create($params, FALSE); $balance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($leaveRequest); // 5 from before + 2 (from the 2 new working days) @@ -233,7 +233,7 @@ public function testDeleteSoftDeletesTheLeaveRequest() { $leaveRequestRecord = new LeaveRequest(); $leaveRequestRecord->id = $leaveRequest->id; - $leaveRequestRecord->find(true); + $leaveRequestRecord->find(TRUE); $this->assertEquals(1, $leaveRequestRecord->is_deleted); } @@ -249,7 +249,7 @@ public function testDeleteSoftDeletesAPublicHolidayLeaveRequest() { $publicHolidayLeaveRequestRecord = new LeaveRequest(); $publicHolidayLeaveRequestRecord->id = $publicHolidayLeaveRequest->id; - $publicHolidayLeaveRequestRecord->find(true); + $publicHolidayLeaveRequestRecord->find(TRUE); $this->assertEquals(1, $publicHolidayLeaveRequestRecord->is_deleted); } @@ -302,7 +302,7 @@ public function testCreateThrowsAnExceptionWhenCurrentUserDoesNotHaveCreateAndUp //logged in user has no permissions, also a contactID different from that of the logged in user is passed $contactID = 2; $params = $this->getDefaultParams(['contact_id' => $contactID]); - $this->getleaveRequestService()->create($params, false); + $this->getleaveRequestService()->create($params, FALSE); } /** @@ -310,7 +310,7 @@ public function testCreateThrowsAnExceptionWhenCurrentUserDoesNotHaveCreateAndUp * @expectedExceptionMessage You can't create a Leave Request with this status */ public function testCreateThrowsAnExceptionWhenTransitionStatusIsNotValidForNewLeaveRequest() { - $this->getLeaveRequestServiceWhenStatusTransitionIsNotAllowed()->create($this->getDefaultParams(), false); + $this->getLeaveRequestServiceWhenStatusTransitionIsNotAllowed()->create($this->getDefaultParams(), FALSE); } public function testCreateThrowsAnExceptionWhenTransitionStatusIsNotValidWhenUpdatingLeaveRequestStatus() { @@ -326,7 +326,7 @@ public function testCreateThrowsAnExceptionWhenTransitionStatusIsNotValidWhenUpd $params['id'] = $leaveRequest->id; $params['status_id'] = $leaveRequestStatuses['awaiting_approval']; - $this->getLeaveRequestServiceWhenStatusTransitionIsNotAllowed()->create($params, false); + $this->getLeaveRequestServiceWhenStatusTransitionIsNotAllowed()->create($params, FALSE); } /** @@ -342,7 +342,7 @@ public function testCreateThrowsAnExceptionWhenLeaveApproverUpdatesDatesForLeave $params['to_date'] = CRM_Utils_Date::processDate('2016-01-15'); $params['id'] = $leaveRequest->id; - $this->getLeaveRequestServiceWhenCurrentUserIsLeaveManager()->create($params, false); + $this->getLeaveRequestServiceWhenCurrentUserIsLeaveManager()->create($params, FALSE); } public function testCreateDoesNotThrowAnExceptionWhenAdminUpdatesDatesForLeaveRequest() { @@ -354,7 +354,7 @@ public function testCreateDoesNotThrowAnExceptionWhenAdminUpdatesDatesForLeaveRe $params['to_date'] = $toDate->modify('+10 days')->format('YmdHis'); $params['id'] = $leaveRequest->id; - $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create($params, false); + $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create($params, FALSE); $this->assertNotNull($leaveRequest->id); } @@ -374,7 +374,7 @@ public function testCreateDoesNotThrowAnExceptionWhenLeaveManagerUpdatesDatesFor $params['to_date'] = $toDate->modify('+10 days')->format('YmdHis'); $params['id'] = $leaveRequest->id; - $this->getLeaveRequestServiceWhenCurrentUserIsLeaveManager()->create($params, false); + $this->getLeaveRequestServiceWhenCurrentUserIsLeaveManager()->create($params, FALSE); } /** @@ -393,7 +393,7 @@ public function testCreateDoesNotThrowAnExceptionWhenAdminUpdatesDatesForAnOpenS $params['to_date'] = $toDate->modify('+10 days')->format('YmdHis'); $params['id'] = $leaveRequest->id; - $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create($params, false); + $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create($params, FALSE); } /** @@ -414,7 +414,7 @@ public function testCreateThrowsAnExceptionWhenLeaveContactUpdatesDatesForAClose $params['to_date'] = $toDate->modify('+10 days')->format('YmdHis'); $params['id'] = $leaveRequest->id; - $this->getLeaveRequestService()->create($params, false); + $this->getLeaveRequestService()->create($params, FALSE); } /** @@ -429,7 +429,7 @@ public function testCreateThrowsAnExceptionWhenLeaveApproverUpdatesAbsenceTypeFo $params['id'] = $leaveRequest->id; $params['type_id'] = 2; - $this->getLeaveRequestServiceWhenCurrentUserIsLeaveManager()->create($params, false); + $this->getLeaveRequestServiceWhenCurrentUserIsLeaveManager()->create($params, FALSE); } /** @@ -444,7 +444,7 @@ public function testCreateThrowsAnExceptionWhenAdminUpdatesAbsenceTypeForLeaveRe $params['id'] = $leaveRequest->id; $params['type_id'] = 2; - $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create($params, false); + $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create($params, FALSE); } /** @@ -477,13 +477,13 @@ public function testDeleteDoesNotThrowAnExceptionWhenLeaveContactWhoIsOwnLeaveAp //Check that the leave request is actually soft deleted. $leaveRequestRecord = new LeaveRequest(); $leaveRequestRecord->id = $leaveRequest->id; - $leaveRequestRecord->find(true); + $leaveRequestRecord->find(TRUE); $this->assertEquals(1, $leaveRequestRecord->is_deleted); $this->unregisterCurrentLoggedInContactFromSession(); } - private function getLeaveRequestService($isAdmin = false, $isManager = false, $allowStatusTransition = true, $mockBalanceChangeService = false) { + private function getLeaveRequestService($isAdmin = FALSE, $isManager = FALSE, $allowStatusTransition = TRUE, $mockBalanceChangeService = FALSE) { $leaveManagerService = $this->createLeaveManagerServiceMock($isAdmin, $isManager); $leaveRequestStatusMatrixService = $this->createLeaveRequestStatusMatrixServiceMock($allowStatusTransition); $leaveRequestRightsService = new LeaveRequestRightsService($leaveManagerService); @@ -526,23 +526,23 @@ public function testLeaveRequestServiceCallsRecalculateExpiredBalanceChangesForL 'status' => 1 ]); - $this->getLeaveRequestServiceWhenCurrentUserIsAdminWithBalanceChangeServiceMock()->create($params, false); + $this->getLeaveRequestServiceWhenCurrentUserIsAdminWithBalanceChangeServiceMock()->create($params, FALSE); } private function getLeaveRequestServiceWhenStatusTransitionIsNotAllowed() { - return $this->getLeaveRequestService(false, false, false); + return $this->getLeaveRequestService(FALSE, FALSE, FALSE); } private function getLeaveRequestServiceWhenCurrentUserIsAdmin() { - return $this->getLeaveRequestService(true, false); + return $this->getLeaveRequestService(TRUE, FALSE); } private function getLeaveRequestServiceWhenCurrentUserIsLeaveManager() { - return $this->getLeaveRequestService(false, true); + return $this->getLeaveRequestService(FALSE, TRUE); } private function getLeaveRequestServiceWhenCurrentUserIsAdminWithBalanceChangeServiceMock() { - return $this->getLeaveRequestService(true, false, true, true); + return $this->getLeaveRequestService(TRUE, FALSE, TRUE, TRUE); } private function getDefaultParams($params = []) { $absenceType = AbsenceTypeFabricator::fabricate(); @@ -570,7 +570,7 @@ public function testBalanceChangeIsUpdatedForAnExistingLeaveRequestWhenChangeBal 'end_date' => CRM_Utils_Date::processDate('2016-12-31'), ]); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); //Leave dates on Monday to Friday, all working days $leaveDates = [ @@ -579,7 +579,7 @@ public function testBalanceChangeIsUpdatedForAnExistingLeaveRequestWhenChangeBal ]; $params = $this->getDefaultParams($leaveDates); - $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, true); + $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, TRUE); //Just to make sure that we have the expected balance change for the leave request $previousBalance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($leaveRequest); @@ -614,7 +614,7 @@ public function testBalanceChangeIsUpdatedForAnExistingLeaveRequestWhenChangeBal $leaveRequest = $this->getleaveRequestService()->create( $params, - false + FALSE ); $balance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($leaveRequest); @@ -632,7 +632,7 @@ public function testBalanceChangeIsNotUpdatedForAnExistingLeaveRequestWhenChange 'end_date' => CRM_Utils_Date::processDate('2016-12-31'), ]); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); //Leave dates on Monday to Friday, all working days $leaveDates = [ @@ -641,7 +641,7 @@ public function testBalanceChangeIsNotUpdatedForAnExistingLeaveRequestWhenChange ]; $params = $this->getDefaultParams($leaveDates); - $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, true); + $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, TRUE); //Just to make sure that we have the expected balance change for the leave request $previousBalance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($leaveRequest); @@ -676,7 +676,7 @@ public function testBalanceChangeIsNotUpdatedForAnExistingLeaveRequestWhenChange $leaveRequest = $this->getleaveRequestService()->create( $params, - false + FALSE ); $balance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($leaveRequest); @@ -694,7 +694,7 @@ public function testBalanceChangeIsUpdatedForAnExistingLeaveRequestWhenChangeBal 'end_date' => CRM_Utils_Date::processDate('2016-12-31'), ]); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); //Leave dates on Monday to Friday, all working days $leaveDates = [ @@ -703,7 +703,7 @@ public function testBalanceChangeIsUpdatedForAnExistingLeaveRequestWhenChangeBal ]; $params = $this->getDefaultParams($leaveDates); - $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, true); + $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, TRUE); //Just to make sure that we have the expected balance change for the leave request $previousBalance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($leaveRequest); @@ -749,7 +749,7 @@ public function testBalanceChangeIsUpdatedForAnExistingLeaveRequestWhenChangeBal $leaveRequest = $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create( $params, - false + FALSE ); //The leave request balance has been updated to pick from the current work pattern @@ -768,7 +768,7 @@ public function testBalanceChangeIsUpdatedForAnExistingLeaveRequestWhenChangeBal 'end_date' => CRM_Utils_Date::processDate('2016-12-31'), ]); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); //Leave dates on Monday to Friday, all working days $leaveDates = [ @@ -777,7 +777,7 @@ public function testBalanceChangeIsUpdatedForAnExistingLeaveRequestWhenChangeBal ]; $params = $this->getDefaultParams($leaveDates); - $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, true); + $leaveRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, TRUE); //Just to make sure that we have the expected balance change for the leave request $previousBalance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($leaveRequest); @@ -823,7 +823,7 @@ public function testBalanceChangeIsUpdatedForAnExistingLeaveRequestWhenChangeBal $leaveRequest = $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create( $params, - false + FALSE ); //The leave request balance has been updated to pick from the current work pattern @@ -843,7 +843,7 @@ public function testBalanceIsUpdatedForExistingToilWhenChangeBalanceIsFalseAndTo 'end_date' => CRM_Utils_Date::processDate('2016-12-31'), ]); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); $toilToAccrue1 = 1; $toilParams = [ @@ -852,7 +852,7 @@ public function testBalanceIsUpdatedForExistingToilWhenChangeBalanceIsFalseAndTo ]; $params = $this->getDefaultParams($toilParams); - $toilRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, true); + $toilRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, TRUE); //Just to make sure that we have the expected balance change for the toil $previousBalance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($toilRequest); @@ -866,7 +866,7 @@ public function testBalanceIsUpdatedForExistingToilWhenChangeBalanceIsFalseAndTo $toilRequest = $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create( $params, - false + FALSE ); //Balance change is updated for the TOIL @@ -885,7 +885,7 @@ public function testBalanceIsUpdatedForExistingToilWhenChangeBalanceIsTrueAndToi 'end_date' => CRM_Utils_Date::processDate('2016-12-31'), ]); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); $toilToAccrue1 = 1; $toilParams = [ @@ -894,7 +894,7 @@ public function testBalanceIsUpdatedForExistingToilWhenChangeBalanceIsTrueAndToi ]; $params = $this->getDefaultParams($toilParams); - $toilRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, true); + $toilRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, TRUE); //Just to make sure that we have the expected balance change for the toil $previousBalance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($toilRequest); @@ -908,7 +908,7 @@ public function testBalanceIsUpdatedForExistingToilWhenChangeBalanceIsTrueAndToi $toilRequest = $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create( $params, - false + FALSE ); //Balance change is updated for the TOIL @@ -927,7 +927,7 @@ public function testBalanceRemainsSameButDatesAreUpdatedForToilWhenChangeBalance 'end_date' => CRM_Utils_Date::processDate('2016-12-31'), ]); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); $toilToAccrue1 = 1; $toilParams = [ @@ -938,7 +938,7 @@ public function testBalanceRemainsSameButDatesAreUpdatedForToilWhenChangeBalance ]; $params = $this->getDefaultParams($toilParams); - $toilRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, true); + $toilRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, TRUE); //Just to make sure that we have the expected balance change for the toil $previousBalance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($toilRequest); @@ -957,7 +957,7 @@ public function testBalanceRemainsSameButDatesAreUpdatedForToilWhenChangeBalance $toilRequest = $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create( $params, - false + FALSE ); $balance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($toilRequest); @@ -978,7 +978,7 @@ public function testBalanceRemainsSameButDatesAreUpdatedForToilWhenChangeBalance 'end_date' => CRM_Utils_Date::processDate('2016-12-31'), ]); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); $toilToAccrue1 = 1; $toilParams = [ @@ -989,7 +989,7 @@ public function testBalanceRemainsSameButDatesAreUpdatedForToilWhenChangeBalance ]; $params = $this->getDefaultParams($toilParams); - $toilRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, true); + $toilRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, TRUE); //Just to make sure that we have the expected balance change for the toil $previousBalance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($toilRequest); @@ -1008,7 +1008,7 @@ public function testBalanceRemainsSameButDatesAreUpdatedForToilWhenChangeBalance $toilRequest = $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create( $params, - false + FALSE ); $balance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($toilRequest); @@ -1029,7 +1029,7 @@ public function testBalanceAndDatesNotUpdatedForExistingToilWhenChangeBalanceIsF 'end_date' => CRM_Utils_Date::processDate('2016-12-31'), ]); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); $toilToAccrue1 = 1; $toilParams = [ @@ -1040,7 +1040,7 @@ public function testBalanceAndDatesNotUpdatedForExistingToilWhenChangeBalanceIsF ]; $params = $this->getDefaultParams($toilParams); - $toilRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, true); + $toilRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, TRUE); //Just to make sure that we have the expected balance change for the toil $previousBalance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($toilRequest); @@ -1052,7 +1052,7 @@ public function testBalanceAndDatesNotUpdatedForExistingToilWhenChangeBalanceIsF $toilRequest = $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create( $params, - false + FALSE ); //Both the dates and balance changes remain the same. @@ -1074,7 +1074,7 @@ public function testBalanceAndDatesRemainsSameForExistingToilWhenChangeBalanceIs 'end_date' => CRM_Utils_Date::processDate('2016-12-31'), ]); - WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => true]); + WorkPatternFabricator::fabricateWithA40HourWorkWeek(['is_default' => TRUE]); $toilToAccrue1 = 1; $toilParams = [ @@ -1085,7 +1085,7 @@ public function testBalanceAndDatesRemainsSameForExistingToilWhenChangeBalanceIs ]; $params = $this->getDefaultParams($toilParams); - $toilRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, true); + $toilRequest = LeaveRequestFabricator::fabricateWithoutValidation($params, TRUE); //Just to make sure that we have the expected balance change for the toil $previousBalance = LeaveBalanceChange::getTotalBalanceChangeForLeaveRequest($toilRequest); @@ -1097,7 +1097,7 @@ public function testBalanceAndDatesRemainsSameForExistingToilWhenChangeBalanceIs $toilRequest = $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create( $params, - false + FALSE ); //Both the dates and balance changes remain the same. @@ -1115,14 +1115,14 @@ public function testGetBreakdownIncludeOnlyTheLeaveBalanceChangesOfTheLeaveReque 'type_id' => 1, 'from_date' => CRM_Utils_Date::processDate('2016-01-01'), 'to_date' => CRM_Utils_Date::processDate('2016-01-02'), - ], true); + ], TRUE); $leaveRequest2 = LeaveRequestFabricator::fabricateWithoutValidation([ 'contact_id' => 1, 'type_id' => 1, 'from_date' => CRM_Utils_Date::processDate('2016-01-03'), 'to_date' => CRM_Utils_Date::processDate('2016-01-03'), - ], true); + ], TRUE); $expectedBreakdown = $this->getExpectedBreakdownForLeaveRequest($leaveRequest1); $breakdown = $this->getLeaveRequestService()->getBreakdown($leaveRequest1->id); @@ -1135,8 +1135,8 @@ public function testGetBreakdownIncludeOnlyTheLeaveBalanceChangesOfTheLeaveReque public function testToilRequestWithPastDatesCanNotBeCancelledWhenUserIsLeaveContactAndAbsenceTypeDoesNotAllowPastAccrual() { $absenceType = AbsenceTypeFabricator::fabricate([ - 'allow_accruals_request' => true, - 'allow_accrue_in_the_past' => false, + 'allow_accruals_request' => TRUE, + 'allow_accrue_in_the_past' => FALSE, 'allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_ALWAYS ]); @@ -1158,13 +1158,13 @@ public function testToilRequestWithPastDatesCanNotBeCancelledWhenUserIsLeaveCont $params['id'] = $toilRequest->id; $this->setExpectedException('RuntimeException', 'You may only cancel TOIL with dates in the future.'); - $this->getLeaveRequestService()->create($params, false); + $this->getLeaveRequestService()->create($params, FALSE); } public function testToilRequestWithPastDatesCanBeCancelledWhenUserIsAdminAndAbsenceTypeDoesNotAllowPastAccrual() { $absenceType = AbsenceTypeFabricator::fabricate([ - 'allow_accruals_request' => true, - 'allow_accrue_in_the_past' => false + 'allow_accruals_request' => TRUE, + 'allow_accrue_in_the_past' => FALSE ]); $leaveStatuses = LeaveRequest::getStatuses(); @@ -1184,7 +1184,7 @@ public function testToilRequestWithPastDatesCanBeCancelledWhenUserIsAdminAndAbse $params['status_id'] = $leaveStatuses['cancelled']; $params['id'] = $toilRequest->id; - $toilRequest = $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create($params, false); + $toilRequest = $this->getLeaveRequestServiceWhenCurrentUserIsAdmin()->create($params, FALSE); $this->assertNotNull($toilRequest->id); $this->assertEquals($toilRequest->status_id, $leaveStatuses['cancelled']); @@ -1192,8 +1192,8 @@ public function testToilRequestWithPastDatesCanBeCancelledWhenUserIsAdminAndAbse public function testToilRequestWithPastDatesCanBeCancelledWhenUserIsManagerAndAbsenceTypeDoesNotAllowPastAccrual() { $absenceType = AbsenceTypeFabricator::fabricate([ - 'allow_accruals_request' => true, - 'allow_accrue_in_the_past' => false + 'allow_accruals_request' => TRUE, + 'allow_accrue_in_the_past' => FALSE ]); $leaveStatuses = LeaveRequest::getStatuses(); @@ -1215,7 +1215,7 @@ public function testToilRequestWithPastDatesCanBeCancelledWhenUserIsManagerAndAb $toilRequest = $this->getLeaveRequestServiceWhenCurrentUserIsLeaveManager()->create( $params, - false + FALSE ); $this->assertNotNull($toilRequest->id); @@ -1224,8 +1224,8 @@ public function testToilRequestWithPastDatesCanBeCancelledWhenUserIsManagerAndAb public function testToilRequestWithPastDatesCanBeCancelledWhenAbsenceTypeAllowsPastAccrual() { $absenceType = AbsenceTypeFabricator::fabricate([ - 'allow_accruals_request' => true, - 'allow_accrue_in_the_past' => true, + 'allow_accruals_request' => TRUE, + 'allow_accrue_in_the_past' => TRUE, 'allow_request_cancelation' => AbsenceType::REQUEST_CANCELATION_ALWAYS ]); @@ -1248,7 +1248,7 @@ public function testToilRequestWithPastDatesCanBeCancelledWhenAbsenceTypeAllowsP $toilRequest = $this->getLeaveRequestService()->create( $params, - false + FALSE ); $this->assertNotNull($toilRequest->id); @@ -1275,10 +1275,10 @@ public function testCreateThrowsAnExceptionWhenUserIsNotAllowedToCancelAbsenceTy RuntimeException::class, 'You cannot cancel leave requests for this Absence type' ); - $leaveRequestService->create($params, false); + $leaveRequestService->create($params, FALSE); } - private function getExpectedBreakdownForLeaveRequest(LeaveRequest $leaveRequest, $amount = false) { + private function getExpectedBreakdownForLeaveRequest(LeaveRequest $leaveRequest, $amount = FALSE) { $leaveRequestDayTypes = LeaveRequest::buildOptions('from_date_type'); $dates = $leaveRequest->getDates(); From 38f18086e9896dfad87432e1a9d70f405af6e84d Mon Sep 17 00:00:00 2001 From: Tunbola Ogunwande Date: Tue, 31 Jul 2018 17:01:38 +0100 Subject: [PATCH 6/6] PCHR-4051: Refactor method for clarity. --- .../CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php b/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php index 1e7d29c83ac..c07ce43b3c9 100644 --- a/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php +++ b/uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/Service/LeaveRequestRights.php @@ -232,9 +232,9 @@ public function canCancelForAbsenceType($absenceTypeId, $contactId, DateTime $le $today = new DateTime('today'); - $absenceTypeRequestCancellationNotNo = - $absenceType->allow_request_cancelation != AbsenceType::REQUEST_CANCELATION_NO; - if ($absenceTypeRequestCancellationNotNo && $leaveFromDate > $today) { + $absenceTypeAllowsFutureCancellation = + $absenceType->allow_request_cancelation == AbsenceType::REQUEST_CANCELATION_IN_ADVANCE_OF_START_DATE; + if ($absenceTypeAllowsFutureCancellation && $leaveFromDate > $today) { return TRUE; }