Update backend

9 files changed, 485 insertions, 242 deletions

diff --git a/lib/Service/AnonymizeService.php b/lib/Service/AnonymizeService.php
index 80f11fa1..27fbdc9b 100644
--- a/lib/Service/AnonymizeService.php
+++ b/lib/Service/AnonymizeService.php
@@ -31,10 +31,19 @@ use OCA\Polls\Db\CommentMapper;
 
 class AnonymizeService {
 
+	/** @var VoteMapper */
 	private $voteMapper;
+
+	/** @var CommentMapper */
 	private $commentMapper;
+
+	/** @var array */
 	private $anonList = array();
+
+	/** @var string */
 	private $userId;
+
+	/** @var int */
 	private $pollId;
 
 	public function __construct(
@@ -74,7 +83,7 @@ class AnonymizeService {
 	 * Initialize anonymizer with pollId and userId
 	 * Creates a mapping list with unique Anonymous strings based on the partcipants of a poll
 	 * @NoAdminRequired
-	 * @param integer $pollId
+	 * @param int $pollId
 	 * @param string $userId - usernames, which will not be anonymized
 	 */
 
diff --git a/lib/Service/CommentService.php b/lib/Service/CommentService.php
index a1cd7131..f2c6ef56 100644
--- a/lib/Service/CommentService.php
+++ b/lib/Service/CommentService.php
@@ -23,10 +23,9 @@
 
 namespace OCA\Polls\Service;
 
-use \Exception;
-use OCP\ILogger;
-
+use Exception;
 use OCA\Polls\Exceptions\NotAuthorizedException;
+
 use OCA\Polls\Db\Comment;
 use OCA\Polls\Db\CommentMapper;
 use OCA\Polls\Model\Acl;
@@ -36,15 +35,20 @@ use OCA\Polls\Service\AnonymizeService;
 
 class CommentService {
 
-	private $comment;
+	/** @var CommentMapper */
 	private $commentMapper;
-	private $logger;
+
+	/** @var Comment */
+	private $comment;
+
+	/** @var AnonymizeService */
 	private $anonymizer;
+
+	/** @var Acl */
 	private $acl;
 
 	/**
 	 * CommentService constructor.
-	 * @param ILogger $logger
 	 * @param CommentMapper $commentMapper
 	 * @param Comment $comment
 	 * @param AnonymizeService $anonymizer
@@ -52,7 +56,6 @@ class CommentService {
 	 */
 
 	public function __construct(
-		ILogger $logger,
 		CommentMapper $commentMapper,
 		Comment $comment,
 		AnonymizeService $anonymizer,
@@ -60,18 +63,18 @@ class CommentService {
 	) {
 		$this->commentMapper = $commentMapper;
 		$this->comment = $comment;
-		$this->logger = $logger;
 		$this->anonymizer = $anonymizer;
 		$this->acl = $acl;
 	}
 
 	/**
-	 * get
+	 * Get comments
 	 * Read all comments of a poll based on the poll id and return list as array
 	 * @NoAdminRequired
-	 * @param integer $pollId
+	 * @param int $pollId
 	 * @param string $token
-	 * @return Array
+	 * @return array
+	 * @throws NotAuthorizedException
 	 */
 	public function list($pollId = 0, $token = '') {
 
@@ -88,12 +91,13 @@ class CommentService {
 	}
 
 	/**
-	 * Write a new comment to the db and returns the new comment as array
+	 * Add comment
 	 * @NoAdminRequired
-	 * @param string $message
 	 * @param int $pollId
+	 * @param string $message
 	 * @param string $token
 	 * @return Comment
+	 * @throws NotAuthorizedException
 	 */
 	public function add($pollId = 0, $message, $token = '') {
 
@@ -114,20 +118,20 @@ class CommentService {
 				throw new NotAuthorizedException;
 			}
 
-		} catch (\Exception $e) {
-			$this->logger->alert('Error writing comment for pollId ' . $pollId . ': '. $e);
+		} catch (Exception $e) {
+			\OC::$server->getLogger()->alert('Error writing comment for pollId ' . $pollId . ': '. $e);
 			throw new NotAuthorizedException($e);
 		}
 
 	}
 
 	/**
-	 * delete
-	 * Delete Comment
+	 * Delete comment
 	 * @NoAdminRequired
 	 * @param int $commentId
 	 * @param string $token
 	 * @return Comment
+	 * @throws NotAuthorizedException
 	 */
 	public function delete($commentId, $token = '') {
 		$this->comment = $this->commentMapper->find($commentId);
diff --git a/lib/Service/LogService.php b/lib/Service/LogService.php
index d6936c04..950abf44 100644
--- a/lib/Service/LogService.php
+++ b/lib/Service/LogService.php
@@ -31,36 +31,38 @@ use OCA\Polls\Db\LogMapper;
 
 class LogService {
 
-	private $mapper;
-	private $logItem;
+	/** @var LogMapper */
+	private $logMapper;
+
+	/** @var Log */
+	private $log;
 
 	/**
 	 * LogService constructor.
-	 * @param LogMapper $mapper
-	 * @param Log $logItem
+	 * @param LogMapper $logMapper
+	 * @param Log $log
 	 */
-
 	public function __construct(
-		LogMapper $mapper,
-		Log $logItem
+		LogMapper $logMapper,
+		Log $log
 	) {
-		$this->mapper = $mapper;
-		$this->logItem = $logItem;
+		$this->logMapper = $logMapper;
+		$this->log = $log;
 	}
 
 
 	/**
 	 * Prevent repetition of the same log event
 	 * @NoAdminRequired
-	 * @return Bool
+	 * @return bool
 	 */
 	public function isRepetition() {
 		try {
-			$lastRecord = $this->mapper->getLastRecord($this->logItem->getPollId());
-			return (intval($lastRecord->getPollId()) === intval($this->logItem->getPollId())
-				&& $lastRecord->getUserId() === $this->logItem->getUserId()
-				&& $lastRecord->getMessageId() === $this->logItem->getMessageId()
-				&& $lastRecord->getMessage() === $this->logItem->getMessage()
+			$lastRecord = $this->logMapper->getLastRecord($this->log->getPollId());
+			return (intval($lastRecord->getPollId()) === intval($this->log->getPollId())
+				&& $lastRecord->getUserId() === $this->log->getUserId()
+				&& $lastRecord->getMessageId() === $this->log->getMessageId()
+				&& $lastRecord->getMessage() === $this->log->getMessage()
 			);
 		} catch (DoesNotExistException $e) {
 			return false;
@@ -77,23 +79,23 @@ class LogService {
 	 * @return Log
 	 */
 	public function setLog($pollId, $messageId, $userId = null, $message = null) {
-		$this->logItem = new Log();
-		$this->logItem->setPollId($pollId);
-		$this->logItem->setCreated(time());
-		$this->logItem->setMessageId($messageId);
-		$this->logItem->setMessage($message);
+		$this->log = new Log();
+		$this->log->setPollId($pollId);
+		$this->log->setCreated(time());
+		$this->log->setMessageId($messageId);
+		$this->log->setMessage($message);
 
 		if ($userId) {
-			$this->logItem->setUserId($userId);
+			$this->log->setUserId($userId);
 		} else {
-			$this->logItem->setUserId(\OC::$server->getUserSession()->getUser()->getUID());
+			$this->log->setUserId(\OC::$server->getUserSession()->getUser()->getUID());
 		}
 
 
 		if ($this->isRepetition()) {
 			return null;
 		} else {
-			return $this->mapper->insert($this->logItem);
+			return $this->logMapper->insert($this->log);
 		}
 	}
 
diff --git a/lib/Service/MailService.php b/lib/Service/MailService.php
index fbd40984..54fe8a34 100644
--- a/lib/Service/MailService.php
+++ b/lib/Service/MailService.php
@@ -34,7 +34,6 @@ use OCP\IL10N;
 use OCP\L10N\IFactory;
 use OCP\Mail\IMailer;
 use OCP\Mail\IEMailTemplate;
-use OCP\ILogger;
 
 use OCA\Polls\Db\SubscriptionMapper;
 use OCA\Polls\Db\Subscription;
@@ -46,18 +45,37 @@ use OCA\Polls\Db\LogMapper;
 
 class MailService {
 
+	/** @var IUserManager */
 	private $userManager;
+
+	/** @var IGroupManager */
 	private $groupManager;
+
+	/** @var IConfig */
 	private $config;
+
+	/** @var IURLGenerator */
 	private $urlGenerator;
+
+	/** @var IL10N */
 	private $trans;
+
+	/** @var IFactory */
 	private $transFactory;
+
+	/** @var IMailer */
 	private $mailer;
-	private $logger;
 
-	private $shareMapper;
+	/** @var SubscriptionMapper */
 	private $subscriptionMapper;
+
+	/** @var ShareMapper */
+	private $shareMapper;
+
+	/** @var PollMapper */
 	private $pollMapper;
+
+	/** @var LogMapper */
 	private $logMapper;
 
 	/**
@@ -69,7 +87,6 @@ class MailService {
 	 * @param IL10N $trans
 	 * @param IFactory $transFactory
 	 * @param IMailer $mailer
-	 * @param ILogger $logger
 	 * @param SubscriptionMapper $subscriptionMapper
 	 * @param ShareMapper $shareMapper
 	 * @param PollMapper $pollMapper
@@ -84,7 +101,6 @@ class MailService {
 		IL10N $trans,
 		IFactory $transFactory,
 		IMailer $mailer,
-		ILogger $logger,
 		ShareMapper $shareMapper,
 		SubscriptionMapper $subscriptionMapper,
 		PollMapper $pollMapper,
@@ -97,7 +113,6 @@ class MailService {
 		$this->trans = $trans;
 		$this->transFactory = $transFactory;
 		$this->mailer = $mailer;
-		$this->logger = $logger;
 		$this->shareMapper = $shareMapper;
 		$this->subscriptionMapper = $subscriptionMapper;
 		$this->pollMapper = $pollMapper;
@@ -135,7 +150,7 @@ class MailService {
 			return null;
 
 		} catch (\Exception $e) {
-			$this->logger->logException($e, ['app' => 'polls']);
+			\OC::$server->getLogger()->logException($e, ['app' => 'polls']);
 			throw $e;
 		}
 
@@ -305,7 +320,7 @@ class MailService {
 				$sentMails[] = $recipient;
 			} catch (Exception $e) {
 				$abortedMails[] = $recipient;
-				$this->logger->alert('Error sending Mail to ' . json_encode($recipient));
+				\OC::$server->getLogger()->alert('Error sending Mail to ' . json_encode($recipient));
 			}
 		}
 		return ['sentMails' => $sentMails, 'abortedMails' => $abortedMails];
@@ -428,7 +443,7 @@ class MailService {
 			try {
 				$this->sendMail($emailTemplate, $subscription->getUserId());
 			} catch (Exception $e) {
-				$this->logger->alert('Error sending Mail to ' . $subscription->getUserId());
+				\OC::$server->getLogger()->alert('Error sending Mail to ' . $subscription->getUserId());
 			}
 		}
 	}
diff --git a/lib/Service/OptionService.php b/lib/Service/OptionService.php
index 0afe4b8c..2bab49a8 100644
--- a/lib/Service/OptionService.php
+++ b/lib/Service/OptionService.php
@@ -26,23 +26,41 @@ namespace OCA\Polls\Service;
 use Exception;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCA\Polls\Exceptions\NotAuthorizedException;
+use OCA\Polls\Exceptions\BadRequestException;
 
-use OCA\Polls\Db\Option;
 use OCA\Polls\Db\OptionMapper;
+use OCA\Polls\Db\Option;
+use OCA\Polls\Db\PollMapper;
+use OCA\Polls\Db\Poll;
 use OCA\Polls\Service\LogService;
 use OCA\Polls\Model\Acl;
 
 class OptionService  {
 
+	/** @var OptionMapper */
 	private $optionMapper;
+
+	/** @var Option */
 	private $option;
+
+	/** @var PollMapper */
+	private $pollMapper;
+
+	/** @var Poll */
+	private $poll;
+
+	/** @var LogService */
 	private $logService;
+
+	/** @var Acl */
 	private $acl;
 
 	/**
 	 * OptionController constructor.
 	 * @param OptionMapper $optionMapper
 	 * @param Option $option
+	 * @param PollMapper $pollMapper
+	 * @param Poll $poll
 	 * @param LogService $logService
 	 * @param Acl $acl
 	 */
@@ -50,85 +68,94 @@ class OptionService  {
 	public function __construct(
 		OptionMapper $optionMapper,
 		Option $option,
+		PollMapper $pollMapper,
+   	 	Poll $poll,
 		LogService $logService,
 		Acl $acl
 	) {
 		$this->optionMapper = $optionMapper;
 		$this->option = $option;
+		$this->pollMapper = $pollMapper;
+		$this->poll = $poll;
 		$this->logService = $logService;
 		$this->acl = $acl;
 	}
 
 	/**
-	 * Set properties from option array
+	 * Get all options of given poll
 	 * @NoAdminRequired
-	 * @param Array $option
+	 * @param int $pollId
+	 * @param string $token
+	 * @return array Array of Option objects
+	 * @throws NotAuthorizedException
 	 */
-	private function set($option) {
-
-		$this->option->setPollId($option['pollId']);
-		$this->option->setPollOptionText(trim(htmlspecialchars($option['pollOptionText'])));
-		$this->option->setTimestamp($option['timestamp']);
+	public function list($pollId = 0, $token = '') {
 
-		if ($option['timestamp']) {
-			$this->option->setOrder($option['timestamp']);
-		} else {
-			$this->option->setOrder($option['order']);
+		if (!$this->acl->setPollIdOrToken($pollId, $token)->getAllowView()) {
+			throw new NotAuthorizedException;
 		}
 
-		if ($option['confirmed']) {
-			// do not update confirmation date, if option is already confirmed
-			if (!$this->option->getConfirmed()) {
-				$this->option->setConfirmed(time());
-			}
-		} else {
-			$this->option->setConfirmed(0);
+		try {
+			return $this->optionMapper->findByPoll($pollId);
+		} catch (DoesNotExistException $e) {
+			return [];
 		}
 	}
 
+
 	/**
-	 * Get all options of given poll
+	 * Add a new option
 	 * @NoAdminRequired
-	 * @param integer $pollId
-	 * @param string $token
-	 * @return array Array of Option objects
+	 * @param int $pollId
+	 * @param int $timestamp
+	 * @param string $pollOptionText
+	 * @return Option
+	 * @throws NotAuthorizedException
 	 */
-	public function list($pollId = 0, $token = '') {
+	public function add($pollId, $timestamp = 0 , $pollOptionText = '') {
 
-		if (!$this->acl->setPollIdOrToken($pollId, $token)->getAllowView()) {
+		$this->poll = $this->pollMapper->find($pollId);
+		if (!$this->acl->setPollId($pollId)->getAllowEdit()) {
 			throw new NotAuthorizedException;
 		}
 
-		return $this->optionMapper->findByPoll($pollId);
+		$this->option = new Option();
+		$this->option->setPollId($pollId);
+		$this->setOption($timestamp, $pollOptionText, 0);
 
+		return $this->optionMapper->insert($this->option);
 	}
 
-
 	/**
-	 * Add a new Option to poll
+	 * Update option
 	 * @NoAdminRequired
-	 * @param Array $option
+	 * @param int $optionId
+	 * @param int $timestamp
+	 * @param string $pollOptionText
+	 * @param int $order
 	 * @return Option
+	 * @throws NotAuthorizedException
 	 */
-	public function add($option) {
+	public function update($optionId, $timestamp = 0 , $pollOptionText = '', $order = 0) {
 
-		if (!$this->acl->setPollId($option['pollId'])->getAllowEdit()) {
+		$this->option = $this->optionMapper->find($optionId);
+		$this->poll = $this->pollMapper->find($this->option->getPollId());
+
+		if (!$this->acl->setPollId($this->option->getPollId())->getAllowEdit()) {
 			throw new NotAuthorizedException;
 		}
 
-		$this->option = new Option();
-		$this->set($option);
-		$this->optionMapper->insert($this->option);
-		$this->logService->setLog($option['pollId'], 'addOption');
+		$this->setOption($timestamp, $pollOptionText, $order);
 
-		return $this->option;
+		return $this->optionMapper->update($this->option);
 	}
 
 	/**
-	 * Remove a single option
+	 * Delete option
 	 * @NoAdminRequired
-	 * @param Option $option
-	 * @return array Array of Option objects
+	 * @param int $optionId
+	 * @return Option deleted Option
+	 * @throws NotAuthorizedException
 	 */
 	public function delete($optionId) {
 		$this->option = $this->optionMapper->find($optionId);
@@ -140,45 +167,79 @@ class OptionService  {
 		$this->optionMapper->delete($this->option);
 
 		return $this->option;
-
 	}
 
 	/**
-	 * Update poll option
+	 * Switch optoin confirmation
 	 * @NoAdminRequired
-	 * @param array $option
-	 * @return Option
+	 * @param int $optionId
+	 * @return Option confirmed Option
+	 * @throws NotAuthorizedException
 	 */
-	public function update($option) {
-		if (!$this->acl->setPollId($option['pollId'])->getAllowEdit()) {
+	public function confirm($optionId) {
+		$this->option = $this->optionMapper->find($optionId);
+
+		if (!$this->acl->setPollId($this->option->getPollId())->getAllowEdit()) {
 			throw new NotAuthorizedException;
 		}
 
-		try {
-			$this->option = $this->optionMapper->find($option['id']);
-			$this->set($option);
-			$this->optionMapper->update($this->option);
-			$this->logService->setLog($option['pollId'], 'updateOption');
+		if ($this->option->setConfirmation()) {
+			$this->option->setConfirmation(0);
+		} else {
+			$this->option->setConfirmation(time());
+		}
+
+		return $this->optionMapper->update($this->option);
+	}
+
+	/**
+	 * Copy options from $fromPoll to $toPoll
+	 * @NoAdminRequired
+	 * @param int $fromPollId
+	 * @param int $toPollId
+	 * @return array Array of Option objects
+	 * @throws NotAuthorizedException
+	 */
+	public function clone($fromPollId, $toPollId) {
 
-			return $this->option;
-		} catch (Exception $e) {
-			return new DoesNotExistException($e);
+		if (!$this->acl->setPollId($fromPollId)->getAllowView()) {
+			throw new NotAuthorizedException;
 		}
 
+		foreach ($this->optionMapper->findByPoll($fromPollId) as $origin) {
+			$option = new Option();
+			$option->setPollId($toPollId);
+			$option->setConfirmed(0);
+			$option->setPollOptionText($origin->getPollOptionText());
+			$option->setTimestamp($origin->getTimestamp());
+			$option->setOrder($origin->getOrder());
+			$this->optionMapper->insert($option);
+		}
+
+		return $this->optionMapper->findByPoll($toPollId);
 	}
 
 	/**
-	 * Set order by order of the given array
+	 * Reorder options with the order specified by $options
 	 * @NoAdminRequired
-	 * @param array $options
+	 * @param int $pollId
+	 * @param array $options - Array of options
 	 * @return array Array of Option objects
+	 * @throws NotAuthorizedException
+	 * @throws BadRequestException
 	 */
 	public function reorder($pollId, $options) {
 
+		$this->poll = $this->pollMapper->find($pollId);
+
 		if (!$this->acl->setPollId($pollId)->getAllowEdit()) {
 			throw new NotAuthorizedException;
 		}
 
+		if ($this->poll->getType() === 'datePoll') {
+			throw new BadRequestException("Not allowed in date polls", 1);
+		}
+
 		$i = 0;
 		foreach ($options as $option) {
 			$this->option = $this->optionMapper->find($option['id']);
@@ -189,28 +250,116 @@ class OptionService  {
 		}
 
 		return $this->optionMapper->findByPoll($pollId);
-
 	}
 
 	/**
-	 * Set order by order of the given array
+	 * Change order for $optionId and reorder the options
 	 * @NoAdminRequired
-	 * @param integer $fromPollId
-	 * @param integer $toPollId
+	 * @param int $optionId
+	 * @param int $newOrder
 	 * @return array Array of Option objects
+	 * @throws NotAuthorizedException
+	 * @throws BadRequestException
 	 */
-	public function clone($fromPollId, $toPollId) {
+	public function setOrder($optionId, $newOrder) {
 
-		if (!$this->acl->setPollId($fromPollId)->getAllowView()) {
+		$this->option = $this->optionMapper->find($optionId);
+		$pollId = $this->option->getPollId();
+		$this->poll = $this->pollMapper->find($pollId);
+
+		if (!$this->acl->setPollId($pollId)->getAllowEdit()) {
 			throw new NotAuthorizedException;
 		}
 
-		foreach ($this->optionMapper->findByPoll($fromPollId) as $option) {
-			$option->setPollId($toPollId);
-			$this->optionMapper->insert($option);
+		if ($this->poll->getType() === 'datePoll') {
+			throw new BadRequestException("Not allowed in date polls", 1);
 		}
 
-		return $this->optionMapper->findByPoll($toPollId);
+		if ($newOrder < 1) {
+			$newOrder = 1;
+		} elseif ($newOrder > getHighestOrder($pollId)) {
+			$newOrder = getHighestOrder($pollId);
+		}
+
+		$oldOrder = $this->option->getOrder();
+
+		foreach ($this->optionMapper->findByPoll($pollId) as $option) {
+			$currentOrder = $option->getOrder();
+			if (
+				   ($currentOrder < $oldOrder && $currentOrder < $newOrder)
+				|| ($currentOrder > $oldOrder && $currentOrder > $newOrder)
+			) {
+
+				continue;
+
+			} elseif ($currentOrder > $oldOrder && $currentOrder <= $newOrder) {
 
+				$option->setOrder($currentOrder - 1);
+				$this->optionMapper->update($option);
+
+			} elseif (
+					   ($currentOrder < $oldOrder && $currentOrder >= $newOrder)
+					|| ($currentOrder < $oldOrder && $currentOrder = $newOrder)
+				) {
+
+				$option->setOrder($currentOrder + 1);
+				$this->optionMapper->update($option);
+
+			} elseif ($currentOrder === $oldOrder) {
+
+				$option->setOrder($newOrder);
+				$this->optionMapper->update($option);
+
+			}
+		}
+
+		return $this->optionMapper->findByPoll($this->option->getPollId());
+	}
+
+	/**
+	 * Set option entities validated
+	 * @NoAdminRequired
+	 * @param int $timestamp
+	 * @param string $pollOptionText
+	 * @param int $order
+	 * @throws BadRequestException
+	 */
+	private function setOption($timestamp = 0 , $pollOptionText = '', $order = 0) {
+		if ($this->poll->getType() === 'datePoll') {
+			if ($timestamp) {
+				$this->option->setTimestamp($timestamp);
+				$this->option->setOrder($timestamp);
+				$this->option->setPollOptionText(date('c', $timestamp));
+			} else {
+				throw new BadRequestException("Date poll must have a timestamp", 1);
+			}
+		} elseif ($this->poll->getType() === 'textPoll') {
+			if ($pollOptionText) {
+				$this->option->setPollOptionText($pollOptionText);
+			} else {
+				throw new BadRequestException("Text poll must have a pollOptionText", 1);
+			}
+
+			if (!$order && !$this->option->getOrder()) {
+				$order = $this->getHighestOrder($this->option->getPollId()) + 1;
+				$this->option->setOrder($order);
+			}
+		}
+	}
+
+	/**
+	 * Get the highest order number in $pollId
+	 * @NoAdminRequired
+	 * @param int $pollId
+	 * @return int Highest order number
+	 */
+	private function getHighestOrder($pollId) {
+		$order = 0;
+		foreach ($this->optionMapper->findByPoll($pollId) as $option) {
+			if ($option->getOrder() > $order) {
+				$order = $option->getOrder();
+			}
+		}
+		return $order;
 	}
 }
diff --git a/lib/Service/PollService.php b/lib/Service/PollService.php
index d23c8595..946a66d2 100644
--- a/lib/Service/PollService.php
+++ b/lib/Service/PollService.php
@@ -31,7 +31,6 @@
  use OCA\Polls\Exceptions\InvalidPollTypeException;
  use OCA\Polls\Exceptions\NotAuthorizedException;
 
- use OCP\ILogger;
 
  use OCA\Polls\Db\PollMapper;
  use OCA\Polls\Db\Poll;
@@ -40,15 +39,20 @@
 
  class PollService {
 
-	private $logger;
+	/** @var PollMapper */
 	private $pollMapper;
+
+	/** @var Poll */
  	private $poll;
+
+	/** @var LogService */
  	private $logService;
+
+	/** @var Acl */
  	private $acl;
 
  	/**
  	 * PollController constructor.
- 	 * @param ILogger $logger
  	 * @param PollMapper $pollMapper
  	 * @param Poll $poll
  	 * @param LogService $logService
@@ -56,13 +60,11 @@
  	 */
 
  	public function __construct(
-		ILogger $logger,
  		PollMapper $pollMapper,
  		Poll $poll,
  		LogService $logService,
  		Acl $acl
  	) {
-		$this->logger = $logger;
  		$this->pollMapper = $pollMapper;
  		$this->poll = $poll;
  		$this->logService = $logService;
@@ -71,9 +73,10 @@
 
 
 	/**
-	 * list
+	 * Get list of polls
 	 * @NoAdminRequired
-	 * @return array
+	 * @return array Array of Poll
+	 * @throws NotAuthorizedException
 	 */
 
 	public function list() {
@@ -97,10 +100,11 @@
 	}
 
 	/**
-	 * get
+	 * get poll configuration
 	 * @NoAdminRequired
-	 * @param integer $pollId
-	 * @return array
+	 * @param int $pollId
+	 * @return Poll
+	 * @throws NotAuthorizedException
 	 */
  	public function get($pollId) {
 
@@ -113,10 +117,11 @@
  	}
 
 	/**
-	 * get
+	 * get poll configuration by token
 	 * @NoAdminRequired
-	 * @param integer $pollId
-	 * @return array
+	 * @param int $pollId
+	 * @return Poll
+	 * @throws NotAuthorizedException
 	 */
  	public function getByToken($token) {
 
@@ -129,57 +134,14 @@
  	}
 
 	/**
-	 * delete
+	 * Add poll
 	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 * @param integer $pollId
-	 * @return Poll
-	 */
-
-	public function delete($pollId) {
-		$this->poll = $this->pollMapper->find($pollId);
-
-		if (!$this->acl->setPollId($pollId)->getAllowEdit()) {
-			throw new NotAuthorizedException;
-		}
-
-		if ($this->poll->getDeleted()) {
-			$this->poll->setDeleted(0);
-		} else {
-			$this->poll->setDeleted(time());
-		}
-
-		$this->poll = $this->pollMapper->update($this->poll);
-		$this->logService->setLog($this->poll->getId(), 'deletePoll');
-
-		return $this->poll;
-	}
-
-	/**
-	 * deletePermanently
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 * @param integer $pollId
-	 * @return Poll
-	 */
-
-	public function deletePermanently($pollId) {
-		$this->poll = $this->pollMapper->find($pollId);
-
-		if (!$this->acl->setPollId($pollId)->getAllowEdit() || !$this->poll->getDeleted()) {
-			throw new NotAuthorizedException;
-		}
-
-		return $this->pollMapper->delete($this->poll);
-	}
-
-	/**
-	 * write
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
 	 * @param string $type
 	 * @param string $title
 	 * @return Poll
+	 * @throws NotAuthorizedException
+	 * @throws InvalidPollTypeException
+	 * @throws EmptyTitleException
 	 */
 
 	public function add($type, $title) {
@@ -221,11 +183,15 @@
 	}
 
 	/**
-	 * update
+	 * Update poll configuration
 	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 * @param Array $poll
+	 * @param int $pollId
+	 * @param array $poll
 	 * @return Poll
+	 * @throws NotAuthorizedException
+	 * @throws EmptyTitleException
+	 * @throws InvalidShowResultsException
+	 * @throws InvalidAccessException
 	 */
 
 	public function update($pollId, $poll) {
@@ -256,36 +222,93 @@
 		return $this->poll;
 	}
 
+
 	/**
-	 * clone
+	 * Switch deleted status (move to deleted polls)
 	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 * @param integer $pollId
+	 * @param int $pollId
 	 * @return Poll
+	 * @throws NotAuthorizedException
 	 */
-	public function clone($pollId) {
 
-		if (!$this->acl->setPollId($this->poll->getId())->getAllowView()) {
+	public function delete($pollId) {
+		$this->poll = $this->pollMapper->find($pollId);
+
+		if (!$this->acl->setPollId($pollId)->getAllowEdit()) {
 			throw new NotAuthorizedException;
 		}
 
+		if ($this->poll->getDeleted()) {
+			$this->poll->setDeleted(0);
+		} else {
+			$this->poll->setDeleted(time());
+		}
+
+		$this->poll = $this->pollMapper->update($this->poll);
+		$this->logService->setLog($this->poll->getId(), 'deletePoll');
+
+		return $this->poll;
+	}
+
+	/**
+	 * Delete poll
+	 * @NoAdminRequired
+	 * @param int $pollId
+	 * @return Poll the deleted poll
+	 * @throws NotAuthorizedException
+	 */
+
+	public function deletePermanently($pollId) {
 		$this->poll = $this->pollMapper->find($pollId);
 
-		$this->poll->setCreated(time());
-		$this->poll->setOwner(\OC::$server->getUserSession()->getUser()->getUID());
-		$this->poll->setTitle('Clone of ' . $this->poll->getTitle());
-		$this->poll->setDeleted(0);
-		$this->poll->setId(0);
+		if (!$this->acl->setPollId($pollId)->getAllowEdit() || !$this->poll->getDeleted()) {
+			throw new NotAuthorizedException;
+		}
 
-		$this->poll = $this->pollMapper->insert($this->poll);
-		$this->logService->setLog($this->poll->getId(), 'addPoll');
+		return $this->pollMapper->delete($this->poll);
+	}
 
-		$this->optionService->clone($pollId, $this->poll->getId());
+	/**
+	 * Clone poll
+	 * @NoAdminRequired
+	 * @param int $pollId
+	 * @return Poll
+	 * @throws NotAuthorizedException
+	 */
+	public function clone($pollId) {
 
-		return $this->poll;
+		$origin = $this->pollMapper->find($pollId);
+		if (!$this->acl->setPollId($origin->getId())->getAllowView()) {
+			throw new NotAuthorizedException;
+		}
+
+		$this->poll = new Poll();
+		$this->poll->setCreated(time());
+		$this->poll->setOwner(\OC::$server->getUserSession()->getUser()->getUID());
+		$this->poll->setTitle('Clone of ' . $origin->getTitle());
+		$this->poll->setDeleted(0);
+		$this->poll->setAccess('hidden');
 
+		$this->poll->setType($origin->getType());
+		$this->poll->setDescription($origin->getDescription());
+		$this->poll->setExpire($origin->getExpire());
+		$this->poll->setAnonymous($origin->getAnonymous());
+		$this->poll->setFullAnonymous($origin->getFullAnonymous());
+		$this->poll->setAllowMaybe($origin->getAllowMaybe());
+		$this->poll->setVoteLimit($origin->getVoteLimit());
+		$this->poll->setSettings($origin->getSettings());
+		$this->poll->setOptions($origin->getOptions());
+		$this->poll->setShowResults($origin->getShowResults());
+		$this->poll->setAdminAccess($origin->getAdminAccess());
+
+		return $this->pollMapper->insert($this->poll);
 	}
 
+	/**
+	 * Get valid values for configuration options
+	 * @NoAdminRequired
+	 * @return array
+	 */
 	public function getValidEnum() {
 		return [
 			'pollType' => $this->getValidPollType(),
@@ -294,14 +317,29 @@
 		];
 	}
 
+	/**
+	 * Get valid values for pollType
+	 * @NoAdminRequired
+	 * @return array
+	 */
 	private function getValidPollType() {
 		return ['datePoll', 'textPoll'];
 	}
 
+	/**
+	 * Get valid values for access
+	 * @NoAdminRequired
+	 * @return array
+	 */
 	private function getValidAccess() {
 		return ['hidden', 'public'];
 	}
 
+	/**
+	 * Get valid values for showResult
+	 * @NoAdminRequired
+	 * @return array
+	 */
 	private function getValidShowResults() {
 		return ['always', 'expired', 'never'];
 	}
diff --git a/lib/Service/ShareService.php b/lib/Service/ShareService.php
index 5d8b47c4..1bd9bcb8 100644
--- a/lib/Service/ShareService.php
+++ b/lib/Service/ShareService.php
@@ -24,54 +24,62 @@
 namespace OCA\Polls\Service;
 
 use Exception;
-
-use OCP\Security\ISecureRandom;
-
 use OCA\Polls\Exceptions\NotAuthorizedException;
 use OCA\Polls\Exceptions\InvalidUsername;
 
-use OCA\Polls\Db\Share;
+use OCP\Security\ISecureRandom;
+
+use OCA\Polls\Controller\SystemController;
 use OCA\Polls\Db\ShareMapper;
+use OCA\Polls\Db\Share;
 use OCA\Polls\Service\MailService;
 use OCA\Polls\Model\Acl;
-use OCA\Polls\Controller\SystemController;
 
 class ShareService {
 
+	/** @var SystemController */
+	private $systemController;
+
+	/** @var ShareMapper */
 	private $shareMapper;
+
+	/** @var Share */
 	private $share;
-	private $systemController;
+
+	/** @var MailService */
 	private $mailService;
+
+	/** @var Acl */
 	private $acl;
 
 	/**
 	 * ShareController constructor.
+	 * @param SystemController $systemController
 	 * @param ShareMapper $shareMapper
 	 * @param Share $share
-	 * @param SystemController $systemController
 	 * @param MailService $mailService
 	 * @param Acl $acl
 	 */
 	public function __construct(
+		SystemController $systemController,
 		ShareMapper $shareMapper,
 		Share $share,
-		SystemController $systemController,
 		MailService $mailService,
 		Acl $acl
 	) {
+		$this->systemController = $systemController;
 		$this->shareMapper = $shareMapper;
 		$this->share = $share;
-		$this->systemController = $systemController;
 		$this->mailService = $mailService;
 		$this->acl = $acl;
 	}
 
 	/**
-	 * get
 	 * Read all shares of a poll based on the poll id and return list as array
 	 * @NoAdminRequired
-	 * @param integer $pollId
-	 * @return array
+	 * @param int $pollId
+	 * @return array array of Share
+	 * @throws NotAuthorizedException
 	 */
 	public function list($pollId) {
 		if (!$this->acl->setPollId($pollId)->getAllowEdit()) {
@@ -82,8 +90,7 @@ class ShareService {
 	}
 
 	/**
-	 * getByToken
-	 * Get pollId by token
+	 * Get share by token
 	 * @NoAdminRequired
 	 * @param string $token
 	 * @return Share
@@ -93,11 +100,14 @@ class ShareService {
 	}
 
 	/**
-	 * Write a new share to the db and returns the new share as array
+	 * Add share
 	 * @NoAdminRequired
 	 * @param int $pollId
-	 * @param string $share
-	 * @return array
+	 * @param string $type
+	 * @param string $userId
+	 * @param string $userEmail
+	 * @return Share
+	 * @throws NotAuthorizedException
 	 */
 	public function add($pollId, $type, $userId, $userEmail = '') {
 
@@ -122,14 +132,16 @@ class ShareService {
 	}
 
 	/**
-	 * createPersonalShare
-	 * Write a new share to the db and returns the new share as array
+	 * Create a personal share from a public share
+	 * or update an email share with the username
 	 * @NoAdminRequired
 	 * @param string $token
 	 * @param string $userName
 	 * @return Share
+	 * @throws NotAuthorizedException
+	 * @throws InvalidUsername
 	 */
-	public function createPersonalShare($token, $userName) {
+	public function personal($token, $userName) {
 		$publicShare = $this->shareMapper->findByToken($token);
 
 		// Return of validatePublicUsername is a DataResponse
@@ -142,7 +154,6 @@ class ShareService {
 
 		if ($publicShare->getType() === 'public') {
 
-
 			$this->share = new Share();
 			$this->share->setToken(\OC::$server->getSecureRandom()->generate(
 				16,
@@ -169,14 +180,15 @@ class ShareService {
 	}
 
 	/**
-	 * remove
+	 * Delete share
 	 * remove share
 	 * @NoAdminRequired
 	 * @param string $token
 	 * @return Share
+	 * @throws NotAuthorizedException
 	 */
 
-	public function remove($token) {
+	public function delete($token) {
 		$this->share = $this->shareMapper->findByToken($token);
 		if (!$this->acl->setPollId($this->share->getPollId())->getAllowEdit()) {
 			throw new NotAuthorizedException;
diff --git a/lib/Service/SubscriptionService.php b/lib/Service/SubscriptionService.php
index f378e476..8809f792 100644
--- a/lib/Service/SubscriptionService.php
+++ b/lib/Service/SubscriptionService.php
@@ -27,7 +27,6 @@ use Exception;
 use OCA\Polls\Exceptions\NotAuthorizedException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
 use OCP\AppFramework\Db\DoesNotExistException;
-use OCP\ILogger;
 
 use OCA\Polls\Db\Subscription;
 use OCA\Polls\Db\SubscriptionMapper;
@@ -35,30 +34,29 @@ use OCA\Polls\Model\Acl;
 
 class SubscriptionService  {
 
+	/** @var Acl */
 	private $acl;
+
+	/** @var SubscriptionMapper */
 	private $subscriptionMapper;
-	private $logger;
 
 	/**
 	 * SubscriptionController constructor.
 	 * @param SubscriptionMapper $subscriptionMapper
-	 * @param ILogger $logger
 	 * @param Acl $acl
 	 */
 
 	public function __construct(
 		SubscriptionMapper $subscriptionMapper,
-		ILogger $logger,
 		Acl $acl
 	) {
 		$this->subscriptionMapper = $subscriptionMapper;
 		$this->acl = $acl;
-		$this->logger = $logger;
 	}
 
 	/**
 	 * @NoAdminRequired
-	 * @param integer $pollId
+	 * @param int $pollId
 	 * @return array
 	 */
 	public function get($pollId) {
@@ -77,7 +75,7 @@ class SubscriptionService  {
 
 	/**
 	 * @NoAdminRequired
-	 * @param integer $pollId
+	 * @param int $pollId
 	 * @return array
 	 */
 	public function set($pollId, $subscribed) {
@@ -111,15 +109,15 @@ class SubscriptionService  {
 		} catch (MultipleObjectsReturnedException $e) {
 			// Duplicates should not exist but if found, fix it
 			// unsubscribe from all and resubscribe, if requested
-			$this->logger->debug('Multiple subscription (dulpicates) found');
+			\OC::$server->getLogger()->debug('Multiple subscription (dulpicates) found');
 			$this->subscriptionMapper->unsubscribe($pollId, $this->acl->getUserId());
-			$this->logger->debug('Unsubscribed all for user ' . $this->acl->getUserId() . 'in poll' . $pollId);
+			\OC::$server->getLogger()->debug('Unsubscribed all for user ' . $this->acl->getUserId() . 'in poll' . $pollId);
 			if ($subscribed) {
 				$subscription = new Subscription();
 				$subscription->setPollId($pollId);
 				$subscription->setUserId($this->acl->getUserId());
 				$this->subscriptionMapper->insert($subscription);
-				$this->logger->debug('Added new subscription');
+				\OC::$server->getLogger()->debug('Added new subscription');
 				return $subscription;
 			} else {
 				return ['status' => 'Unsubscribed from poll ' . $pollId];
diff --git a/lib/Service/VoteService.php b/lib/Service/VoteService.php
index 4df01c70..5073b732 100644
--- a/lib/Service/VoteService.php
+++ b/lib/Service/VoteService.php
@@ -27,8 +27,8 @@ use Exception;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCA\Polls\Exceptions\NotAuthorizedException;
 
-use OCA\Polls\Db\Vote;
 use OCA\Polls\Db\VoteMapper;
+use OCA\Polls\Db\Vote;
 use OCA\Polls\Db\OptionMapper;
 use OCA\Polls\Service\AnonymizeService;
 use OCA\Polls\Service\LogService;
@@ -36,11 +36,22 @@ use OCA\Polls\Model\Acl;
 
 class VoteService  {
 
+	/** @var VoteMapper */
 	private $voteMapper;
+
+	/** @var Vote */
 	private $vote;
+
+	/** @var OptionMapper */
 	private $optionMapper;
+
+	/** @var AnonymizeService */
 	private $anonymizer;
+
+	/** @var LogService */
 	private $logService;
+
+	/** @var Acl */
 	private $acl;
 
 	/**
@@ -69,12 +80,12 @@ class VoteService  {
 	}
 
 	/**
-	 * Get all votes of given poll
 	 * Read all votes of a poll based on the poll id and return list as array
 	 * @NoAdminRequired
-	 * @param integer $pollId
+	 * @param int $pollId
 	 * @param string $token
-	 * @return Vote
+	 * @return array
+	 * @throws NotAuthorizedException
 	 */
 	public function list($pollId = 0, $token = '') {
 		if (!$this->acl->setPollIdOrToken($pollId, $token)->getAllowView()) {
@@ -92,24 +103,29 @@ class VoteService  {
 	}
 
 	/**
-	 * set
+	 * Set vote
 	 * @NoAdminRequired
-	 * @param integer $pollId
-	 * @param Array $option
+	 * @param int $optionId
 	 * @param string $setTo
 	 * @param string $token
 	 * @return Vote
+	 * @throws NotAuthorizedException
 	 */
-	public function set($pollId = 0, $pollOptionText, $setTo, $token = '') {
+	public function set($optionId, $setTo, $token = '') {
+
+		$option = $this->optionMapper->find($optionId);
+		$pollId = $option->getPollId();
 
-		if (!$this->acl->setPollIdOrToken($pollId, $token)->getAllowVote()) {
+		if (!$this->acl->setPollIdOrToken($option->getPollId(), $token)->getAllowVote()) {
 			throw new NotAuthorizedException;
 		}
 
-		$option = $this->optionMapper->findByPollAndText($this->acl->getpollId(), $pollOptionText);
+		if (!$option->getPollId() === $this->acl->getPollId()) {
+			throw new NotAuthorizedException;
+		}
 
 		try {
-			$this->vote = $this->voteMapper->findSingleVote($this->acl->getpollId(), $option->getPollOptionText(), $this->acl->getUserId());
+			$this->vote = $this->voteMapper->findSingleVote($this->acl->getPollId(), $option->getPollOptionText(), $this->acl->getUserId());
 			$this->vote->setVoteAnswer($setTo);
 			$this->voteMapper->update($this->vote);
 
@@ -117,7 +133,7 @@ class VoteService  {
 			// Vote does not exist, insert as new Vote
 			$this->vote = new Vote();
 
-			$this->vote->setPollId($this->acl->getpollId());
+			$this->vote->setPollId($this->acl->getPollId());
 			$this->vote->setUserId($this->acl->getUserId());
 			$this->vote->setVoteOptionText($option->getPollOptionText());
 			$this->vote->setVoteOptionId($option->getId());
@@ -125,19 +141,19 @@ class VoteService  {
 			$this->voteMapper->insert($this->vote);
 
 		} finally {
-			$this->logService->setLog($this->vote->getPollId(), 'setVote', $this->vote->getUserId());
+			$this->logService->setLog($this->acl->getPollId(), 'setVote', $this->vote->getUserId());
 			return $this->vote;
 		}
 	}
 
 	/**
-	 * delete
+	 * Remove user from poll
 	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 * @param integer $voteId
+	 * @param int $voteId
 	 * @param string $userId
-	 * @param integer $pollId
+	 * @param int $pollId
 	 * @return Vote
+	 * @throws NotAuthorizedException
 	 */
 	public function delete($pollId, $userId) {