ApprovalsDispatcher - Code Metrics - Inspection of "Remove approvers action, combine into environment..." - silverstripe/deploynaut - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#795)

by Sean

created 2016-11-09 04:14 UTC

ApprovalsDispatcher B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	302
Duplicated Lines	13.58 %

Coupling/Cohesion

Components	1
Dependencies	9

Importance

Changes

Metric	Value
wmc	45
lcom	1
cbo	9
dl	41
loc	302
rs	8.3673
c	0
b	0
f	0

8 Methods

Rating	Name	Duplication	Size	Complexity
A	init()	14	14	3
D	submit()	0	44	9
B	cancel()	0	32	5
C	approve()	12	72	14
C	reject()	3	41	7
B	validateDeployment()	12	12	5
A	getModel()	0	3	1
A	Link()	0	3	1

How to fix Duplicated Code Complexity

Duplicated Code

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:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like ApprovalsDispatcher 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 ApprovalsDispatcher, and based on these observations, apply Extract Interface, too.

<?php

class ApprovalsDispatcher extends Dispatcher {
namespace YourVendor;

class YourClass { }

	const ACTION_APPROVALS = 'approvals';

	const ALLOW_APPROVAL = 'ALLOW_APPROVAL';

	const ALLOW_APPROVAL_BYPASS = 'ALLOW_APPROVAL_BYPASS';

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

		'approvers',
		'submit',
		'cancel',
		'approve',
		'reject'
	];

	private static $dependencies = [
		'formatter' => '%$DeploynautAPIFormatter'
	];

	/**
	 * @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();
		}

		// Performs canView permission check by limiting visible projects
		$this->environment = $this->getCurrentEnvironment($this->project);
		if (!$this->environment) {
			return $this->environment404Response();
		}
	}

	/**
	 * @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);
		}

		$this->checkSecurityToken();

		$deployment = DNDeployment::get()->byId($request->postVar('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;
		}

		$approver = Member::get()->byId($request->postVar('approver_id'));
		if ($approver && $approver->exists()) {
			if (!$this->project->allowed(ApprovalsDispatcher::ALLOW_APPROVAL, $approver)) {
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
				return $this->getAPIResponse(['message' => 'The given approver does not have permissions to approve'], 403);
			}

			$deployment->ApproverID = $approver->ID;
			$deployment->write();
		}

		// title and summary may have changed, ensure they are saved
		if ($request->postVar('title')) {
			$deployment->Title = $request->postVar('title');
		}
		if ($request->postVar('summary')) {
			$deployment->Summary = $request->postVar('summary');

		}

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

		return $this->getAPIResponse([
			'message' => 'Deployment request has been submitted',
			'deployment' => $this->formatter->getDeploymentData($deployment)
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
		], 200);
	}

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

		$this->checkSecurityToken();

		$deployment = DNDeployment::get()->byId($request->postVar('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 the person cancelling is not the one who created the deployment, update the deployer
		if (Member::currentUserID() !== $deployment->DeployerID) {
			$deployment->DeployerID = Member::currentUserID();
			$deployment->write();
		}

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

		return $this->getAPIResponse([
			'message' => 'Deployment request has been cancelled',
			'deployment' => $this->formatter->getDeploymentData($deployment)
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
		], 200);
	}

	/**
	 * @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);
		}

		$this->checkSecurityToken();

		$deployment = DNDeployment::get()->byId($request->postVar('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;
		}

		// ensure we have either bypass or approval permission of the logged in user
		if (
			!$this->project->allowed(self::ALLOW_APPROVAL_BYPASS, Member::currentUser())

			|| !$this->project->allowed(self::ALLOW_APPROVAL, Member::currentUser())

		) {
			return $this->getAPIResponse(['message' => 'You are not authorised to approve or bypass this deployment'], 403);
		}

		// check for specific permission depending on the current state of the deployment:
		// submitted => approved requires approval permissions
		// new => approved requires bypass permissions.
		if (

			$deployment->State === DNDeployment::STATE_SUBMITTED
			&& !$this->project->allowed(self::ALLOW_APPROVAL, Member::currentUser())

		) {
			return $this->getAPIResponse(['message' => 'You are not authorised to approve this deployment'], 403);
		}
		if (

			$deployment->State === DNDeployment::STATE_NEW
			&& !$this->project->allowed(self::ALLOW_APPROVAL_BYPASS, Member::currentUser())

		) {
			return $this->getAPIResponse(['message' => 'You are not authorised to bypass approval of this deployment'], 403);
		}

		if ($deployment->State === DNDeployment::STATE_NEW) {
			// Bypassing approval: Ensure that approver is not set. This may happen when someone has requested approval,
			// cancelled approval, then bypassed.
			$deployment->ApproverID = 0;
			$deployment->write();
		} else {
			// if the current user is not the person who was selected for approval on submit, but they got
			// here because they still have permission, then change the approver to the current user
			if (Member::currentUserID() !== $deployment->ApproverID) {
				$deployment->ApproverID = Member::currentUserID();
				$deployment->write();
			}
		}

		// title and summary may have changed, ensure they are saved
		if ($request->postVar('title')) {
			$deployment->Title = $request->postVar('title');
		}
		if ($request->postVar('summary')) {
			$deployment->Summary = $request->postVar('summary');

		}

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

		return $this->getAPIResponse([
			'message' => 'Deployment request has been approved',
			'deployment' => $this->formatter->getDeploymentData($deployment)
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
		], 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);
		}

		$this->checkSecurityToken();

		$deployment = DNDeployment::get()->byId($request->postVar('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;
		}
		// reject permissions are the same as can approve
		if (!$this->project->allowed(self::ALLOW_APPROVAL, Member::currentUser())) {

			return $this->getAPIResponse(['message' => 'You are not authorised to reject this deployment'], 403);
		}

		// if the current user is not the person who was selected for approval on submit, but they got
		// here because they still have permission, then change the approver to the current user
		if (Member::currentUserID() !== $deployment->ApproverID) {
			$deployment->ApproverID = Member::currentUserID();
			$deployment->write();
		}

		if ($request->postVar('rejected_reason')) {
			$deployment->RejectedReason = $request->postVar('rejected_reason');
		}

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

		return $this->getAPIResponse([
			'message' => 'Deployment request has been rejected',
			'deployment' => $this->formatter->getDeploymentData($deployment)
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
		], 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($deployment) {

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

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

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

}


silverstripe / deploynaut