{"action":"created","comment":{"url":"https://api.github.com/repos/civicrm/civihr/pulls/comments/172340121","pull_request_review_id":101341051,"id":172340121,"diff_hunk":"@@ -803,15 +803,20 @@ private static function validateAbsenceTypeIsActiveAndValid($params, $absenceTyp\n    *\n    * @param int $contactID\n    * @param \\CRM_HRLeaveAndAbsences_BAO_PublicHoliday $publicHoliday\n+   * @param boolean $nonDeletedOnly\n+   *   When true will only return result if the given public\n+   *   holiday request is not yet deleted.\n    *\n    * @return \\CRM_HRLeaveAndAbsences_BAO_LeaveRequest|null\n    */\n-  public static function findPublicHolidayLeaveRequest($contactID, PublicHoliday $publicHoliday) {\n+  public static function findPublicHolidayLeaveRequest($contactID, PublicHoliday $publicHoliday, $nonDeletedOnly = TRUE) {","path":"uk.co.compucorp.civicrm.hrleaveandabsences/CRM/HRLeaveAndAbsences/BAO/LeaveRequest.php","position":11,"original_position":11,"commit_id":"0e9f6eb656cd2e9016d9ed0751242ce00c1c3307","original_commit_id":"0e9f6eb656cd2e9016d9ed0751242ce00c1c3307","user":{"login":"tunbola","id":6951813,"avatar_url":"https://avatars1.githubusercontent.com/u/6951813?v=4","gravatar_id":"","url":"https://api.github.com/users/tunbola","html_url":"https://github.com/tunbola","followers_url":"https://api.github.com/users/tunbola/followers","following_url":"https://api.github.com/users/tunbola/following{/other_user}","gists_url":"https://api.github.com/users/tunbola/gists{/gist_id}","starred_url":"https://api.github.com/users/tunbola/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tunbola/subscriptions","organizations_url":"https://api.github.com/users/tunbola/orgs","repos_url":"https://api.github.com/users/tunbola/repos","events_url":"https://api.github.com/users/tunbola/events{/privacy}","received_events_url":"https://api.github.com/users/tunbola/received_events","type":"User","site_admin":false},"body":"That means we would have two identical methods with only a line being the difference between both methods? The parameter seems to me as a filter that filters off some records and we have many of such methods in L&A. ","created_at":"2018-03-05T21:49:15Z","updated_at":"2018-03-05T21:49:15Z","html_url":"https://github.com/civicrm/civihr/pull/2518#discussion_r172340121","pull_request_url":"https://api.github.com/repos/civicrm/civihr/pulls/2518","author_association":"COLLABORATOR","_links":{"self":{"href":"https://api.github.com/repos/civicrm/civihr/pulls/comments/172340121"},"html":{"href":"https://github.com/civicrm/civihr/pull/2518#discussion_r172340121"},"pull_request":{"href":"https://api.github.com/repos/civicrm/civihr/pulls/2518"}},"in_reply_to_id":172294809},"pull_request":{"url":"https://api.github.com/repos/civicrm/civihr/pulls/2518","id":172926848,"html_url":"https://github.com/civicrm/civihr/pull/2518","diff_url":"https://github.com/civicrm/civihr/pull/2518.diff","patch_url":"https://github.com/civicrm/civihr/pull/2518.patch","issue_url":"https://api.github.com/repos/civicrm/civihr/issues/2518","number":2518,"state":"open","locked":false,"title":"PCHR-3423: Do not allow deleted Public holiday Leave Request for Contact to be re-created","user":{"login":"tunbola","id":6951813,"avatar_url":"https://avatars1.githubusercontent.com/u/6951813?v=4","gravatar_id":"","url":"https://api.github.com/users/tunbola","html_url":"https://github.com/tunbola","followers_url":"https://api.github.com/users/tunbola/followers","following_url":"https://api.github.com/users/tunbola/following{/other_user}","gists_url":"https://api.github.com/users/tunbola/gists{/gist_id}","starred_url":"https://api.github.com/users/tunbola/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tunbola/subscriptions","organizations_url":"https://api.github.com/users/tunbola/orgs","repos_url":"https://api.github.com/users/tunbola/repos","events_url":"https://api.github.com/users/tunbola/events{/privacy}","received_events_url":"https://api.github.com/users/tunbola/received_events","type":"User","site_admin":false},"body":"## Overview\r\nCurrently when a public holiday leave request is deleted by the Admin for a contact, If a condition that triggers the creation of public holiday leave requests occurs e.g change in Work Pattern, the public holiday leave request is created for the contact again.\r\n\r\n## Before\r\nWhen a public holiday leave request is deleted by the Admin for a contact, it gets created again when a condition that triggers public holiday creation happens.\r\n\r\n## After\r\nWhen a public holiday leave request is deleted by the Admin for a contact, it does not get created again when a condition that triggers public holiday creation happens.\r\n\r\n- The Admin deletes public holiday leave requests via the `LeaveRequestService.delete` function, rather than hard delete the requests, the function was modified to now soft delete public holiday leave requests.\r\n```php\r\nif ($leaveRequest->request_type == LeaveRequest::REQUEST_TYPE_PUBLIC_HOLIDAY) {\r\n       $publicHoliday = new PublicHoliday();\r\n       $publicHoliday->date = $leaveRequest->from_date;\r\n       $this->publicHolidayLeaveRequestDeletionService->deleteForContact($leaveRequest->contact_id, $publicHoliday, TRUE);\r\n}\r\n```\r\nNote the TRUE passed as the last parameter to `publicHolidayLeaveRequestDeletionService.deleteForContact` method. This method was modified to accept an optional parameter to specify whether to soft delete or hard delete a public holiday leave request.\r\nIt is still necessary to hard delete some public holiday leave requests for example when a Work pattern changes, all future public holiday leave requests are hard deleted and then created again with the new respective balance changes.\r\n\r\n- The logic for creating public holiday leave requests was modified to check not only for the existence of a public holiday request before attempting to create a public holiday request. The logic now checks for existence of both soft deleted and non deleted public holiday leave requests, if such public holiday requests exists for the given date for the contact, the request does not get created.\r\n\r\n```php\r\n  public function createForContact($contactID, PublicHoliday $publicHoliday) {\r\n    if (!$this->absenceType) {\r\n      return;\r\n    }\r\n\r\n    $existingLeaveRequest = LeaveRequest::findPublicHolidayLeaveRequest($contactID, $publicHoliday, FALSE);\r\n    if($existingLeaveRequest) {\r\n      return;\r\n    }\r\n\r\n    $leaveRequest = $this->createLeaveRequest($contactID, $this->absenceType, $publicHoliday);\r\n    $this->createLeaveBalanceChangeRecord($leaveRequest);\r\n    $this->recalculateExpiredBalanceChange($leaveRequest);\r\n\r\n    return $leaveRequest;\r\n  }\r\n```\r\nNote the last parameter passed to the `LeaveRequest::findPublicHolidayLeaveRequest` function. The method was modified to either find all  public holiday requests or only non deleted public holiday requests via the optional parameter `nonDeletedOnly`.\r\n","created_at":"2018-03-05T17:00:32Z","updated_at":"2018-03-05T21:49:15Z","closed_at":null,"merged_at":null,"merge_commit_sha":"ac2809bb267fc219cfbf7dd89a17431b12c8e659","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":712229315,"url":"https://api.github.com/repos/civicrm/civihr/labels/ready%20to%20test","name":"ready to test","color":"0052cc","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/civicrm/civihr/pulls/2518/commits","review_comments_url":"https://api.github.com/repos/civicrm/civihr/pulls/2518/comments","review_comment_url":"https://api.github.com/repos/civicrm/civihr/pulls/comments{/number}","comments_url":"https://api.github.com/repos/civicrm/civihr/issues/2518/comments","statuses_url":"https://api.github.com/repos/civicrm/civihr/statuses/0e9f6eb656cd2e9016d9ed0751242ce00c1c3307","head":{"label":"civicrm:PCHR-3423-dont-recreate-public-holiday","ref":"PCHR-3423-dont-recreate-public-holiday","sha":"0e9f6eb656cd2e9016d9ed0751242ce00c1c3307","user":{"login":"civicrm","id":136373,"avatar_url":"https://avatars2.githubusercontent.com/u/136373?v=4","gravatar_id":"","url":"https://api.github.com/users/civicrm","html_url":"https://github.com/civicrm","followers_url":"https://api.github.com/users/civicrm/followers","following_url":"https://api.github.com/users/civicrm/following{/other_user}","gists_url":"https://api.github.com/users/civicrm/gists{/gist_id}","starred_url":"https://api.github.com/users/civicrm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/civicrm/subscriptions","organizations_url":"https://api.github.com/users/civicrm/orgs","repos_url":"https://api.github.com/users/civicrm/repos","events_url":"https://api.github.com/users/civicrm/events{/privacy}","received_events_url":"https://api.github.com/users/civicrm/received_events","type":"Organization","site_admin":false},"repo":{"id":10086903,"name":"civihr","full_name":"civicrm/civihr","owner":{"login":"civicrm","id":136373,"avatar_url":"https://avatars2.githubusercontent.com/u/136373?v=4","gravatar_id":"","url":"https://api.github.com/users/civicrm","html_url":"https://github.com/civicrm","followers_url":"https://api.github.com/users/civicrm/followers","following_url":"https://api.github.com/users/civicrm/following{/other_user}","gists_url":"https://api.github.com/users/civicrm/gists{/gist_id}","starred_url":"https://api.github.com/users/civicrm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/civicrm/subscriptions","organizations_url":"https://api.github.com/users/civicrm/orgs","repos_url":"https://api.github.com/users/civicrm/repos","events_url":"https://api.github.com/users/civicrm/events{/privacy}","received_events_url":"https://api.github.com/users/civicrm/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/civicrm/civihr","description":"CiviHR","fork":false,"url":"https://api.github.com/repos/civicrm/civihr","forks_url":"https://api.github.com/repos/civicrm/civihr/forks","keys_url":"https://api.github.com/repos/civicrm/civihr/keys{/key_id}","collaborators_url":"https://api.github.com/repos/civicrm/civihr/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/civicrm/civihr/teams","hooks_url":"https://api.github.com/repos/civicrm/civihr/hooks","issue_events_url":"https://api.github.com/repos/civicrm/civihr/issues/events{/number}","events_url":"https://api.github.com/repos/civicrm/civihr/events","assignees_url":"https://api.github.com/repos/civicrm/civihr/assignees{/user}","branches_url":"https://api.github.com/repos/civicrm/civihr/branches{/branch}","tags_url":"https://api.github.com/repos/civicrm/civihr/tags","blobs_url":"https://api.github.com/repos/civicrm/civihr/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/civicrm/civihr/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/civicrm/civihr/git/refs{/sha}","trees_url":"https://api.github.com/repos/civicrm/civihr/git/trees{/sha}","statuses_url":"https://api.github.com/repos/civicrm/civihr/statuses/{sha}","languages_url":"https://api.github.com/repos/civicrm/civihr/languages","stargazers_url":"https://api.github.com/repos/civicrm/civihr/stargazers","contributors_url":"https://api.github.com/repos/civicrm/civihr/contributors","subscribers_url":"https://api.github.com/repos/civicrm/civihr/subscribers","subscription_url":"https://api.github.com/repos/civicrm/civihr/subscription","commits_url":"https://api.github.com/repos/civicrm/civihr/commits{/sha}","git_commits_url":"https://api.github.com/repos/civicrm/civihr/git/commits{/sha}","comments_url":"https://api.github.com/repos/civicrm/civihr/comments{/number}","issue_comment_url":"https://api.github.com/repos/civicrm/civihr/issues/comments{/number}","contents_url":"https://api.github.com/repos/civicrm/civihr/contents/{+path}","compare_url":"https://api.github.com/repos/civicrm/civihr/compare/{base}...{head}","merges_url":"https://api.github.com/repos/civicrm/civihr/merges","archive_url":"https://api.github.com/repos/civicrm/civihr/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/civicrm/civihr/downloads","issues_url":"https://api.github.com/repos/civicrm/civihr/issues{/number}","pulls_url":"https://api.github.com/repos/civicrm/civihr/pulls{/number}","milestones_url":"https://api.github.com/repos/civicrm/civihr/milestones{/number}","notifications_url":"https://api.github.com/repos/civicrm/civihr/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/civicrm/civihr/labels{/name}","releases_url":"https://api.github.com/repos/civicrm/civihr/releases{/id}","deployments_url":"https://api.github.com/repos/civicrm/civihr/deployments","created_at":"2013-05-15T20:06:06Z","updated_at":"2018-02-07T06:48:22Z","pushed_at":"2018-03-05T17:38:14Z","git_url":"git://github.com/civicrm/civihr.git","ssh_url":"git@github.com:civicrm/civihr.git","clone_url":"https://github.com/civicrm/civihr.git","svn_url":"https://github.com/civicrm/civihr","homepage":null,"size":62070,"stargazers_count":26,"watchers_count":26,"language":"PHP","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":38,"mirror_url":null,"archived":false,"open_issues_count":14,"license":null,"forks":38,"open_issues":14,"watchers":26,"default_branch":"staging"}},"base":{"label":"civicrm:staging","ref":"staging","sha":"aae3ec6712090268ea2b62e1df97e9a3ba2a5433","user":{"login":"civicrm","id":136373,"avatar_url":"https://avatars2.githubusercontent.com/u/136373?v=4","gravatar_id":"","url":"https://api.github.com/users/civicrm","html_url":"https://github.com/civicrm","followers_url":"https://api.github.com/users/civicrm/followers","following_url":"https://api.github.com/users/civicrm/following{/other_user}","gists_url":"https://api.github.com/users/civicrm/gists{/gist_id}","starred_url":"https://api.github.com/users/civicrm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/civicrm/subscriptions","organizations_url":"https://api.github.com/users/civicrm/orgs","repos_url":"https://api.github.com/users/civicrm/repos","events_url":"https://api.github.com/users/civicrm/events{/privacy}","received_events_url":"https://api.github.com/users/civicrm/received_events","type":"Organization","site_admin":false},"repo":{"id":10086903,"name":"civihr","full_name":"civicrm/civihr","owner":{"login":"civicrm","id":136373,"avatar_url":"https://avatars2.githubusercontent.com/u/136373?v=4","gravatar_id":"","url":"https://api.github.com/users/civicrm","html_url":"https://github.com/civicrm","followers_url":"https://api.github.com/users/civicrm/followers","following_url":"https://api.github.com/users/civicrm/following{/other_user}","gists_url":"https://api.github.com/users/civicrm/gists{/gist_id}","starred_url":"https://api.github.com/users/civicrm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/civicrm/subscriptions","organizations_url":"https://api.github.com/users/civicrm/orgs","repos_url":"https://api.github.com/users/civicrm/repos","events_url":"https://api.github.com/users/civicrm/events{/privacy}","received_events_url":"https://api.github.com/users/civicrm/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/civicrm/civihr","description":"CiviHR","fork":false,"url":"https://api.github.com/repos/civicrm/civihr","forks_url":"https://api.github.com/repos/civicrm/civihr/forks","keys_url":"https://api.github.com/repos/civicrm/civihr/keys{/key_id}","collaborators_url":"https://api.github.com/repos/civicrm/civihr/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/civicrm/civihr/teams","hooks_url":"https://api.github.com/repos/civicrm/civihr/hooks","issue_events_url":"https://api.github.com/repos/civicrm/civihr/issues/events{/number}","events_url":"https://api.github.com/repos/civicrm/civihr/events","assignees_url":"https://api.github.com/repos/civicrm/civihr/assignees{/user}","branches_url":"https://api.github.com/repos/civicrm/civihr/branches{/branch}","tags_url":"https://api.github.com/repos/civicrm/civihr/tags","blobs_url":"https://api.github.com/repos/civicrm/civihr/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/civicrm/civihr/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/civicrm/civihr/git/refs{/sha}","trees_url":"https://api.github.com/repos/civicrm/civihr/git/trees{/sha}","statuses_url":"https://api.github.com/repos/civicrm/civihr/statuses/{sha}","languages_url":"https://api.github.com/repos/civicrm/civihr/languages","stargazers_url":"https://api.github.com/repos/civicrm/civihr/stargazers","contributors_url":"https://api.github.com/repos/civicrm/civihr/contributors","subscribers_url":"https://api.github.com/repos/civicrm/civihr/subscribers","subscription_url":"https://api.github.com/repos/civicrm/civihr/subscription","commits_url":"https://api.github.com/repos/civicrm/civihr/commits{/sha}","git_commits_url":"https://api.github.com/repos/civicrm/civihr/git/commits{/sha}","comments_url":"https://api.github.com/repos/civicrm/civihr/comments{/number}","issue_comment_url":"https://api.github.com/repos/civicrm/civihr/issues/comments{/number}","contents_url":"https://api.github.com/repos/civicrm/civihr/contents/{+path}","compare_url":"https://api.github.com/repos/civicrm/civihr/compare/{base}...{head}","merges_url":"https://api.github.com/repos/civicrm/civihr/merges","archive_url":"https://api.github.com/repos/civicrm/civihr/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/civicrm/civihr/downloads","issues_url":"https://api.github.com/repos/civicrm/civihr/issues{/number}","pulls_url":"https://api.github.com/repos/civicrm/civihr/pulls{/number}","milestones_url":"https://api.github.com/repos/civicrm/civihr/milestones{/number}","notifications_url":"https://api.github.com/repos/civicrm/civihr/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/civicrm/civihr/labels{/name}","releases_url":"https://api.github.com/repos/civicrm/civihr/releases{/id}","deployments_url":"https://api.github.com/repos/civicrm/civihr/deployments","created_at":"2013-05-15T20:06:06Z","updated_at":"2018-02-07T06:48:22Z","pushed_at":"2018-03-05T17:38:14Z","git_url":"git://github.com/civicrm/civihr.git","ssh_url":"git@github.com:civicrm/civihr.git","clone_url":"https://github.com/civicrm/civihr.git","svn_url":"https://github.com/civicrm/civihr","homepage":null,"size":62070,"stargazers_count":26,"watchers_count":26,"language":"PHP","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":38,"mirror_url":null,"archived":false,"open_issues_count":14,"license":null,"forks":38,"open_issues":14,"watchers":26,"default_branch":"staging"}},"_links":{"self":{"href":"https://api.github.com/repos/civicrm/civihr/pulls/2518"},"html":{"href":"https://github.com/civicrm/civihr/pull/2518"},"issue":{"href":"https://api.github.com/repos/civicrm/civihr/issues/2518"},"comments":{"href":"https://api.github.com/repos/civicrm/civihr/issues/2518/comments"},"review_comments":{"href":"https://api.github.com/repos/civicrm/civihr/pulls/2518/comments"},"review_comment":{"href":"https://api.github.com/repos/civicrm/civihr/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/civicrm/civihr/pulls/2518/commits"},"statuses":{"href":"https://api.github.com/repos/civicrm/civihr/statuses/0e9f6eb656cd2e9016d9ed0751242ce00c1c3307"}},"author_association":"COLLABORATOR"}}
{"actor":{"display_login":"tunbola"}}