ApprovalsDispatcher::validateDeployment() - Code Metrics - Inspection of "[wip] Adding approval functionality" - silverstripe/deploynaut - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#681)

by Sean

created 2016-10-04 02:44 UTC

ApprovalsDispatcher::validateDeployment() A

↳ Parent: ApprovalsDispatcher

Complexity

Conditions	4
Paths	3

Size

Total Lines	9
Code Lines	6

Duplication

Lines	9
Ratio	100 %

Importance

Changes

Metric	Value
dl	9
loc	9
rs	9.2
c	0
b	0
f	0
cc	4
eloc	6
nc	3
nop	1

<?php

class ApprovalsDispatcher extends Dispatcher {
namespace YourVendor;

class YourClass { }

	const ACTION_APPROVALS = 'approvals';

	/**
	 * @var array
	 */
	public static $allowed_actions = [
		'approvers',
		'approve',
		'reject'
	];

	/**
	 * @var \DNProject
	 */
	protected $project = null;

	/**
	 * @var \DNEnvironment
	 */
	protected $environment = null;

	/**
	 * @var array
	 */
	private static $action_types = [

		self::ACTION_APPROVALS
	];

	public function init() {
		parent::init();

		$this->project = $this->getCurrentProject();
		if (!$this->project) {
			return $this->project404Response();
		}
	}

	/**
	 * @param \SS_HTTPRequest $request
	 * @return \SS_HTTPResponse
	 */
	public function approvers(SS_HTTPRequest $request) {
		$list = [];
		foreach ($this->project->listMembers() as $data) {
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
			if ($this->canApprove(Member::get()->byId($data['MemberID']))) {

				$list[] = [
					'id' => $data['MemberID'],
					'email' => $data['Email'],
					'role' => $data['RoleTitle'],
					'name' => $data['FullName']
				];
			}
		}

		return $this->getAPIResponse([
			'approvers' => $list
		], 200);
	}

	/**
	 * @param \SS_HTTPRequest $request
	 * @return \SS_HTTPResponse
	 */
	public function submit(SS_HTTPRequest $request) {
		if ($request->httpMethod() !== 'POST') {
			return $this->getAPIResponse(['message' => 'Method not allowed, requires POST'], 405);
		}

		// @todo 
	}

	/**
	 * @param \SS_HTTPRequest $request
	 * @return \SS_HTTPResponse
	 */
	public function approve(SS_HTTPRequest $request) {

		if ($request->httpMethod() !== 'POST') {
			return $this->getAPIResponse(['message' => 'Method not allowed, requires POST'], 405);
		}

		$deployment = DNDeployment::get()->byId($request->param('ID'));
		$errorResponse = $this->validateDeployment($deployment);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
		if ($errorResponse instanceof \SS_HTTPResponse) {
			return $errorResponse;
		}
		if (!$this->canApprove()) {
			return $this->getAPIResponse(['message' => 'You are not authorised to reject this deployment'], 403);
		}

		try {
			$deployment->getMachine()->apply(DNDeployment::TR_APPROVE);
		} catch (\Exception $e) {
			return $this->getAPIResponse([
				'status' => 'FAILED',
				'message' => $e->getMessage()
			], 400);
		}

		return $this->getAPIResponse([
			'status' => 'OK',
			'id' => $deployment->ID
		], 200);
	}

	/**
	 * @param \SS_HTTPRequest $request
	 * @return \SS_HTTPResponse
	 */
	public function reject(SS_HTTPRequest $request) {

		if ($request->httpMethod() !== 'POST') {
			return $this->getAPIResponse(['message' => 'Method not allowed, requires POST'], 405);
		}

		$deployment = DNDeployment::get()->byId($request->param('ID'));
		$errorResponse = $this->validateDeployment($deployment);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
		if ($errorResponse instanceof \SS_HTTPResponse) {
			return $errorResponse;
		}
		// can reject permissions are the same as can approve
		if (!$this->canApprove()) {
			return $this->getAPIResponse(['message' => 'You are not authorised to reject this deployment'], 403);
		}

		try {
			$deployment->getMachine()->apply(DNDeployment::TR_REJECT);
		} catch (\Exception $e) {
			return $this->getAPIResponse([
				'status' => 'FAILED',
				'message' => $e->getMessage()
			], 400);
		}

		return $this->getAPIResponse([
			'status' => 'OK',
			'message' => 'Deployment has been rejected',
			'id' => $deployment->ID,
		], 200);
	}

	/**
	 * Check if a DNDeployment exists and do permission checks on it. If there is something wrong it will return
	 * an APIResponse with the error, otherwise null.
	 *
	 * @param \DNDeployment $deployment
	 *
	 * @return null|SS_HTTPResponse
	 */
	protected function validateDeployment(\DNDeployment $deployment) {

		if (!$deployment || !$deployment->exists()) {
			return $this->getAPIResponse(['message' => 'This deployment does not exist'], 404);
		}
		if (!$deployment->canView()) {
			return $this->getAPIResponse(['message' => 'You are not authorised to view this deployment'], 403);
		}
		return null;
	}

