GitHub PullRequestReviewCommentEvent at 2014-09-06 17:00:03
Repository: acquia_lift
Actor: katbailey
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/acquia/acquia_lift/pulls/comments/17209982","id":17209982,"diff_hunk":"@@ -1284,14 +1295,64 @@ class AcquiaLiftABReport extends AcquiaLiftReportBase {\n    */\n   public function buildCampaignReports($decision_name = NULL, $date_from = NULL, $date_to = NULL) {\n     $report_data = $this->loadReportData($decision_name, $date_from, $date_to);\n-    $reports = array(\n-      'metrics' => $this->buildABMetricsReport($report_data),\n-      'summary' => $this->buildSummaryReport($report_data),\n-    );\n-    if (!is_array($reports['metrics']) || !is_array($reports['summary'])) {\n+    $confidence = $report_data['confidence']['overview']['confidence'];\n+    $winner = $report_data['confidence']['overview']['winner'];\n+    $reports = array();\n+    $reports['all'] = $this->buildConfidenceReports(array(\n+      'name' => t('All goals'),\n+      'detail' => $report_data['confidence_detail'],\n+      'summary' => $report_data['confidence'],\n+    ), $confidence, $winner);\n+    foreach ($report_data['goals'] as $goal_id => $goal_data) {\n+      $reports['goals'][$goal_id] = $this->buildConfidenceReports($goal_data, $confidence, $winner);\n+    }\n+    if ($reports['all'] == FALSE) {\n       drupal_set_message(t('There was a problem retrieving the report data.  Please try again later.'), 'error');\n     }\n-    return $reports;\n+    $build['reports'] = array(\n+      '#type' => 'container',\n+      '#attributes' => array(\n+        'class' => array('lift-statistics'),\n+      ),\n+    );\n+    $build['reports']['all'] = $reports['all'];\n+    $build['reports']['goals'] = $reports['goals'];\n+    return $build;\n+  }\n+\n+  /**\n+   * Handles all of the logic to load and extract a confidence report for an\n+   * A/B test.\n+   *\n+   * @param $report_data\n+   *   The array of existing report data.\n+   * @param $detail\n+   *   True if the report should be a detailed report and false for summary.\n+   * @param array $options\n+   *   An array of report options such as a goal for limiting.\n+   * @return array|bool\n+   *   An array of loaded and extracted data if the data can be loaded.\n+   *   True if the data already exists\n+   *   False if the data loaded in error.\n+   */\n+  protected function loadConfidenceReportHelper($report_data, $detail, $options = array()) {\n+    if ($detail) {\n+      $options = array_merge($options, $this->getConfidenceDetailReportOptions());\n+    }\n+    $report_name = $this->getConfidenceReportRawName($options);\n+    if (isset($report_data['raw'][$report_name])) {\n+      return TRUE;","path":"plugins/agent_types/AcquiaLiftAgent.inc","position":132,"original_position":132,"commit_id":"a842f4bc8e5477577c712a81d5ada5e378e33777","original_commit_id":"a842f4bc8e5477577c712a81d5ada5e378e33777","user":{"login":"katbailey","id":58409,"avatar_url":"https://avatars.githubusercontent.com/u/58409?v=2","gravatar_id":"91bac2492f99dec8f695c8603be6ab22","url":"https://api.github.com/users/katbailey","html_url":"https://github.com/katbailey","followers_url":"https://api.github.com/users/katbailey/followers","following_url":"https://api.github.com/users/katbailey/following{/other_user}","gists_url":"https://api.github.com/users/katbailey/gists{/gist_id}","starred_url":"https://api.github.com/users/katbailey/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/katbailey/subscriptions","organizations_url":"https://api.github.com/users/katbailey/orgs","repos_url":"https://api.github.com/users/katbailey/repos","events_url":"https://api.github.com/users/katbailey/events{/privacy}","received_events_url":"https://api.github.com/users/katbailey/received_events","type":"User","site_admin":false},"body":"I know we already have lots of methods where we return *either* an array *or* boolean FALSE if nothing can be returned but that's generally not good practice and we really shouldn't do it. And here we return an array *or* TRUE *or* FALSE. I know this is just a helper method, not part of the public interface, and I'm being overly picky - but it seems we could just refactor things slightly so that the calling code gets the report name and checks for its existence in the report data before calling this method. Maybe with the merging of options it becomes trickier, so feel free to respond with a big \"MEH!\" ;)","created_at":"2014-09-06T17:00:03Z","updated_at":"2014-09-06T17:00:03Z","html_url":"https://github.com/acquia/acquia_lift/pull/45#discussion_r17209982","pull_request_url":"https://api.github.com/repos/acquia/acquia_lift/pulls/45","_links":{"self":{"href":"https://api.github.com/repos/acquia/acquia_lift/pulls/comments/17209982"},"html":{"href":"https://github.com/acquia/acquia_lift/pull/45#discussion_r17209982"},"pull_request":{"href":"https://api.github.com/repos/acquia/acquia_lift/pulls/45"}}},"pull_request":{"url":"https://api.github.com/repos/acquia/acquia_lift/pulls/45","id":20689904,"html_url":"https://github.com/acquia/acquia_lift/pull/45","diff_url":"https://github.com/acquia/acquia_lift/pull/45.diff","patch_url":"https://github.com/acquia/acquia_lift/pull/45.patch","issue_url":"https://api.github.com/repos/acquia/acquia_lift/issues/45","number":45,"state":"open","locked":false,"title":"Ch 830: Ability to split reports by goal and by metric","user":{"login":"lkhaas","id":3512988,"avatar_url":"https://avatars.githubusercontent.com/u/3512988?v=2","gravatar_id":"223242be828faa0385a2ddbdb976ccbc","url":"https://api.github.com/users/lkhaas","html_url":"https://github.com/lkhaas","followers_url":"https://api.github.com/users/lkhaas/followers","following_url":"https://api.github.com/users/lkhaas/following{/other_user}","gists_url":"https://api.github.com/users/lkhaas/gists{/gist_id}","starred_url":"https://api.github.com/users/lkhaas/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lkhaas/subscriptions","organizations_url":"https://api.github.com/users/lkhaas/orgs","repos_url":"https://api.github.com/users/lkhaas/repos","events_url":"https://api.github.com/users/lkhaas/events{/privacy}","received_events_url":"https://api.github.com/users/lkhaas/received_events","type":"User","site_admin":false},"body":"","created_at":"2014-09-03T16:46:25Z","updated_at":"2014-09-06T17:00:03Z","closed_at":null,"merged_at":null,"merge_commit_sha":"00cf40d879bbd6d6d4615617b53db7c0c70a2031","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/acquia/acquia_lift/pulls/45/commits","review_comments_url":"https://api.github.com/repos/acquia/acquia_lift/pulls/45/comments","review_comment_url":"https://api.github.com/repos/acquia/acquia_lift/pulls/comments/{number}","comments_url":"https://api.github.com/repos/acquia/acquia_lift/issues/45/comments","statuses_url":"https://api.github.com/repos/acquia/acquia_lift/statuses/a842f4bc8e5477577c712a81d5ada5e378e33777","head":{"label":"acquia:CH-830","ref":"CH-830","sha":"a842f4bc8e5477577c712a81d5ada5e378e33777","user":{"login":"acquia","id":514789,"avatar_url":"https://avatars.githubusercontent.com/u/514789?v=2","gravatar_id":"62eace84babd797b1c35b1ce3ccecd6e","url":"https://api.github.com/users/acquia","html_url":"https://github.com/acquia","followers_url":"https://api.github.com/users/acquia/followers","following_url":"https://api.github.com/users/acquia/following{/other_user}","gists_url":"https://api.github.com/users/acquia/gists{/gist_id}","starred_url":"https://api.github.com/users/acquia/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/acquia/subscriptions","organizations_url":"https://api.github.com/users/acquia/orgs","repos_url":"https://api.github.com/users/acquia/repos","events_url":"https://api.github.com/users/acquia/events{/privacy}","received_events_url":"https://api.github.com/users/acquia/received_events","type":"Organization","site_admin":false},"repo":{"id":17344049,"name":"acquia_lift","full_name":"acquia/acquia_lift","owner":{"login":"acquia","id":514789,"avatar_url":"https://avatars.githubusercontent.com/u/514789?v=2","gravatar_id":"62eace84babd797b1c35b1ce3ccecd6e","url":"https://api.github.com/users/acquia","html_url":"https://github.com/acquia","followers_url":"https://api.github.com/users/acquia/followers","following_url":"https://api.github.com/users/acquia/following{/other_user}","gists_url":"https://api.github.com/users/acquia/gists{/gist_id}","starred_url":"https://api.github.com/users/acquia/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/acquia/subscriptions","organizations_url":"https://api.github.com/users/acquia/orgs","repos_url":"https://api.github.com/users/acquia/repos","events_url":"https://api.github.com/users/acquia/events{/privacy}","received_events_url":"https://api.github.com/users/acquia/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/acquia/acquia_lift","description":"Mirror of the drupal.org repository for Acquia Lift module","fork":false,"url":"https://api.github.com/repos/acquia/acquia_lift","forks_url":"https://api.github.com/repos/acquia/acquia_lift/forks","keys_url":"https://api.github.com/repos/acquia/acquia_lift/keys{/key_id}","collaborators_url":"https://api.github.com/repos/acquia/acquia_lift/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/acquia/acquia_lift/teams","hooks_url":"https://api.github.com/repos/acquia/acquia_lift/hooks","issue_events_url":"https://api.github.com/repos/acquia/acquia_lift/issues/events{/number}","events_url":"https://api.github.com/repos/acquia/acquia_lift/events","assignees_url":"https://api.github.com/repos/acquia/acquia_lift/assignees{/user}","branches_url":"https://api.github.com/repos/acquia/acquia_lift/branches{/branch}","tags_url":"https://api.github.com/repos/acquia/acquia_lift/tags","blobs_url":"https://api.github.com/repos/acquia/acquia_lift/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/acquia/acquia_lift/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/acquia/acquia_lift/git/refs{/sha}","trees_url":"https://api.github.com/repos/acquia/acquia_lift/git/trees{/sha}","statuses_url":"https://api.github.com/repos/acquia/acquia_lift/statuses/{sha}","languages_url":"https://api.github.com/repos/acquia/acquia_lift/languages","stargazers_url":"https://api.github.com/repos/acquia/acquia_lift/stargazers","contributors_url":"https://api.github.com/repos/acquia/acquia_lift/contributors","subscribers_url":"https://api.github.com/repos/acquia/acquia_lift/subscribers","subscription_url":"https://api.github.com/repos/acquia/acquia_lift/subscription","commits_url":"https://api.github.com/repos/acquia/acquia_lift/commits{/sha}","git_commits_url":"https://api.github.com/repos/acquia/acquia_lift/git/commits{/sha}","comments_url":"https://api.github.com/repos/acquia/acquia_lift/comments{/number}","issue_comment_url":"https://api.github.com/repos/acquia/acquia_lift/issues/comments/{number}","contents_url":"https://api.github.com/repos/acquia/acquia_lift/contents/{+path}","compare_url":"https://api.github.com/repos/acquia/acquia_lift/compare/{base}...{head}","merges_url":"https://api.github.com/repos/acquia/acquia_lift/merges","archive_url":"https://api.github.com/repos/acquia/acquia_lift/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/acquia/acquia_lift/downloads","issues_url":"https://api.github.com/repos/acquia/acquia_lift/issues{/number}","pulls_url":"https://api.github.com/repos/acquia/acquia_lift/pulls{/number}","milestones_url":"https://api.github.com/repos/acquia/acquia_lift/milestones{/number}","notifications_url":"https://api.github.com/repos/acquia/acquia_lift/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/acquia/acquia_lift/labels{/name}","releases_url":"https://api.github.com/repos/acquia/acquia_lift/releases{/id}","created_at":"2014-03-02T18:42:14Z","updated_at":"2014-08-25T15:09:47Z","pushed_at":"2014-09-06T16:42:22Z","git_url":"git://github.com/acquia/acquia_lift.git","ssh_url":"git@github.com:acquia/acquia_lift.git","clone_url":"https://github.com/acquia/acquia_lift.git","svn_url":"https://github.com/acquia/acquia_lift","homepage":"","size":2156,"stargazers_count":2,"watchers_count":2,"language":null,"has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":1,"mirror_url":null,"open_issues_count":6,"forks":1,"open_issues":6,"watchers":2,"default_branch":"master"}},"base":{"label":"acquia:CH-774","ref":"CH-774","sha":"efc1578c0dda1f614503d4cf23afff788456954c","user":{"login":"acquia","id":514789,"avatar_url":"https://avatars.githubusercontent.com/u/514789?v=2","gravatar_id":"62eace84babd797b1c35b1ce3ccecd6e","url":"https://api.github.com/users/acquia","html_url":"https://github.com/acquia","followers_url":"https://api.github.com/users/acquia/followers","following_url":"https://api.github.com/users/acquia/following{/other_user}","gists_url":"https://api.github.com/users/acquia/gists{/gist_id}","starred_url":"https://api.github.com/users/acquia/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/acquia/subscriptions","organizations_url":"https://api.github.com/users/acquia/orgs","repos_url":"https://api.github.com/users/acquia/repos","events_url":"https://api.github.com/users/acquia/events{/privacy}","received_events_url":"https://api.github.com/users/acquia/received_events","type":"Organization","site_admin":false},"repo":{"id":17344049,"name":"acquia_lift","full_name":"acquia/acquia_lift","owner":{"login":"acquia","id":514789,"avatar_url":"https://avatars.githubusercontent.com/u/514789?v=2","gravatar_id":"62eace84babd797b1c35b1ce3ccecd6e","url":"https://api.github.com/users/acquia","html_url":"https://github.com/acquia","followers_url":"https://api.github.com/users/acquia/followers","following_url":"https://api.github.com/users/acquia/following{/other_user}","gists_url":"https://api.github.com/users/acquia/gists{/gist_id}","starred_url":"https://api.github.com/users/acquia/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/acquia/subscriptions","organizations_url":"https://api.github.com/users/acquia/orgs","repos_url":"https://api.github.com/users/acquia/repos","events_url":"https://api.github.com/users/acquia/events{/privacy}","received_events_url":"https://api.github.com/users/acquia/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/acquia/acquia_lift","description":"Mirror of the drupal.org repository for Acquia Lift module","fork":false,"url":"https://api.github.com/repos/acquia/acquia_lift","forks_url":"https://api.github.com/repos/acquia/acquia_lift/forks","keys_url":"https://api.github.com/repos/acquia/acquia_lift/keys{/key_id}","collaborators_url":"https://api.github.com/repos/acquia/acquia_lift/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/acquia/acquia_lift/teams","hooks_url":"https://api.github.com/repos/acquia/acquia_lift/hooks","issue_events_url":"https://api.github.com/repos/acquia/acquia_lift/issues/events{/number}","events_url":"https://api.github.com/repos/acquia/acquia_lift/events","assignees_url":"https://api.github.com/repos/acquia/acquia_lift/assignees{/user}","branches_url":"https://api.github.com/repos/acquia/acquia_lift/branches{/branch}","tags_url":"https://api.github.com/repos/acquia/acquia_lift/tags","blobs_url":"https://api.github.com/repos/acquia/acquia_lift/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/acquia/acquia_lift/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/acquia/acquia_lift/git/refs{/sha}","trees_url":"https://api.github.com/repos/acquia/acquia_lift/git/trees{/sha}","statuses_url":"https://api.github.com/repos/acquia/acquia_lift/statuses/{sha}","languages_url":"https://api.github.com/repos/acquia/acquia_lift/languages","stargazers_url":"https://api.github.com/repos/acquia/acquia_lift/stargazers","contributors_url":"https://api.github.com/repos/acquia/acquia_lift/contributors","subscribers_url":"https://api.github.com/repos/acquia/acquia_lift/subscribers","subscription_url":"https://api.github.com/repos/acquia/acquia_lift/subscription","commits_url":"https://api.github.com/repos/acquia/acquia_lift/commits{/sha}","git_commits_url":"https://api.github.com/repos/acquia/acquia_lift/git/commits{/sha}","comments_url":"https://api.github.com/repos/acquia/acquia_lift/comments{/number}","issue_comment_url":"https://api.github.com/repos/acquia/acquia_lift/issues/comments/{number}","contents_url":"https://api.github.com/repos/acquia/acquia_lift/contents/{+path}","compare_url":"https://api.github.com/repos/acquia/acquia_lift/compare/{base}...{head}","merges_url":"https://api.github.com/repos/acquia/acquia_lift/merges","archive_url":"https://api.github.com/repos/acquia/acquia_lift/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/acquia/acquia_lift/downloads","issues_url":"https://api.github.com/repos/acquia/acquia_lift/issues{/number}","pulls_url":"https://api.github.com/repos/acquia/acquia_lift/pulls{/number}","milestones_url":"https://api.github.com/repos/acquia/acquia_lift/milestones{/number}","notifications_url":"https://api.github.com/repos/acquia/acquia_lift/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/acquia/acquia_lift/labels{/name}","releases_url":"https://api.github.com/repos/acquia/acquia_lift/releases{/id}","created_at":"2014-03-02T18:42:14Z","updated_at":"2014-08-25T15:09:47Z","pushed_at":"2014-09-06T16:42:22Z","git_url":"git://github.com/acquia/acquia_lift.git","ssh_url":"git@github.com:acquia/acquia_lift.git","clone_url":"https://github.com/acquia/acquia_lift.git","svn_url":"https://github.com/acquia/acquia_lift","homepage":"","size":2156,"stargazers_count":2,"watchers_count":2,"language":null,"has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":1,"mirror_url":null,"open_issues_count":6,"forks":1,"open_issues":6,"watchers":2,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/acquia/acquia_lift/pulls/45"},"html":{"href":"https://github.com/acquia/acquia_lift/pull/45"},"issue":{"href":"https://api.github.com/repos/acquia/acquia_lift/issues/45"},"comments":{"href":"https://api.github.com/repos/acquia/acquia_lift/issues/45/comments"},"review_comments":{"href":"https://api.github.com/repos/acquia/acquia_lift/pulls/45/comments"},"review_comment":{"href":"https://api.github.com/repos/acquia/acquia_lift/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/acquia/acquia_lift/pulls/45/commits"},"statuses":{"href":"https://api.github.com/repos/acquia/acquia_lift/statuses/a842f4bc8e5477577c712a81d5ada5e378e33777"}}}}