Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like GithubController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use GithubController, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
32 | class GithubController extends AppController |
||
33 | { |
||
34 | public $helpers = [ |
||
35 | 'Html', |
||
36 | 'Form', |
||
37 | ]; |
||
38 | |||
39 | public $components = ['GithubApi']; |
||
40 | |||
41 | 5 | public function beforeFilter(Event $event) |
|
47 | |||
48 | /** |
||
49 | * create Github Issue. |
||
50 | * |
||
51 | * @param int $reportId The report number |
||
52 | * |
||
53 | * @throws NotFoundException |
||
54 | * @return void |
||
55 | */ |
||
56 | 1 | public function create_issue($reportId) |
|
57 | { |
||
58 | 1 | if (! isset($reportId) || ! $reportId) { |
|
59 | throw new NotFoundException(__('Invalid report')); |
||
60 | } |
||
61 | |||
62 | 1 | $reportsTable = TableRegistry::get('Reports'); |
|
|
|||
63 | 1 | $report = $reportsTable->findById($reportId)->all()->first(); |
|
64 | |||
65 | 1 | if (! $report) { |
|
66 | 1 | throw new NotFoundException(__('Invalid report')); |
|
67 | } |
||
68 | |||
69 | 1 | $reportArray = $report->toArray(); |
|
70 | 1 | if (empty($this->request->data)) { |
|
71 | 1 | $this->set('error_name', $reportArray['error_name']); |
|
72 | 1 | $this->set('error_message', $reportArray['error_message']); |
|
73 | |||
74 | 1 | return; |
|
75 | } |
||
76 | |||
77 | 1 | $this->autoRender = false; |
|
78 | $data = [ |
||
79 | 1 | 'title' => $this->request->data['summary'], |
|
80 | 1 | 'labels' => $this->request->data['labels'] ? explode(',', $this->request->data['labels']) : [], |
|
81 | ]; |
||
82 | 1 | $incidents_query = TableRegistry::get('Incidents')->findByReportId($reportId)->all(); |
|
83 | 1 | $incident = $incidents_query->first(); |
|
84 | 1 | $reportArray['exception_type'] = $incident['exception_type'] ? 'php' : 'js'; |
|
85 | 1 | $reportArray['description'] = $this->request->data['description']; |
|
86 | |||
87 | 1 | $data['body'] |
|
88 | 1 | = $this->_getReportDescriptionText($reportId, $reportArray); |
|
89 | 1 | $data['labels'][] = 'automated-error-report'; |
|
90 | |||
91 | 1 | list($issueDetails, $status) = $this->GithubApi->createIssue( |
|
92 | 1 | Configure::read('GithubRepoPath'), |
|
93 | $data, |
||
94 | 1 | $this->request->session()->read('access_token') |
|
95 | ); |
||
96 | |||
97 | 1 | if ($this->_handleGithubResponse($status, 1, $reportId, $issueDetails['number'])) { |
|
98 | // Update report status |
||
99 | 1 | $report->status = $this->_getReportStatusFromIssueState($issueDetails['state']); |
|
100 | 1 | $reportsTable->save($report); |
|
101 | |||
102 | 1 | $this->redirect(['controller' => 'reports', 'action' => 'view', |
|
103 | 1 | $reportId, |
|
104 | ]); |
||
105 | View Code Duplication | } else { |
|
106 | 1 | $flash_class = 'alert alert-error'; |
|
107 | 1 | $this->Flash->default( |
|
108 | 1 | $this->_getErrors($issueDetails, $status), |
|
109 | 1 | ['params' => ['class' => $flash_class]] |
|
110 | ); |
||
111 | } |
||
112 | 1 | } |
|
113 | |||
114 | /** |
||
115 | * Links error report to existing issue on Github. |
||
116 | * |
||
117 | * @param int $reportId The report Id |
||
118 | * @return void |
||
119 | */ |
||
120 | 1 | public function link_issue($reportId) |
|
121 | { |
||
122 | 1 | if (! isset($reportId) || ! $reportId) { |
|
123 | throw new NotFoundException(__('Invalid reportId')); |
||
124 | } |
||
125 | |||
126 | 1 | $reportsTable = TableRegistry::get('Reports'); |
|
127 | 1 | $report = $reportsTable->findById($reportId)->all()->first(); |
|
128 | |||
129 | 1 | if (! $report) { |
|
130 | 1 | throw new NotFoundException(__('Invalid report')); |
|
131 | } |
||
132 | |||
133 | 1 | $ticket_id = intval($this->request->query['ticket_id']); |
|
134 | 1 | if (! $ticket_id) { |
|
135 | 1 | throw new NotFoundException(__('Invalid Ticket ID!!')); |
|
136 | } |
||
137 | 1 | $reportArray = $report->toArray(); |
|
138 | |||
139 | 1 | $incidents_query = TableRegistry::get('Incidents')->findByReportId($reportId)->all(); |
|
140 | 1 | $incident = $incidents_query->first(); |
|
141 | 1 | $reportArray['exception_type'] = $incident['exception_type'] ? 'php' : 'js'; |
|
142 | |||
143 | 1 | $commentText = $this->_getReportDescriptionText( |
|
144 | 1 | $reportId, |
|
145 | $reportArray |
||
146 | ); |
||
147 | 1 | list($commentDetails, $status) = $this->GithubApi->createComment( |
|
148 | 1 | Configure::read('GithubRepoPath'), |
|
149 | 1 | ['body' => $commentText], |
|
150 | $ticket_id, |
||
151 | 1 | $this->request->session()->read('access_token') |
|
152 | ); |
||
153 | 1 | if ($this->_handleGithubResponse($status, 2, $reportId, $ticket_id)) { |
|
154 | // Update report status |
||
155 | 1 | $report->status = 'forwarded'; |
|
156 | |||
157 | 1 | list($issueDetails, $status) = $this->GithubApi->getIssue( |
|
158 | 1 | Configure::read('GithubRepoPath'), |
|
159 | 1 | [], |
|
160 | $ticket_id, |
||
161 | 1 | $this->request->session()->read('access_token') |
|
162 | ); |
||
163 | 1 | if ($this->_handleGithubResponse($status, 4, $reportId, $ticket_id)) { |
|
164 | // If linked Github issue state is available, use it to update Report's status |
||
165 | 1 | $report->status = $this->_getReportStatusFromIssueState( |
|
166 | 1 | $issueDetails['state'] |
|
167 | ); |
||
168 | } |
||
169 | |||
170 | 1 | $reportsTable->save($report); |
|
171 | View Code Duplication | } else { |
|
172 | 1 | $flash_class = 'alert alert-error'; |
|
173 | 1 | $this->Flash->default( |
|
174 | 1 | $this->_getErrors($commentDetails, $status), |
|
175 | 1 | ['params' => ['class' => $flash_class]] |
|
176 | ); |
||
177 | } |
||
178 | |||
179 | 1 | $this->redirect(['controller' => 'reports', 'action' => 'view', |
|
180 | 1 | $reportId, |
|
181 | ]); |
||
182 | 1 | } |
|
183 | |||
184 | /** |
||
185 | * Un-links error report to associated issue on Github. |
||
186 | * |
||
187 | * @param int $reportId The report Id |
||
188 | * @return void |
||
189 | */ |
||
190 | 1 | public function unlink_issue($reportId) |
|
191 | { |
||
192 | 1 | if (! isset($reportId) || ! $reportId) { |
|
193 | throw new NotFoundException(__('Invalid reportId')); |
||
194 | } |
||
195 | |||
196 | 1 | $reportsTable = TableRegistry::get('Reports'); |
|
197 | 1 | $report = $reportsTable->findById($reportId)->all()->first(); |
|
198 | |||
199 | 1 | if (! $report) { |
|
200 | 1 | throw new NotFoundException(__('Invalid report')); |
|
201 | } |
||
202 | |||
203 | 1 | $reportArray = $report->toArray(); |
|
204 | 1 | $ticket_id = $reportArray['sourceforge_bug_id']; |
|
205 | |||
206 | 1 | if (! $ticket_id) { |
|
207 | 1 | throw new NotFoundException(__('Invalid Ticket ID!!')); |
|
208 | } |
||
209 | |||
210 | // "formatted" text of the comment. |
||
211 | $commentText = 'This Issue is no longer associated with [Report#' |
||
212 | 1 | . $reportId |
|
213 | 1 | . '](' |
|
214 | 1 | . Router::url('/reports/view/' . $reportId, true) |
|
215 | 1 | . ')' |
|
216 | 1 | . "\n\n*This comment is posted automatically by phpMyAdmin's " |
|
217 | 1 | . '[error-reporting-server](https://reports.phpmyadmin.net).*'; |
|
218 | |||
219 | 1 | list($commentDetails, $status) = $this->GithubApi->createComment( |
|
220 | 1 | Configure::read('GithubRepoPath'), |
|
221 | 1 | ['body' => $commentText], |
|
222 | $ticket_id, |
||
223 | 1 | $this->request->session()->read('access_token') |
|
224 | ); |
||
225 | |||
226 | 1 | if ($this->_handleGithubResponse($status, 3, $reportId)) { |
|
227 | // Update report status |
||
228 | 1 | $report->status = 'new'; |
|
229 | 1 | $reportsTable->save($report); |
|
230 | View Code Duplication | } else { |
|
231 | 1 | $flash_class = 'alert alert-error'; |
|
232 | 1 | $this->Flash->default( |
|
233 | 1 | $this->_getErrors($commentDetails, $status), |
|
234 | 1 | ['params' => ['class' => $flash_class]] |
|
235 | ); |
||
236 | } |
||
237 | |||
238 | 1 | $this->redirect(['controller' => 'reports', 'action' => 'view', |
|
239 | 1 | $reportId, |
|
240 | ]); |
||
241 | 1 | } |
|
242 | |||
243 | /** |
||
244 | * Returns pretty error message string. |
||
245 | * |
||
246 | * @param object $response the response returned by Github api |
||
247 | * @param int $status status returned by Github API |
||
248 | * |
||
249 | * @return string error string |
||
250 | */ |
||
251 | 3 | protected function _getErrors($response, $status) |
|
263 | |||
264 | /** |
||
265 | * Returns the text to be added while creating an issue |
||
266 | * |
||
267 | * @param integer $reportId Report Id |
||
268 | * @param array $report Report associative array |
||
269 | * |
||
270 | * @return string |
||
271 | */ |
||
272 | 2 | protected function _getReportDescriptionText($reportId, $report) |
|
297 | |||
298 | /** |
||
299 | * Github Response Handler. |
||
300 | * |
||
301 | * @param int $response the status returned by Github API |
||
302 | * @param int $type type of response. 1 for create_issue, 2 for link_issue, 3 for unlink_issue, |
||
303 | * 1 for create_issue, |
||
304 | * 2 for link_issue, |
||
305 | * 3 for unlink_issue, |
||
306 | * 4 for get_issue |
||
307 | * @param int $report_id report id |
||
308 | * @param int $ticket_id ticket id, required for link ticket only |
||
309 | * |
||
310 | * @return bool value. True on success. False on any type of failure. |
||
311 | */ |
||
312 | 5 | protected function _handleGithubResponse($response, $type, $report_id, $ticket_id = 1) |
|
389 | |||
390 | /** |
||
391 | * Get Incident counts for a report and |
||
392 | * all its related reports |
||
393 | * |
||
394 | * @param int $reportId The report Id |
||
395 | * |
||
396 | * @return int Total Incident count for a report |
||
397 | */ |
||
398 | 2 | protected function _getTotalIncidentCount($reportId) |
|
399 | { |
||
400 | 2 | $incidents_query = TableRegistry::get('Incidents')->findByReportId($reportId)->all(); |
|
401 | 2 | $incident_count = $incidents_query->count(); |
|
402 | |||
403 | $params_count = [ |
||
404 | 2 | 'fields' => ['inci_count' => 'inci_count'], |
|
405 | 'conditions' => [ |
||
406 | 2 | 'related_to = ' . $reportId, |
|
407 | ], |
||
408 | ]; |
||
409 | $subquery_params_count = [ |
||
410 | 'fields' => [ |
||
411 | 2 | 'report_id' => 'report_id', |
|
412 | ], |
||
413 | ]; |
||
414 | 2 | $subquery_count = TableRegistry::get('Incidents')->find( |
|
415 | 2 | 'all', |
|
416 | $subquery_params_count |
||
417 | ); |
||
418 | 2 | $inci_count_related = TableRegistry::get('Reports')->find('all', $params_count)->innerJoin( |
|
419 | 2 | ['incidents' => $subquery_count], |
|
420 | 2 | ['incidents.report_id = Reports.related_to'] |
|
421 | 2 | )->count(); |
|
422 | |||
423 | 2 | return $incident_count + $inci_count_related; |
|
424 | } |
||
425 | |||
426 | /** |
||
427 | * Get corresponding report status from Github issue state |
||
428 | * |
||
429 | * @param string $issueState Linked Github issue's state |
||
430 | * |
||
431 | * @return string Corresponding status to which the linked report should be updated to |
||
432 | */ |
||
433 | 4 | protected function _getReportStatusFromIssueState($issueState) |
|
434 | { |
||
435 | // default |
||
436 | 4 | $reportStatus = ''; |
|
437 | 4 | switch ($issueState) { |
|
438 | 4 | case 'closed': |
|
439 | 3 | $reportStatus = 'resolved'; |
|
440 | 3 | break; |
|
441 | |||
442 | default: |
||
443 | 4 | $reportStatus = 'forwarded'; |
|
444 | 4 | break; |
|
445 | } |
||
446 | |||
447 | 4 | return $reportStatus; |
|
448 | } |
||
449 | |||
450 | /** |
||
451 | * Synchronize Report Statuses from Github issues |
||
452 | * |
||
453 | * To be used as a cron job (using webroot/cron_dispatcher.php). |
||
454 | * |
||
455 | * Can not (& should not) be directly accessed via web. |
||
456 | * @return void |
||
457 | */ |
||
458 | 2 | public function sync_issue_status() |
|
527 | } |
||
528 |
This method has been deprecated. The supplier of the class has supplied an explanatory message.
The explanatory message should give you some clue as to whether and when the method will be removed from the class and what other method or class to use instead.