QE-903: import #3841
QE-903: import #3841
Conversation
nonsensical code styling rules imposed upon us
…per line in a multi-line
…to Task/QE-903-create-service-for-activate-and-deactivate-survey
…survey' into feature/QE-903-import
| * @return integer number of rows affected by the execution. | ||
| * @throws CDbException execution failed | ||
| */ | ||
| public function createTableFromPattern($table, $pattern, $columns = [], $where = []) |
There was a problem hiding this comment.
whats the reason not to use Yii::app()->db->createCommand()->renameTable($sOldSurveyTableName, $sNewSurveyTableName); like it was before
There was a problem hiding this comment.
@ptelu because
- We save the (sid, gid, qidsuffix, type) of the questions of the survey and we do not intend to rename lime_questions and rendering the survey site unusable (the reason why we do this archivation is that later when the survey is to be reactivated, then we can compare the type of each question with the type it had at the time of deactivation to know whether we can restore the data for the question)
- When we restore the tokens and the timings, we copy the tables and we intend to allow the archvies to exist
| * @return integer number of rows affected by the execution. | ||
| * @throws CDbException execution failed | ||
| */ | ||
| public function createTableFromPattern($table, $pattern, $columns = [], $where = []) |
There was a problem hiding this comment.
a lot of the functions being added here are very specific to archiving procedure and should be located there as well
There was a problem hiding this comment.
@ptelu can you elaborate on what you mean by archiving procedure?
There was a problem hiding this comment.
everything we are doing here, deactivate -> save response as archive table activate -> import archived table
There was a problem hiding this comment.
@ptelu I would gladly move these functions to the appropriate place, but I do not understand where do you intend them to be moved. Can you elaborate?
| * @throws CException execution failed | ||
| */ | ||
| public function getUnchangedColumns($sid, $sTimestamp, $qTimestamp) | ||
| { |
There was a problem hiding this comment.
- in core we have 2 modes when importing and it can also happen that the table is incompatible. I cant find where this is checked in this code and in its calling function restoreData() for core it was/is called DataEntry->isCompatible()
- we don't want to use plain sql to prevent compatibility issue as well as maintain readability, i know you wanted to rewrite because it was slow before but im sure this can also be expressed with the yii query builder
There was a problem hiding this comment.
- this is happening at reactivation, so we want to restore the latest of the data and we check for question compatibility rather than table compatibility. Question compatibility is being checked at
The reason for this is that people are deactivating their surveys to make edits upon them and we need to recover their latest responses. If a question was removed since then, or created since then, then we simply do not have anything to recover for it.
If a question has its type changed then we don't recover data for it either.
But if the question has the same type as it had before, then we recover the data.
So if someone just deactivated the survey and 3 questions are not recoverable, but 97 other questions are recoverable, then we recover the 97 questions that can be recovered instead or rendering the table incompatible.
- Okay, I will look into Yii query builder.
There was a problem hiding this comment.
@ptelu are you sure this can be rewritten with Yii's query builder?
I wonder how we can load column and table data from information_schema without massive performance loss using the query builder.
There was a problem hiding this comment.
@ptelu can you elaborate on the compatibility problem you are worried of? Can you describe an example when the current solution could go wrong according to you?
| $sParts = explode("_", $archives['survey']); | ||
| $sTimestamp = $sParts[count($sParts) - 1]; | ||
| $dynamicColumns = $this->app->getUnchangedColumns($sid, $sTimestamp, $qTimestamp); | ||
| $this->app->recoverSurveyResponses($this->app->db->tablePrefix . "survey_" . $sid, $archives["survey"], $dynamicColumns); |
There was a problem hiding this comment.
i think encryption got forgotten here, which is needed when the encryption got changed to on or off, in core included in DataEntry->import()
There was a problem hiding this comment.
@ptelu indeed, you are right, we need to make sure the encryption/decryption is being executed on the attributes whose encryption-state has changed (either they were encrypted but they are no longer, or they were not encrypted but they are encrypted now). We will need to make sure that we do a batch operation due to performance-related concerns. Thanks for pointing that out.
The goal of this PR is to import the archived data back when the survey is activated again, which can be set by passing restore: true as a parameter to the request.