Skip to content

Commit

Permalink
Fixed issue #18582: RemoteControl invite_participants unexpectedly st…
Browse files Browse the repository at this point in the history
…ops after first failure (#2866)

Co-authored-by: lapiudevgit <devgit@lapiu.biz>
  • Loading branch information
gabrieljenik and lapiudevgit committed Jan 26, 2023
1 parent 58f50f7 commit a406818
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 9 deletions.
15 changes: 12 additions & 3 deletions application/helpers/admin/token_helper.php
Expand Up @@ -19,9 +19,10 @@
* @param integer $iSurveyID
* @param CActiveRecord[] $aResultTokens
* @param string $sType type of notification invite|register|remind|confirm
* @param bool $continueOnError Don't stop on first invalid participant
* @return array of results
*/
function emailTokens($iSurveyID, $aResultTokens, $sType)
function emailTokens($iSurveyID, $aResultTokens, $sType, $continueOnError = false)
{
if (!in_array($sType, ['invite','remind','register','confirm'])) {
throw new Exception('Invalid email type');
Expand All @@ -43,7 +44,11 @@ function emailTokens($iSurveyID, $aResultTokens, $sType)
'status' => 'fail',
'error' => 'Token not valid yet'
);
break 1;
if ($continueOnError) {
continue;
} else {
break 1;
}
}
if (isset($aTokenRow['validuntil']) && trim($aTokenRow['validuntil']) != '' && convertDateTimeFormat($aTokenRow['validuntil'], 'Y-m-d H:i:s', 'U') * 1 < date('U') * 1) {
$aResult[$aTokenRow['tid']] = array(
Expand All @@ -52,7 +57,11 @@ function emailTokens($iSurveyID, $aResultTokens, $sType)
'status' => 'fail',
'error' => 'Token not valid anymore'
);
break 1;
if ($continueOnError) {
continue;
} else {
break 1;
}
}
if ($mail->sendMessage()) {
$oToken = Token::model($iSurveyID)->findByPk($aTokenRow['tid']);
Expand Down
10 changes: 6 additions & 4 deletions application/helpers/remotecontrol/remotecontrol_handle.php
Expand Up @@ -2686,9 +2686,10 @@ public function mail_registered_participants($sSessionKey, $iSurveyID, $override
* @param int $iSurveyID ID of the survey that participants belong
* @param array $aTokenIds Ids of the participant to invite
* @param bool $bEmail Send only pending invites (TRUE) or resend invites only (FALSE)
* @param bool $continueOnError Don't stop on first invalid participant
* @return array Result of the action
*/
public function invite_participants($sSessionKey, $iSurveyID, $aTokenIds = null, $bEmail = true)
public function invite_participants($sSessionKey, $iSurveyID, $aTokenIds = null, $bEmail = true, $continueOnError = false)
{
Yii::app()->loadHelper('admin/token');
if (!$this->_checkSessionKey($sSessionKey)) {
Expand Down Expand Up @@ -2736,7 +2737,7 @@ public function invite_participants($sSessionKey, $iSurveyID, $aTokenIds = null,
if (empty($aResultTokens)) {
return array('status' => 'Error: No candidate tokens');
}
$aResult = emailTokens($iSurveyID, $aResultTokens, 'invite');
$aResult = emailTokens($iSurveyID, $aResultTokens, 'invite', $continueOnError);
$iLeft = $iAllTokensCount - count($aResultTokens);
$aResult['status'] = $iLeft . " left to send";

Expand All @@ -2758,9 +2759,10 @@ public function invite_participants($sSessionKey, $iSurveyID, $aTokenIds = null,
* @param int $iMinDaysBetween (optional) parameter days from last reminder
* @param int $iMaxReminders (optional) parameter Maximum reminders count
* @param array $aTokenIds Ids of the participant to remind (optional filter)
* @param bool $continueOnError Don't stop on first invalid participant
* @return array in case of success array of result of each email send action and count of invitations left to send in status key
*/
public function remind_participants($sSessionKey, $iSurveyID, $iMinDaysBetween = null, $iMaxReminders = null, $aTokenIds = false)
public function remind_participants($sSessionKey, $iSurveyID, $iMinDaysBetween = null, $iMaxReminders = null, $aTokenIds = false, $continueOnError = false)
{
Yii::app()->loadHelper('admin/token');
if (!$this->_checkSessionKey($sSessionKey)) {
Expand Down Expand Up @@ -2806,7 +2808,7 @@ public function remind_participants($sSessionKey, $iSurveyID, $iMinDaysBetween =
return array('status' => 'Error: No candidate tokens');
}

$aResult = emailTokens($iSurveyID, $aResultTokens, 'remind');
$aResult = emailTokens($iSurveyID, $aResultTokens, 'remind', $continueOnError);

$iLeft = $iAllTokensCount - count($aResultTokens);
$aResult['status'] = $iLeft . " left to send";
Expand Down
Binary file not shown.
3 changes: 1 addition & 2 deletions tests/unit/helpers/remotecontrol/BaseTest.php
Expand Up @@ -2,7 +2,7 @@

namespace ls\tests;

abstract class BaseTest extends TestHelper
abstract class BaseTest extends TestBaseClass
{
/* @var User */
public $user;
Expand All @@ -16,7 +16,6 @@ abstract class BaseTest extends TestHelper

protected function setUp(): void
{
$this->importAll();
\Yii::import('application.helpers.remotecontrol.remotecontrol_handle', true);

$user = \User::model()->findByPk(1);
Expand Down
90 changes: 90 additions & 0 deletions tests/unit/helpers/remotecontrol/InviteParticipantsTest.php
@@ -0,0 +1,90 @@
<?php

namespace ls\tests;

class InviteParticipantsTest extends BaseTest
{
public static function setUpBeforeClass(): void
{
parent::setUpBeforeClass();

/**
* Import survey with five participants. Participant 3 is invalid (validuntil in the past)
*/
$filename = self::$surveysFolder . '/limesurvey_survey_InviteParticipantsTest.lsa';
self::importSurvey($filename);
\Yii::app()->setController(new DummyController('dummyid'));
}

public function testInviteParticipants()
{
$sessionKey = $this->handler->get_session_key($this->getUsername(), $this->getPassword());
$result = $this->handler->invite_participants($sessionKey, self::$surveyId);
// Assert emails for participants 1 and 2 were sent
$this->assertParticipantResultIsOk($result, 1);
$this->assertParticipantResultIsOk($result, 2);
// Assert email for participant 3 was not sent
$this->assertParticipantResultIsNotOk($result, 3);
// Assert email sending stopped on error
$this->assertArrayNotHasKey(4, $result);
$this->assertArrayNotHasKey(5, $result);
}

public function testInviteParticipantsSkippingErrors()
{
$sessionKey = $this->handler->get_session_key($this->getUsername(), $this->getPassword());
$result = $this->handler->invite_participants($sessionKey, self::$surveyId, null, true, true);
// Assert emails for participants 1 and 2 were sent
$this->assertParticipantResultIsOk($result, 1);
$this->assertParticipantResultIsOk($result, 2);
// Assert email for participant 3 was not sent
$this->assertParticipantResultIsNotOk($result, 3);
// Assert emails for participants 4 and 5 were sent
$this->assertParticipantResultIsOk($result, 4);
$this->assertParticipantResultIsOk($result, 5);
}

/**
* Assert the results for a participant are OK.
* @param array<mixed> $results The raw results from invite_participants or remind_participants
* @param int $tid The participant ID
*/
private function assertParticipantResultIsOk($results, $tid)
{
$this->assertArrayHasKey($tid, $results);
$participantResults = $results[$tid];
$this->assertArrayHasKey("status", $participantResults);
$isOk = $this->isParticipantResultOk($participantResults, $tid);
$this->assertTrue($isOk);
}

/**
* Assert the results for a participant are not OK.
* @param array<mixed> $results The raw results from invite_participants or remind_participants
* @param int $tid The participant ID
*/
private function assertParticipantResultIsNotOk($results, $tid)
{
$this->assertArrayHasKey($tid, $results);
$participantResults = $results[$tid];
$this->assertArrayHasKey("status", $participantResults);
$isOk = $this->isParticipantResultOk($participantResults, $tid);
$this->assertFalse($isOk);
}

/**
* Returns true if the results for a participant are OK.
* @param array<mixed> Specific participant result from invite_participants or remind_participants
* @param int $tid The participant ID
* @return bool
*/
private function isParticipantResultOk($results, $tid)
{
// For this test, the result is considered OK if it's actually OK (status = OK),
// or if it failed because email function couldn't be instantiated, as we don't
// have a simple way to mock the LimeMailer. We could use a plugin that returns
// true on beforeTokenEmail, but it doesn't seem much better than this.
$validError = 'Could not instantiate mail function.';
return $results['status'] == 'OK' || (!empty($results['error']) && $results['error'] == $validError);
}
}

0 comments on commit a406818

Please sign in to comment.