	protected function canApprove(Member $member = null) {
		if (!$member) {
			$member = Member::currentUser();
		}
		if (!$member) {
			return false;
		}
		if (Permission::checkMember($member, 'ADMIN')) {
			return true;
		}

		foreach ($this->project->listMembers() as $data) {
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
			if ($data['MemberID'] == $member->ID && in_array($data['RoleTitle'], [
				GroupExtension::STACK_MANAGER,
				GroupExtension::RELEASE_MANAGER
			])) {
				return true;
			}
		}

		return false;
	}

	/**
	 * @param string $name
	 *
	 * @return array
	 */
	public function getModel($name = '') {
		return [];
	}

	/**
	 * @param string $action
	 * @return string
	 */
	public function Link($action = '') {
		return \Controller::join_links($this->project->Link(), self::ACTION_APPROVALS, $action);
	}

}


1		<?php
2
3		class ApprovalsDispatcher extends Dispatcher {
		0 ignored issues – show Coding Style introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report The property $allowed_actions is not named in camelCase. This check marks property names that have not been written in camelCase. In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes `databaseConnectionString`. Loading history... Coding Style introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report The property $action_types is not named in camelCase. This check marks property names that have not been written in camelCase. In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes `databaseConnectionString`. Loading history... Coding Style Compatibility introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report PSR1 recommends that each class must be in a namespace of at least one level to avoid collisions. You can fix this by adding a namespace to your class: namespace YourVendor; class YourClass { } When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries. Loading history...
4
5		const ACTION_APPROVALS = 'approvals';
6
7		/**
8		* @var array
9		*/
10		public static $allowed_actions = [
11		'approvers',
12		'approve',
13		'reject'
14		];
15
16		/**
17		* @var \DNProject
18		*/
19		protected $project = null;
20
21		/**
22		* @var \DNEnvironment
23		*/
24		protected $environment = null;
25
26		/**
27		* @var array
28		*/
29		private static $action_types = [
		0 ignored issues – show Comprehensibility introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report Consider using a different property name as you override a private property of the parent class. Loading history... Unused Code introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report The property `$action_types` is not used and could be removed. This check marks private properties in classes that are never used. Those properties can be removed. Loading history...
30		self::ACTION_APPROVALS
31		];
32
33		public function init() {
34		parent::init();
35
36		$this->project = $this->getCurrentProject();
37		if (!$this->project) {
38		return $this->project404Response();
39		}
40		}
41
42		/**
43		* @param \SS_HTTPRequest $request
44		* @return \SS_HTTPResponse
45		*/
46		public function approvers(SS_HTTPRequest $request) {
47		$list = [];
48		foreach ($this->project->listMembers() as $data) {
		0 ignored issues – show Documentation Bug introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report The method `listMembers` does not exist on `object<DNProject>`? Since you implemented `__call`, maybe consider adding a @method annotation. If you implement `__call` and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class. This is often the case, when `__call` is implemented by a parent class and only the child class knows which methods exist: class ParentClass { private $data = array(); public function __call($method, array $args) { if (0 === strpos($method, 'get')) { return $this->data[strtolower(substr($method, 3))]; } throw new \LogicException(sprintf('Unsupported method: %s', $method)); } } /** * If this class knows which fields exist, you can specify the methods here: * * @method string getName() */ class SomeClass extends ParentClass { } Loading history...
49		if ($this->canApprove(Member::get()->byId($data['MemberID']))) {
		0 ignored issues – show Bug introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report It seems like `\Member::get()->byId($data['MemberID'])` targeting `DataList::byID()` can also be of type `object<DataObject>`; however, `ApprovalsDispatcher::canApprove()` does only seem to accept `null\|object<Member>`, maybe add an additional type check? This check looks at variables that are passed out again to other methods. If the outgoing method call has stricter type requirements than the method itself, an issue is raised. An additional type check may prevent trouble. Loading history...
50		$list[] = [
51		'id' => $data['MemberID'],
52		'email' => $data['Email'],
53		'role' => $data['RoleTitle'],
54		'name' => $data['FullName']
55		];
56		}
57		}
58
59		return $this->getAPIResponse([
60		'approvers' => $list
61		], 200);
62		}
63
64		/**
65		* @param \SS_HTTPRequest $request
66		* @return \SS_HTTPResponse
67		*/
68		public function submit(SS_HTTPRequest $request) {
69		if ($request->httpMethod() !== 'POST') {
70		return $this->getAPIResponse(['message' => 'Method not allowed, requires POST'], 405);
71		}
72
73		// @todo
74		}
75
76		/**
77		* @param \SS_HTTPRequest $request
78		* @return \SS_HTTPResponse
79		*/
80	View Code Duplication	public function approve(SS_HTTPRequest $request) {
		0 ignored issues – show Duplication introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
81		if ($request->httpMethod() !== 'POST') {
82		return $this->getAPIResponse(['message' => 'Method not allowed, requires POST'], 405);
83		}
84
85		$deployment = DNDeployment::get()->byId($request->param('ID'));
86		$errorResponse = $this->validateDeployment($deployment);
		0 ignored issues – show Documentation introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report `$deployment` is of type `object<DataObject>\|null`, but the function expects a `object<DNDeployment>`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
87		if ($errorResponse instanceof \SS_HTTPResponse) {
88		return $errorResponse;
89		}
90		if (!$this->canApprove()) {
91		return $this->getAPIResponse(['message' => 'You are not authorised to reject this deployment'], 403);
92		}
93
94		try {
95		$deployment->getMachine()->apply(DNDeployment::TR_APPROVE);
96		} catch (\Exception $e) {
97		return $this->getAPIResponse([
98		'status' => 'FAILED',
99		'message' => $e->getMessage()
100		], 400);
101		}
102
103		return $this->getAPIResponse([
104		'status' => 'OK',
105		'id' => $deployment->ID
106		], 200);
107		}
108
109		/**
110		* @param \SS_HTTPRequest $request
111		* @return \SS_HTTPResponse
112		*/
113	View Code Duplication	public function reject(SS_HTTPRequest $request) {
		0 ignored issues – show Duplication introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
114		if ($request->httpMethod() !== 'POST') {
115		return $this->getAPIResponse(['message' => 'Method not allowed, requires POST'], 405);
116		}
117
118		$deployment = DNDeployment::get()->byId($request->param('ID'));
119		$errorResponse = $this->validateDeployment($deployment);
		0 ignored issues – show Documentation introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report `$deployment` is of type `object<DataObject>\|null`, but the function expects a `object<DNDeployment>`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
120		if ($errorResponse instanceof \SS_HTTPResponse) {
121		return $errorResponse;
122		}
123		// can reject permissions are the same as can approve
124		if (!$this->canApprove()) {
125		return $this->getAPIResponse(['message' => 'You are not authorised to reject this deployment'], 403);
126		}
127
128		try {
129		$deployment->getMachine()->apply(DNDeployment::TR_REJECT);
130		} catch (\Exception $e) {
131		return $this->getAPIResponse([
132		'status' => 'FAILED',
133		'message' => $e->getMessage()
134		], 400);
135		}
136
137		return $this->getAPIResponse([
138		'status' => 'OK',
139		'message' => 'Deployment has been rejected',
140		'id' => $deployment->ID,
141		], 200);
142		}
143
144		/**
145		* Check if a DNDeployment exists and do permission checks on it. If there is something wrong it will return
146		* an APIResponse with the error, otherwise null.
147		*
148		* @param \DNDeployment $deployment
149		*
150		* @return null\|SS_HTTPResponse
151		*/
152	View Code Duplication	protected function validateDeployment(\DNDeployment $deployment) {
		0 ignored issues – show Duplication introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
153		if (!$deployment \|\| !$deployment->exists()) {
154		return $this->getAPIResponse(['message' => 'This deployment does not exist'], 404);
155		}
156		if (!$deployment->canView()) {
157		return $this->getAPIResponse(['message' => 'You are not authorised to view this deployment'], 403);
158		}
159		return null;
160		}
161
162		protected function canApprove(Member $member = null) {
163		if (!$member) {
164		$member = Member::currentUser();
165		}
166		if (!$member) {
167		return false;
168		}
169		if (Permission::checkMember($member, 'ADMIN')) {
170		return true;
171		}
172
173		foreach ($this->project->listMembers() as $data) {
		0 ignored issues – show Documentation Bug introduced 2016-10-04 02:48 UTC by Report Bug Copy Issue Report The method `listMembers` does not exist on `object<DNProject>`? Since you implemented `__call`, maybe consider adding a @method annotation. If you implement `__call` and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class. This is often the case, when `__call` is implemented by a parent class and only the child class knows which methods exist: class ParentClass { private $data = array(); public function __call($method, array $args) { if (0 === strpos($method, 'get')) { return $this->data[strtolower(substr($method, 3))]; } throw new \LogicException(sprintf('Unsupported method: %s', $method)); } } /** * If this class knows which fields exist, you can specify the methods here: * * @method string getName() */ class SomeClass extends ParentClass { } Loading history...
174		if ($data['MemberID'] == $member->ID && in_array($data['RoleTitle'], [
175		GroupExtension::STACK_MANAGER,
176		GroupExtension::RELEASE_MANAGER
177		])) {
178		return true;
179		}
180		}
181
182		return false;
183		}
184
185		/**
186		* @param string $name
187		*
188		* @return array
189		*/
190		public function getModel($name = '') {
191		return [];
192		}
193
194		/**
195		* @param string $action
196		* @return string
197		*/
198		public function Link($action = '') {
199		return \Controller::join_links($this->project->Link(), self::ACTION_APPROVALS, $action);
200		}
201
202		}
203

silverstripe / deploynaut

Pull Request — master (#681)

ApprovalsDispatcher::validateDeployment() A

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like