View Issue Details

This bug affects 1 person(s).
 4
IDProjectCategoryView StatusLast Update
18699Bug reportsOtherpublic2023-03-24 09:04
Reportergabrieljenik Assigned To 
PrioritynoneSeverityblock 
Status confirmedResolutionopen 
Product Version5.6.x 
Summary18699: LSActiveRecord::getMaxId() doesn't get refreshed when using from SurveyDynamic with multiple surveys on the same HTTP request
Description

Caught while reviewing 18655
https://github.com/LimeSurvey/LimeSurvey/pull/2998#pullrequestreview-1352365154


GJ: The diema is this is only happening on these tests as the model is used with different surveyIds within the same HTTP request. This is not happening on real life.

DC: Until we use in in Question::model and Survey::model in same call …
There are an issue in this code … there are an easy way to fix : why you don't want to fix it ?


A workaround was implemented here for being able to use the remote_control->export_responses.
https://github.com/LimeSurvey/LimeSurvey/pull/2998

Steps To Reproduce

Steps to reproduce

Undo this change
https://github.com/LimeSurvey/LimeSurvey/pull/2998/files#diff-22d09ac4b2de7e29721c3fc14eb239f215498a280b8c37f4b718074646f01f5dR3168

Make a script for RC for exporting rsposnes from survey A and then from survey B.

Expected result

Survey B responses will be exported.

Actual result

No survey B responses will be exported.

TagsNo tags attached.
Bug heat4
Complete LimeSurvey version number (& build)5.6.7
I will donate to the project if issue is resolvedNo
Browser
Database type & versionMySQL
Server OS (if known)
Webserver software & version (if known)
PHP Version8.0

Relationships

related to 18655 resolvedc_schmitz Data loss if field in response table is missing 

Users monitoring this issue

User List There are no users monitoring this issue.

Activities

gabrieljenik

gabrieljenik

2023-03-23 14:37

manager   ~74246

I would go with:

1- Add a new optional parameter to getMaxId
public function getMaxId($field = null, $forceRefresh = false, $clear=false)

2 - Create LSActiveRecord::refreshMetadata() (overload of CActiveRecord).
Make the refreshMatedata to call getMaxId using the clear parameter.

gabrieljenik

gabrieljenik

2023-03-23 14:38

manager   ~74247

DC said...

a lot must be automatic. If not : it's unusuable

For example, you can do

$maxId1 = \Group::model()->getMaxId();
$maxId2 = \QuestionGroup::model()->getMaxId();
$maxId3 = \Token::model(123)->getMaxId();
$maxId4 = \SurveyDynamic::model(123)->getMaxId();
$maxId5 = \SurveyDynamic::model(321)->getMaxId();
$maxId6 = \Response::model(321)->getMaxId();

You must have the real $maxId at each time.

And more

$maxId2 = \QuestionGroup::model()->getMaxId();
$oQuestionGroup = new \QuestionGroup());
…
$oQuestionGroup->save();
$maxId3 = \QuestionGroup::model()->getMaxId();
gabrieljenik

gabrieljenik

2023-03-23 14:38

manager   ~74248

DC Said

A question : did we really need this one ? Maybe just delete this statci var

I think it's best to optimize other request (and use static or cache)

https://bugs.limesurvey.org/view.php?id=18473
https://bugs.limesurvey.org/view.php?id=18472
etc …

DenisChenu

DenisChenu

2023-03-23 14:53

developer   ~74250

There are an issue in this code … there are an easy way to fix : why you don't want to fix it ?

Not so easy … ;)

gabrieljenik

gabrieljenik

2023-03-23 15:37

manager   ~74251

haha I was trying to port the conversation :)

DenisChenu

DenisChenu

2023-03-23 15:41

developer   ~74252

Draft broken test : https://github.com/LimeSurvey/LimeSurvey/pull/3006

I like to keep some statci var when multiple load too

For example : wen you ask Options for Survey #1 in Group #1 : you ask option in Survey #1 + option on Group #1 + Global option , the n if you as after option in Survey #2 : i like to keep option on Group #1 + Global option

Same for Template option (where you can have more than on Global option …)

maxIds is a good test for all of this ;)

gabrieljenik

gabrieljenik

2023-03-23 21:59

manager   ~74259

Is it ok if I take this?
I will do a test script as well as the fix

DenisChenu

DenisChenu

2023-03-24 09:04

developer   ~74270

I think i get it
https://github.com/LimeSurvey/LimeSurvey/pull/3006

I need to add same for minIds

OK ?

Issue History

Date Modified Username Field Change
2023-03-23 14:35 gabrieljenik New Issue
2023-03-23 14:35 gabrieljenik Issue generated from: 18655
2023-03-23 14:35 gabrieljenik Relationship added related to 18655
2023-03-23 14:36 gabrieljenik Description Updated
2023-03-23 14:37 gabrieljenik Note Added: 74246
2023-03-23 14:37 gabrieljenik Bug heat 0 => 2
2023-03-23 14:38 gabrieljenik Note Added: 74247
2023-03-23 14:38 gabrieljenik Note Added: 74248
2023-03-23 14:53 DenisChenu Note Added: 74250
2023-03-23 14:53 DenisChenu Bug heat 2 => 4
2023-03-23 15:37 gabrieljenik Note Added: 74251
2023-03-23 15:41 DenisChenu Note Added: 74252
2023-03-23 21:59 gabrieljenik Note Added: 74259
2023-03-23 22:00 gabrieljenik Status new => confirmed
2023-03-24 09:04 DenisChenu Note Added: 74270