View Issue Details

This bug affects 1 person(s).
IDProjectCategoryView StatusLast Update
16488Bug reportsQuestion editorpublic2020-09-08 07:43
Reportergabrieljenik Assigned Togabrieljenik  
Status closedResolutionfixed 
Product Version3.22.25 
Summary16488: Reviewing updateQuestionOrder usage
DescriptionOn v4 we have noticed that sometimes question_order number is set following the survey_id.
That was due to a wrong usage of question::updateQuestionOrder() mehod and its parameters.

As the codebase for that seems similar on v3, review there if it used correctly and possible impacts
Additional InformationSee application/controllers/admin/database.php::756
                    // if the group has changed then fix the sortorder of old and new group
                    Question::model()->updateQuestionOrder($oldgid, $iSurveyID);
TagsNo tags attached.
Bug heat6
Complete LimeSurvey version number (& build)3.22.25
I will donate to the project if issue is resolvedNo
Database & DB-VersionMysql
Server OS (if known)
Webserver software & version (if known)
PHP Version7


related to 16454 closedgabrieljenik Sometimes, question_order is assigned from survey_id 




2020-07-10 20:47

manager   ~58904

The method `Question ::updateQuestionOrder()` receives 3 args: `gid`, `language`, `startPosition`.
The database controller calls this method wrongly three times directly, passing the survey ID as second parameter (where it was supposed to be language), so those invocations have no effect, as the questions query returns no items.

At last, when moving questions in between groups (to a lower group), the database controller calls the function shiftOrderQuestions (which in turn calls updateQuestionOrder). That seems to be OK and seems to be the only point where it's required.

Should we remove those bad invocations?


2020-08-04 19:54

manager   ~59307

- Removed language from Question::updateQuestionOrder parameters.
  The method now uses the survey base language to work.

- Changed how order is handled in `actionUpdateQuestion` (`database.php`)
- Removed call to `Question::updateQuestionOrder` in `actionInsertCopyQuestion()` (database.php), because order was already handled.
- Updated `shiftOrderQuestions` (common_helper.php) for not using language inside of it.
  This function is not used anymore in Limesurvey's codebase, but still it might be used by others, or in the future.
- Calls to Question::updateSortOrder where replaced by Question::updateQuestionOrder for consistency.
  `updateSortOrder` is not removed from the codebase, just in case.

Test scenarios:
- Insert Questions
- Edit question group attribute. Move to upper group
- Edit question group attribute. Move to lower group
- Move questions from sidebar to another group
- Move questions from sidebar within the same group
- Move questions from Reorganizer
- Copy from one group to another group
- Copy within the same group
- Delete
All times, order should be checked visually, refreshing screens and also on the DB.



2020-09-07 16:00

manager   ~59727

In the latest update getting a lot of: 500: Internal Server Error
CDbCommand failed to execute the SQL statement: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'question_order' cannot be null when saving questions. Not sure if related to the changes from here.


2020-09-08 02:36

manager   ~59729

Yes, new PR uploaded and rebased on curren lsv3


2020-09-08 07:33

manager   ~59730

Fix committed to 3.x-LTS branch:


2020-09-08 07:43

administrator   ~59731

Fixed in Release 3.23.3+200909

Related Changesets

LimeSurvey: 3.x-LTS 27d16225

2020-09-08 07:33:46


Committer: GitHub Details Diff
Fixed Issue 16488: [Hotfix] Reviewing updateQuestionOrder usage

Code review
Affected Issues
mod - application/controllers/admin/database.php Diff File

Issue History

Date Modified Username Field Change
2020-07-10 20:09 gabrieljenik New Issue
2020-07-10 20:09 gabrieljenik Issue generated from: 16454
2020-07-10 20:09 gabrieljenik Relationship added related to 16454
2020-07-10 20:47 gabrieljenik Note Added: 58904
2020-07-22 17:27 user225042 Status new => confirmed
2020-07-22 17:27 user225042 Zoho Sprints => |Yes|
2020-07-22 17:27 swendrich Zoho Sprints ID => 14469000000188037
2020-08-04 19:54 gabrieljenik Note Added: 59307
2020-09-07 16:00 cdorin Note Added: 59727
2020-09-08 02:36 gabrieljenik Note Added: 59729
2020-09-08 07:33 gabrieljenik Changeset attached => LimeSurvey 3.x-LTS 27d16225
2020-09-08 07:33 gabrieljenik Note Added: 59730
2020-09-08 07:33 gabrieljenik Assigned To => gabrieljenik
2020-09-08 07:33 gabrieljenik Resolution open => fixed
2020-09-08 07:43 lime_release_bot Zoho Sprints Yes => |Yes|
2020-09-08 07:43 lime_release_bot Note Added: 59731
2020-09-08 07:43 lime_release_bot Status confirmed => closed