From f921d383a953fd9ddeaecfac103beced7f872f2b Mon Sep 17 00:00:00 2001 From: m Date: Fri, 6 Apr 2018 14:00:20 +0200 Subject: [PATCH 1/4] bug/test-name-on-vote-edit --- adminstuds.php | 2 ++ .../Framadate/Repositories/VoteRepository.php | 15 +++++++++++++++ app/classes/Framadate/Services/PollService.php | 7 ++++++- studs.php | 4 +++- 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/adminstuds.php b/adminstuds.php index ab2cacd..5555b82 100644 --- a/adminstuds.php +++ b/adminstuds.php @@ -223,6 +223,8 @@ if (!empty($_POST['save'])) { // Save edition of an old vote } else { $message = new Message('danger', __('Error', 'Update vote failed')); } + } catch (AlreadyExistsException $aee) { + $message = new Message('danger', __('Error', 'The name you\'ve chosen already exist in this poll!')); } catch (ConcurrentEditionException $cee) { $message = new Message('danger', __('Error', 'Poll has been updated before you vote')); } catch (ConcurrentVoteException $cve) { diff --git a/app/classes/Framadate/Repositories/VoteRepository.php b/app/classes/Framadate/Repositories/VoteRepository.php index 1a74f5a..682357c 100644 --- a/app/classes/Framadate/Repositories/VoteRepository.php +++ b/app/classes/Framadate/Repositories/VoteRepository.php @@ -85,4 +85,19 @@ class VoteRepository extends AbstractRepository { $prepared->execute([$poll_id, $name]); return $prepared->rowCount() > 0; } + + /** + * Check if name is already used for the given poll and another vote. + * + * @param int $poll_id ID of the poll + * @param string $name Name of the vote + * @param int $vote_id ID of the current vote + * @return bool true if vote already exists + */ + public function existsByPollIdAndNameAndVoteId($poll_id, $name, $vote_id) { + $prepared = $this->prepare('SELECT 1 FROM `' . Utils::table('vote') . '` WHERE poll_id = ? AND name = ? AND id != ?'); + $prepared->execute([$poll_id, $name, $vote_id]); + return $prepared->rowCount() > 0; + } + } diff --git a/app/classes/Framadate/Services/PollService.php b/app/classes/Framadate/Services/PollService.php index ad180b1..8e19789 100644 --- a/app/classes/Framadate/Services/PollService.php +++ b/app/classes/Framadate/Services/PollService.php @@ -100,7 +100,12 @@ class PollService { // Check if slots are still the same $this->checkThatSlotsDidntChanged($poll, $slots_hash); - + + // Check if vote already exists with the same name + if ($this->voteRepository->existsByPollIdAndNameAndVoteId($poll_id, $name, $vote_id)) { + throw new AlreadyExistsException(); + } + // Update vote $choices = implode($choices); return $this->voteRepository->update($poll_id, $vote_id, $name, $choices); diff --git a/studs.php b/studs.php index 6cef3ee..2d5efab 100644 --- a/studs.php +++ b/studs.php @@ -145,7 +145,9 @@ if ($accessGranted) { } else { $message = new Message('danger', __('Error', 'Update vote failed')); } - } catch (ConcurrentEditionException $cee) { + } catch (AlreadyExistsException $aee) { + $message = new Message('danger', __('Error', 'The name you\'ve chosen already exist in this poll!')); + } catch (ConcurrentEditionException $cee) { $message = new Message('danger', __('Error', 'Poll has been updated before you vote')); } catch (ConcurrentVoteException $cve) { $message = new Message('danger', __('Error', "Your vote wasn't counted, because someone voted in the meantime and it conflicted with your choices and the poll conditions. Please retry.")); From c4f27dc6e0380ed9713b6030715b1c2071ea9738 Mon Sep 17 00:00:00 2001 From: m Date: Fri, 6 Apr 2018 14:04:07 +0200 Subject: [PATCH 2/4] code style --- app/classes/Framadate/Repositories/VoteRepository.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/classes/Framadate/Repositories/VoteRepository.php b/app/classes/Framadate/Repositories/VoteRepository.php index 682357c..cbbbd77 100644 --- a/app/classes/Framadate/Repositories/VoteRepository.php +++ b/app/classes/Framadate/Repositories/VoteRepository.php @@ -99,5 +99,5 @@ class VoteRepository extends AbstractRepository { $prepared->execute([$poll_id, $name, $vote_id]); return $prepared->rowCount() > 0; } - } + From 179235eaf98251af5e428541d238b342beb38c56 Mon Sep 17 00:00:00 2001 From: m Date: Fri, 6 Apr 2018 21:16:28 +0200 Subject: [PATCH 3/4] refactoring vote checks --- .../Framadate/Services/PollService.php | 67 +++++++++++-------- 1 file changed, 39 insertions(+), 28 deletions(-) diff --git a/app/classes/Framadate/Services/PollService.php b/app/classes/Framadate/Services/PollService.php index 8e19789..7ccfe7b 100644 --- a/app/classes/Framadate/Services/PollService.php +++ b/app/classes/Framadate/Services/PollService.php @@ -88,29 +88,19 @@ class PollService { * @param $name * @param $choices * @param $slots_hash + * @throws AlreadyExistsException * @throws ConcurrentEditionException * @throws ConcurrentVoteException * @return bool */ public function updateVote($poll_id, $vote_id, $name, $choices, $slots_hash) { - $poll = $this->findById($poll_id); - - // Check that no-one voted in the meantime and it conflicts the maximum votes constraint - $this->checkMaxVotes($choices, $poll, $poll_id); - - // Check if slots are still the same - $this->checkThatSlotsDidntChanged($poll, $slots_hash); - - // Check if vote already exists with the same name - if ($this->voteRepository->existsByPollIdAndNameAndVoteId($poll_id, $name, $vote_id)) { - throw new AlreadyExistsException(); - } + $this->checkVoteConstraints($choices, $poll_id, $slots_hash, $name, $vote_id); // Update vote $choices = implode($choices); return $this->voteRepository->update($poll_id, $vote_id, $name, $choices); } - + /** * @param $poll_id * @param $name @@ -122,19 +112,8 @@ class PollService { * @return \stdClass */ function addVote($poll_id, $name, $choices, $slots_hash) { - $poll = $this->findById($poll_id); - - // Check that no-one voted in the meantime and it conflicts the maximum votes constraint - $this->checkMaxVotes($choices, $poll, $poll_id); - - // Check if slots are still the same - $this->checkThatSlotsDidntChanged($poll, $slots_hash); - - // Check if vote already exists - if ($this->voteRepository->existsByPollIdAndName($poll_id, $name)) { - throw new AlreadyExistsException(); - } - + $this->checkVoteConstraints($choices, $poll_id, $slots_hash, $name, NULL); + // Insert new vote $choices = implode($choices); $token = $this->random(16); @@ -145,7 +124,8 @@ class PollService { if ($this->commentRepository->exists($poll_id, $name, $comment)) { return true; } - return $this->commentRepository->insert($poll_id, $name, $comment); + + return $this->commentRepository->insert($poll_id, $name, $comment); } /** @@ -312,7 +292,38 @@ class PollService { private function random($length) { return Token::getToken($length); } - + + /** + * @param $choices + * @param $poll_id + * @param $slots_hash + * @param $name + * @param string|NULL $vote_id + * @throws AlreadyExistsException + * @throws ConcurrentVoteException + * @throws ConcurrentEditionException + */ + private function checkVoteConstraints($choices, $poll_id, $slots_hash, $name, $vote_id) { + // Check if vote already exists with the same name + if (!isset($vote_id)) { + $exists = $this->voteRepository->existsByPollIdAndName($poll_id, $name); + } else { + $exists = $this->voteRepository->existsByPollIdAndNameAndVoteId($poll_id, $name, $vote_id); + } + + if ($exists) { + throw new AlreadyExistsException(); + } + + $poll = $this->findById($poll_id); + + // Check that no-one voted in the meantime and it conflicts the maximum votes constraint + $this->checkMaxVotes($choices, $poll, $poll_id); + + // Check if slots are still the same + $this->checkThatSlotsDidntChanged($poll, $slots_hash); + } + /** * This method checks if the hash send by the user is the same as the computed hash. * From 46df3b6fce418735acb36c14b326883740dd3720 Mon Sep 17 00:00:00 2001 From: m Date: Sun, 8 Apr 2018 11:29:03 +0200 Subject: [PATCH 4/4] change test --- app/classes/Framadate/Services/PollService.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/classes/Framadate/Services/PollService.php b/app/classes/Framadate/Services/PollService.php index 7ccfe7b..d3d6590 100644 --- a/app/classes/Framadate/Services/PollService.php +++ b/app/classes/Framadate/Services/PollService.php @@ -112,7 +112,7 @@ class PollService { * @return \stdClass */ function addVote($poll_id, $name, $choices, $slots_hash) { - $this->checkVoteConstraints($choices, $poll_id, $slots_hash, $name, NULL); + $this->checkVoteConstraints($choices, $poll_id, $slots_hash, $name); // Insert new vote $choices = implode($choices); @@ -298,14 +298,14 @@ class PollService { * @param $poll_id * @param $slots_hash * @param $name - * @param string|NULL $vote_id + * @param string $vote_id * @throws AlreadyExistsException * @throws ConcurrentVoteException * @throws ConcurrentEditionException */ - private function checkVoteConstraints($choices, $poll_id, $slots_hash, $name, $vote_id) { + private function checkVoteConstraints($choices, $poll_id, $slots_hash, $name, $vote_id = FALSE) { // Check if vote already exists with the same name - if (!isset($vote_id)) { + if (FALSE === $vote_id) { $exists = $this->voteRepository->existsByPollIdAndName($poll_id, $name); } else { $exists = $this->voteRepository->existsByPollIdAndNameAndVoteId($poll_id, $name, $vote_id);