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 #17991: Importing a question from the "List questions" menu fails with 400 error #2349

Conversation

gabrieljenik
Copy link
Collaborator

@gabrieljenik gabrieljenik commented Apr 13, 2022

This is just a workaround.

We still need to review if the change below (making the groupid mandatory) is still necesarry to review a more integral approach. e877a50#diff-8ba52d76c0ab5c25746214e40fd926cfdb0ae2024bd7bcdd90f1035014f5be12R141

If not necessary, the urls that point to the screen may need some reviewal in the order of the parameters and the ones being sent.

@glimz glimz added Tested OK This PR has been tested by QA and works as expected and removed Needs testing labels Apr 29, 2022
@glimz glimz requested a review from Shnoulle April 29, 2022 09:13
@Shnoulle
Copy link
Collaborator

public function importView($groupid = 0, $surveyid)

work without issue, seem better.

And the best : public function importView($surveyid, $groupid = null) if it can be done without updating other function

Copy link
Collaborator

@Shnoulle Shnoulle left a comment

Choose a reason for hiding this comment

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

Seem better to update function to allow empty groupid

@@ -54,7 +54,7 @@
<span class="icon-add"></span>
<?php eT("Add new question"); ?>
</a>
<a class="btn btn-default" href='<?php echo $this->createUrl("admin/questions/sa/importview/surveyid/".$oSurvey->sid); ?>' role="button">
<a class="btn btn-default" href='<?php echo $this->createUrl("admin/questions/sa/importview/surveyid/" . $oSurvey->sid . "/groupid"); ?>' role="button">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move to surveyid as param. And really dislike "" for param. 0 or update function

@sonarcloud
Copy link

sonarcloud bot commented Apr 29, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Shnoulle Shnoulle self-requested a review May 6, 2022 11:56
@Shnoulle Shnoulle added Code review done Version checked for code issue without testing and removed Needs update by author Needs code review labels May 6, 2022
@Shnoulle
Copy link
Collaborator

Shnoulle commented May 6, 2022

Sorry for delay …

@olleharstedt olleharstedt merged commit 02783b4 into LimeSurvey:3.x-LTS May 10, 2022
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
5 participants