Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed issue #19528: Bad usability when open in a new tab for participant edit #3826

Merged
merged 9 commits into from May 2, 2024

Conversation

Shnoulle
Copy link
Collaborator

Dev: use Yii::app()->request->isAjaxRequest

…ant edit

Dev: use Yii::app()->request->isAjaxRequest
@Shnoulle Shnoulle added Code review done Version checked for code issue without testing and removed Needs code review labels Apr 29, 2024
@tiborpacalat tiborpacalat added Tested OK This PR has been tested by QA and works as expected and removed Needs testing labels Apr 30, 2024
@@ -839,7 +838,14 @@ public function edit($iSurveyId, $iTokenId, $ajax = false)
foreach ($aAdditionalAttributeFields as $attr_name => $desc) {
$value = $request->getPost($attr_name, '');
if ($desc['mandatory'] == 'Y' && trim($value) == '') {
$sOutput .= sprintf(gT("Notice: Field '%s' was left empty, even though it is a mandatory attribute."), $desc['description']) . '<br>';
/* All this part is disable via JS, no way to submit : issue #19548 https://bugs.limesurvey.org/view.php?id=19548*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by disable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by disable?

I put the number of mantis issue : https://bugs.limesurvey.org/view.php?id=19548

Here it's the PHP part : allow to save empty value in required attribute : but a JS function disable this.

"surveyid" => $iSurveyId,
]
);
$this->getController()->redirect($redirectUrl);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this make sense if we are running on ajax mode?

Copy link
Collaborator Author

@Shnoulle Shnoulle May 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\ls\ajax\AjaxHelper::outputError($firstError[0]);
// App->end in AjaxHelper
}
App()->setFlashMessage(CHtml::errorSummary($token, '<div><strong>' . gT('The survey participant was not updated.') . '</strong></div>'), 'danger');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would run if on ajax mode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tiborpacalat tiborpacalat merged commit d506861 into master May 2, 2024
20 checks passed
@tiborpacalat tiborpacalat deleted the bug/19528_ajax_token branch May 2, 2024 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code review done Version checked for code issue without testing Tested OK This PR has been tested by QA and works as expected
Projects
None yet
3 participants