diff --git a/src/App/Projects/TrackerProject.php b/src/App/Projects/TrackerProject.php index b4707fabd..0b2be9f21 100644 --- a/src/App/Projects/TrackerProject.php +++ b/src/App/Projects/TrackerProject.php @@ -360,7 +360,7 @@ public function getMilestones() $milestones = $db ->setQuery( $db->getQuery(true) ->from($db->quoteName($table->getTableName())) - ->select(array('milestone_id', 'title', 'description', 'state', 'due_on')) + ->select(array('milestone_id', 'milestone_number', 'title', 'description', 'state', 'due_on')) ->where($db->quoteName('project_id') . ' = ' . $this->project_id) ->order($db->quoteName('milestone_number')) )->loadObjectList(); diff --git a/src/App/Tracker/Controller/Issue/Save.php b/src/App/Tracker/Controller/Issue/Save.php index 4d3edf752..72d50dcae 100644 --- a/src/App/Tracker/Controller/Issue/Save.php +++ b/src/App/Tracker/Controller/Issue/Save.php @@ -29,12 +29,12 @@ class Save extends AbstractTrackerController /** * Execute the controller. * - * @throws \Exception - * @throws \JTracker\Authentication\Exception\AuthenticationException - * * @return string The rendered view. * * @since 1.0 + * @throws \JTracker\Authentication\Exception\AuthenticationException + * @throws \RuntimeException + * @throws \UnexpectedValueException */ public function execute() { @@ -64,6 +64,17 @@ public function execute() { // The user has full "edit" permission. $data = $src; + + // Allow admins to update labels and milestones + if (!$user->check('admin')) + { + if (!empty($item->labels)) + { + $data['labels'] = explode(',', $item->labels); + } + + $data['milestone_id'] = $item->milestone_id; + } } elseif($user->canEditOwn($item->opened_by)) { @@ -74,12 +85,18 @@ public function execute() $data['description_raw'] = $src['description_raw']; // Take the remaining values from the stored item - $data['status'] = $item->status; - $data['priority'] = $item->priority; - $data['build'] = $item->build; - $data['rel_number'] = $item->rel_number; - $data['rel_type'] = $item->rel_type; - $data['easy'] = $item->easy; + if (!empty($item->labels)) + { + $data['labels'] = explode(',', $item->labels); + } + + $data['status'] = $item->status; + $data['priority'] = $item->priority; + $data['build'] = $item->build; + $data['rel_number'] = $item->rel_number; + $data['rel_type'] = $item->rel_type; + $data['easy'] = $item->easy; + $data['milestone_id'] = $item->milestone_id; } else { @@ -93,31 +110,67 @@ public function execute() $oldState = $model->getOpenClosed($item->status); $state = $model->getOpenClosed($data['status']); + // Project is managed on GitHub if ($project->gh_user && $project->gh_project) { - // Project is managed on GitHub + // @todo assignee + $assignee = null; + + // Prepare labels + $ghLabels = []; + + if (!empty($data['labels'])) + { + foreach ($project->getLabels() as $id => $label) + { + if (in_array($id, $data['labels'])) + { + $ghLabels[] = $label->name; + } + } + } + + // Prepare milestone + $ghMilestone = null; + + if (!empty($data['milestone_id'])) + { + foreach ($project->getMilestones() as $milestone) + { + if ($milestone->milestone_id == $data['milestone_id']) + { + $ghMilestone = $milestone->milestone_number; + } + } + } try { - $gitHubResponse = $this->updateGitHub($item->issue_number, $data, $state, $oldState); + $gitHubResponse = $this->updateGitHub( + $item->issue_number, $data, $state, $oldState, $assignee, $ghMilestone, $ghLabels + ); // Set the modified_date from GitHub (important!) $data['modified_date'] = $gitHubResponse->updated_at; } catch (GithubException $exception) { - // DEBUG - @todo remove in stable - $dump = [ - 'exception' => $exception->getCode() . ' ' . $exception->getMessage(), - 'user' => $project->gh_user, 'project' => $project->gh_project, - 'issueNo' => $item->issue_number, - 'state' => $state, 'old_state' => $oldState, - 'data' => $data - ]; - - var_dump($dump); - - die('Unrecoverable GitHub error - sry ;('); + $this->getContainer()->get('app')->getLogger()->error( + sprintf( + 'Error code %1$s received from GitHub when editing an issue with the following data:' + . ' GitHub User: %2$s; GitHub Repo: %3$s; Issue Number: %4$s; State: %5$s, Old state: %6$s' + . ' The error message returned was: %7$s', + $exception->getCode(), + $project->gh_user, + $project->gh_project, + $item->issue_number, + $state, + $oldState, + $exception->getMessage() + ) + ); + + throw new \RuntimeException('Invalid response from GitHub'); } // Render the description text using GitHub's markdown renderer. @@ -188,7 +241,7 @@ public function execute() if (!isset($gitHubResponse->id)) { - throw new \Exception('Invalid response from GitHub'); + throw new \RuntimeException('Invalid response from GitHub'); } $data->created_at = $gitHubResponse->created_at; @@ -234,7 +287,7 @@ public function execute() '/tracker/' . $application->input->get('project_alias') . '/' . $issueNumber ); } - catch (\Exception $exception) + catch (\RuntimeException $exception) { $application->enqueueMessage($exception->getMessage(), 'error'); @@ -259,16 +312,18 @@ public function execute() * @param array $data The issue data. * @param string $state The issue state (either 'open' or 'closed). * @param string $oldState The previous issue state. + * @param string $assignee The login for the GitHub user that this issue should be assigned to. + * @param integer $milestone The milestone to associate this issue with. + * @param array $labels The labels to associate with this issue. * - * @throws \Exception - * @throws \JTracker\Github\Exception\GithubException + * @throws \JTracker\Github\Exception\GithubException + * @throws \RuntimeException * - * - * @return object + * @return object The issue data * * @since 1.0 */ - private function updateGitHub($issueNumber, array $data, $state, $oldState) + private function updateGitHub($issueNumber, array $data, $state, $oldState, $assignee, $milestone, $labels) { /* @type \JTracker\Application $application */ $application = $this->getContainer()->get('app'); @@ -277,11 +332,87 @@ private function updateGitHub($issueNumber, array $data, $state, $oldState) try { - // Try to update the project on GitHub using thew current user credentials - $gitHubResponse = GithubFactory::getInstance($application)->issues->edit( + // Try to perform the action on behalf of current user + $gitHub = GithubFactory::getInstance($application); + + // Look if we have a bot user configured + if ($project->getGh_Editbot_User() && $project->getGh_Editbot_Pass()) + { + // Try to perform the action on behalf of an authorized bot + $gitHubBot = GithubFactory::getInstance($application, true, $project->getGh_Editbot_User(), $project->getGh_Editbot_Pass()); + } + } + catch (\RuntimeException $exception) + { + throw new \RuntimeException('Error retrieving an instance of the Github object'); + } + + try + { + $gitHubResponse = $gitHub->issues->edit( $project->gh_user, $project->gh_project, - $issueNumber, $state, $data['title'], $data['description_raw'] + $issueNumber, $state, $data['title'], $data['description_raw'], + $assignee, $milestone, $labels ); + + $needUpdate = false; + $isAllowed = $application->getUser()->check('admin'); + + // The milestone and labels are silently dropped, + // so try to update the milestone and/or labels if they are not set. + if ((!empty($milestone) && empty($gitHubResponse->milestone) + || (!empty($milestone) && $milestone != $gitHubResponse->milestone))) + { + $needUpdate = true; + } + else + { + // Allow only specific group to reset milestone + if (!empty($gitHubResponse->milestone) && $isAllowed) + { + $milestone = ''; + $needUpdate = true; + } + } + + if (!empty($labels)) + { + if (empty($gitHubResponse->labels)) + { + $needUpdate = true; + } + + if (!empty($gitHubResponse->labels)) + { + foreach ($gitHubResponse->labels as $ghLabel) + { + // If labels differ then need to update + if (!in_array($ghLabel->name, $labels)) + { + $needUpdate = true; + + break; + } + } + } + } + else + { + // Allow only specific group to reset labels + if (!empty($gitHubResponse->labels) && $isAllowed) + { + $needUpdate = true; + } + } + + if ($needUpdate && isset($gitHubBot)) + { + $gitHubBot->issues->edit( + $project->gh_user, $project->gh_project, + $gitHubResponse->number, 'open', $data['title'], $data['description_raw'], + $assignee, $milestone, $labels + ); + } } catch (GithubException $exception) { @@ -291,21 +422,74 @@ private function updateGitHub($issueNumber, array $data, $state, $oldState) throw $exception; } - // Look if we have a bot user configured. - if (!$project->getGh_Editbot_User() || !$project->getGh_Editbot_Pass()) + if (!isset($gitHubBot)) { throw $exception; } - // Try to perform the action on behalf of an authorized bot. - $gitHubBot = GithubFactory::getInstance($application, true, $project->getGh_Editbot_User(), $project->getGh_Editbot_Pass()); - - // Update the project on GitHub $gitHubResponse = $gitHubBot->issues->edit( $project->gh_user, $project->gh_project, - $issueNumber, $state, $data['title'], $data['description_raw'] + $issueNumber, $state, $data['title'], $data['description_raw'], + $assignee, $milestone, $labels ); + $needUpdate = false; + + // The milestone and labels are silently dropped, + // so try to update the milestone and/or labels if they are not set. + if ((!empty($milestone) && empty($gitHubResponse->milestone) + || (!empty($milestone) && $milestone != $gitHubResponse->milestone))) + { + $needUpdate = true; + } + else + { + if (!empty($gitHubResponse->milestone)) + { + $milestone = ''; + $needUpdate = true; + } + } + + if (!empty($labels)) + { + if (empty($gitHubResponse->labels)) + { + $needUpdate = true; + } + + if (!empty($gitHubResponse->labels)) + { + foreach ($gitHubResponse->labels as $ghLabel) + { + // If labels differ then need to update + if (!in_array($ghLabel->name, $labels)) + { + $needUpdate = true; + + break; + } + } + } + } + else + { + if (!empty($gitHubResponse->labels)) + { + $needUpdate = true; + } + } + + // Try to update the milestone and/or labels + if ($needUpdate) + { + $gitHubBot->issues->edit( + $project->gh_user, $project->gh_project, + $gitHubResponse->number, 'open', $data['title'], $data['description_raw'], + $assignee, $milestone, $labels + ); + } + // Add a comment stating that this action has been performed by a MACHINE !! // (only if the "state" has changed - open <=> closed) if ($state != $oldState) @@ -324,7 +508,7 @@ private function updateGitHub($issueNumber, array $data, $state, $oldState) ) ); - $gitHubBot->issues->comments->create( + $gitHub->issues->comments->create( $project->gh_user, $project->gh_project, $issueNumber, $body ); @@ -333,7 +517,7 @@ private function updateGitHub($issueNumber, array $data, $state, $oldState) if (!isset($gitHubResponse->id)) { - throw new \Exception('Invalid response from GitHub'); + throw new \RuntimeException('Invalid response from GitHub'); } return $gitHubResponse; diff --git a/src/App/Tracker/Controller/Issue/Submit.php b/src/App/Tracker/Controller/Issue/Submit.php index af4307f23..3ffe7f8a9 100644 --- a/src/App/Tracker/Controller/Issue/Submit.php +++ b/src/App/Tracker/Controller/Issue/Submit.php @@ -14,6 +14,8 @@ use Joomla\Date\Date; use JTracker\Controller\AbstractTrackerController; +use JTracker\Github\Exception\GithubException; +use JTracker\Github\GithubFactory; /** * Add issues controller class. @@ -28,7 +30,7 @@ class Submit extends AbstractTrackerController * @return void * * @since 1.0 - * @throws \Exception + * @throws \RuntimeException */ public function execute() { @@ -46,53 +48,78 @@ public function execute() $body = $application->input->get('body', '', 'raw'); + if (!$body) + { + throw new \RuntimeException('No body received.'); + } + // Prepare issue for the store $data = array(); - $data['title'] = $application->input->getString('title'); + $data['title'] = $application->input->getString('title'); + $data['milestone_id'] = $application->input->getInt('milestone_id'); - if (!$body) + // Process labels + $labels = []; + + foreach ($application->input->get('labels', [], 'array') as $labelId) + { + // Filter integer + $labels[] = (int) $labelId; + } + + /** + * Store the "No code attached yet" label for CMS issue + * @todo Remove after #596 is implemented + */ + if ($project->project_id == 1 && !in_array(35, $labels)) { - throw new \Exception('No body received.'); + $labels[] = 35; } + $data['labels'] = implode(',', $labels); + $issueModel = new IssueModel($this->getContainer()->get('db')); $issueModel->setProject($project); + // Project is managed on GitHub if ($project->gh_user && $project->gh_project) { - // Project is managed on GitHub - try + // @todo assignee + $assignee = null; + + // Prepare labels + $ghLabels = []; + + if (!empty($labels)) { - $gitHubResponse = $gitHub->issues->create( - $project->gh_user, $project->gh_project, - $data['title'], $body - ); + foreach ($project->getLabels() as $id => $label) + { + if (in_array($id, $labels)) + { + $ghLabels[] = $label->name; + } + } } - catch (\Exception $e) - { - $this->getContainer()->get('app')->getLogger()->error( - sprintf( - 'Error code %1$s received from GitHub when creating an issue with the following data:' - . ' GitHub User: %2$s; GitHub Repo: %3$s; Title: %4$s; Body Text: %5$s' - . ' The error message returned was: %6$s', - $e->getCode(), - $project->gh_user, - $project->gh_project, - $data['title'], - $body, - $e->getMessage() - ) - ); - throw $e; - } + // Prepare milestone + $ghMilestone = null; - if (!isset($gitHubResponse->id)) + if (!empty($data['milestone_id'])) { - throw new \Exception('Invalid response from GitHub'); + foreach ($project->getMilestones() as $milestone) + { + if ($milestone->milestone_id == $data['milestone_id']) + { + $ghMilestone = $milestone->milestone_number; + } + } } + $gitHubResponse = $this->updateGitHub( + $data['title'], $body, $assignee, $ghMilestone, $ghLabels + ); + $data['opened_date'] = $gitHubResponse->created_at; $data['modified_date'] = $gitHubResponse->created_at; $data['opened_by'] = $gitHubResponse->user->login; @@ -104,30 +131,23 @@ public function execute() $project->gh_user . '/' . $project->gh_project ); } + // Project is managed by JTracker only else { - // Project is managed by JTracker only $data['opened_date'] = (new Date)->format($this->getContainer()->get('db')->getDateFormat()); $data['modified_date'] = (new Date)->format($this->getContainer()->get('db')->getDateFormat()); $data['opened_by'] = $user->username; $data['modified_by'] = $user->username; $data['number'] = $issueModel->getNextNumber(); - $data['description'] = $gitHub->markdown->render($body, 'markdown'); + $data['description'] = $gitHub->markdown->render($body, 'markdown'); } $data['priority'] = $application->input->getInt('priority'); - $data['milestone_id'] = $application->input->getInt('milestone_id'); $data['build'] = $application->input->getString('build'); $data['project_id'] = $project->project_id; $data['issue_number'] = $data['number']; $data['description_raw'] = $body; - // Store the "No code attached yet" label for CMS issues - if ($project->project_id == 1) - { - $data['labels'] = 35; - } - // Store the issue try { @@ -140,7 +160,7 @@ public function execute() $categoryModel = new CategoryModel($this->getContainer()->get('db')); $categoryModel->saveCategory($category); } - catch (\Exception $e) + catch (\RuntimeException $e) { $application->enqueueMessage($e->getMessage(), 'error'); @@ -159,4 +179,92 @@ public function execute() return; } + + /** + * Update the issue on GitHub. + * + * The method will first try to perform the action with the logged in user credentials and then, if it fails, perform + * the action using a configured "edit bot". If the GitHub status changes (e.g. open <=> close), a comment will be + * created automatically stating that the action has been performed by a bot. + * + * @param string $title The title of the issue. + * @param string $body The contents of the issue. + * @param string $assignee The login for the GitHub user that this issue should be assigned to. + * @param integer $milestone The milestone to associate this issue with. + * @param array $labels The labels to associate with this issue. + * + * @return object The issue data + * + * @since 1.0 + * @throws \RuntimeException + */ + private function updateGitHub($title, $body, $assignee, $milestone, $labels) + { + /* @type \JTracker\Application $application */ + $application = $this->getContainer()->get('app'); + + $project = $application->getProject(); + + try + { + // Try to perform the action on behalf of current user + $gitHub = GithubFactory::getInstance($application); + + // Look if we have a bot user configured + if ($project->getGh_Editbot_User() && $project->getGh_Editbot_Pass()) + { + // Try to perform the action on behalf of an authorized bot + $gitHubBot = GithubFactory::getInstance($application, true, $project->getGh_Editbot_User(), $project->getGh_Editbot_Pass()); + } + } + catch (\RuntimeException $exception) + { + throw new \RuntimeException('Error retrieving an instance of the Github object'); + } + + try + { + $gitHubResponse = $gitHub->issues->create( + $project->gh_user, $project->gh_project, + $title, $body, $assignee, $milestone, $labels + ); + } + catch (GithubException $exception) + { + $this->getContainer()->get('app')->getLogger()->error( + sprintf( + 'Error code %1$s received from GitHub when creating an issue with the following data:' + . ' GitHub User: %2$s; GitHub Repo: %3$s; Title: %4$s; Body Text: %5$s' + . ' The error message returned was: %6$s', + $exception->getCode(), + $project->gh_user, + $project->gh_project, + $title, + $body, + $exception->getMessage() + ) + ); + + throw new \RuntimeException('Invalid response from GitHub'); + } + + /** + * Try to update the milestone and/or labels. + * We are not throwing any error because the issue is already created. + */ + if (isset($gitHubBot)) + { + if ((!empty($milestone) && empty($gitHubResponse->milestone)) + || (!empty($labels) && empty($gitHubResponse->labels))) + { + $gitHubBot->issues->edit( + $project->gh_user, $project->gh_project, + $gitHubResponse->number, 'open', $title, $body, + $assignee, $milestone, $labels + ); + } + } + + return $gitHubResponse; + } } diff --git a/src/App/Tracker/Model/IssueModel.php b/src/App/Tracker/Model/IssueModel.php index bf60553e5..7dc9f0b85 100644 --- a/src/App/Tracker/Model/IssueModel.php +++ b/src/App/Tracker/Model/IssueModel.php @@ -383,8 +383,8 @@ public function save(array $src) $data['modified_date'] = $filter->clean($src['modified_date'], 'string'); } - $data['modified_by'] = $filter->clean($src['modified_by'], 'string'); - $data['milestone_id'] = isset($src['milestone_id']) ? $filter->clean($src['milestone_id'], 'int') : null; + $data['modified_by'] = $filter->clean($src['modified_by'], 'string'); + $data['milestone_id'] = isset($src['milestone_id']) ? $filter->clean($src['milestone_id'], 'int') : null; $state = $src['new_state']; $changedState = $src['old_state'] != $src['new_state']; @@ -403,6 +403,20 @@ public function save(array $src) $data['closed_by'] = null; } + $data['labels'] = null; + + if (!empty($src['labels'])) + { + $labels = []; + + foreach ($src['labels'] as $labelId) + { + $labels[] = (int) $labelId; + } + + $data['labels'] = implode(',', $labels); + } + if (!$data['id']) { throw new \RuntimeException('Missing ID'); diff --git a/src/JTracker/View/Renderer/TrackerExtension.php b/src/JTracker/View/Renderer/TrackerExtension.php index 3eccfb480..90a45ef96 100644 --- a/src/JTracker/View/Renderer/TrackerExtension.php +++ b/src/JTracker/View/Renderer/TrackerExtension.php @@ -111,6 +111,7 @@ public function getFunctions() new \Twig_SimpleFunction('renderLabels', array($this, 'renderLabels')), new \Twig_SimpleFunction('arrayDiff', array($this, 'arrayDiff')), new \Twig_SimpleFunction('userTestOptions', array($this, 'getUserTestOptions')), + new \Twig_SimpleFunction('getMilestoneTitle', array($this, 'getMilestoneTitle')), ); if (!JDEBUG) @@ -688,4 +689,39 @@ public function getUserTestOptions($id = null) return ($id !== null && array_key_exists($id, $options)) ? $options[$id] : $options; } + + /** + * Get the title of the milestone by id + * + * @param integer $id The id of the milestone + * + * @return string The title of the milestone + * + * @since 1.0 + */ + public function getMilestoneTitle($id) + { + static $milestones = array(); + + if (!$milestones) + { + $db = $this->container->get('db'); + + $milestones = $db->setQuery( + $db->getQuery(true) + ->select($db->quoteName(array('milestone_id', 'title'))) + ->from($db->quoteName('#__tracker_milestones')) + )->loadObjectList(); + } + + foreach ($milestones as $milestone) + { + if ($milestone->milestone_id == $id) + { + return $milestone->title; + } + } + + return ''; + } } diff --git a/templates/fields.twig b/templates/fields.twig index 61c9ff36f..3b9bae2c4 100644 --- a/templates/fields.twig +++ b/templates/fields.twig @@ -59,8 +59,25 @@ title="{{ 'Select Milestone...'|_ }}"> {% for item in items %} - + {% endif %} + {% endfor %} + +{% endmacro %} + +{% macro selectLabels(name, labels, selecteds, id, class) %} + diff --git a/templates/tracker/issue.add.twig b/templates/tracker/issue.add.twig index 088417c26..bd4d9b4b1 100644 --- a/templates/tracker/issue.add.twig +++ b/templates/tracker/issue.add.twig @@ -61,16 +61,25 @@ {% if project.categories %} - {{ fields.label('categories[]', 'Categories'|_) }} - {{ fields.selectCategories('categories[]', project.categories) }} -
- {{ "Please select one or more Categories here that match to your issue."|_ }} -
+ {{ fields.label('categories[]', 'Categories'|_) }} + {{ fields.selectCategories('categories[]', project.categories) }} +
+ {{ "Please select one or more Categories here that match to your issue."|_ }} +
{% endif %} - {% if user.check("edit") and project.milestones %} - {{ fields.label('milestone_id', 'Milestone'|_) }} - {{ fields.selectMilestone('milestone_id', project.milestones, 0, 'milestone_id', 'span12') }} + {% if user.check('admin') %} + + {% if project.labels %} + {{ fields.label('labels', 'Labels'|_) }} + {{ fields.selectLabels('labels[]', project.labels, item.labels|split(','), 'labels') }} + {% endif %} + + {% if project.milestones %} + {{ fields.label('milestone_id', 'Milestone'|_) }} + {{ fields.selectMilestone('milestone_id', project.milestones, 0, 'milestone_id', 'span12') }} + {% endif %} + {% endif %} diff --git a/templates/tracker/issue.edit.twig b/templates/tracker/issue.edit.twig index 1aa4cdcf1..6bac9bfc8 100644 --- a/templates/tracker/issue.edit.twig +++ b/templates/tracker/issue.edit.twig @@ -64,69 +64,81 @@ - {% if user.check("edit") %} - {# Show only if the user has *full*^edit rights - e.g. not "edit own" #} - + {% if user.check('edit') %} + {# Show only if the user has *full*^edit rights - e.g. not "edit own" #} + - + {% endif %}
diff --git a/templates/tracker/issue.index.twig b/templates/tracker/issue.index.twig index 87aea2019..c8a683aeb 100644 --- a/templates/tracker/issue.index.twig +++ b/templates/tracker/issue.index.twig @@ -476,8 +476,10 @@ {{ activities.easy(change.old, change.new) }} {% elseif 'rel_type' == change.name %} {{ activities.relationType(change.old, change.new) }} - {% elseif "category" == change.name %} + {% elseif 'category' == change.name %} {{ activities.category(change.old, change.new) }} + {% elseif 'milestone_id' == change.name %} + {{ activities.milestone(change.old, change.new) }} {% else %} {{ activities.change(change.name, change.old, change.new) }} {% endif %} diff --git a/templates/tracker/tpl/activities.twig b/templates/tracker/tpl/activities.twig index 31e706856..8cefb6888 100644 --- a/templates/tracker/tpl/activities.twig +++ b/templates/tracker/tpl/activities.twig @@ -125,6 +125,23 @@ {% endmacro %} +{% macro milestone(old, new) %} + + + {{ 'Milestone'|_ }} + + + {{ getMilestoneTitle(old) }} + + + ⇒ + + + {{ getMilestoneTitle(new) }} + + +{% endmacro %} + {% macro relationType(old, new) %}