Relationship Graph
View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
17120 | Bug reports | Survey editing | public | 2021-02-22 15:21 | 2021-04-22 16:19 |
Reporter | gabrieljenik | Assigned To | gabrieljenik | ||
Priority | none | Severity | minor | ||
Status | resolved | Resolution | fixed | ||
Product Version | 4.4.0-RC3 | ||||
Summary | 17120: After picking a new question theme, the settings are not properly updated | ||||
Description | If you change from theme "A" (saved) to theme "B", the settings displayed are the ones from theme "A". | ||||
Additional Information | This happens using simple selector as well as full selector | ||||
Tags | No tags attached. | ||||
Complete LimeSurvey version number (& build) | 4.4.0-RC5 | ||||
I will donate to the project if issue is resolved | No | ||||
Browser | not relevant | ||||
Database & DB-Version | not relevant | ||||
Server OS (if known) | not relevant | ||||
Webserver software & version (if known) | not relevant | ||||
PHP Version | not relevant | ||||
related to | 17022 | closed | gabrieljenik | QuestionTheme : specific categories are not updated |
Near this one : https://bugs.limesurvey.org/view.php?id=17022 QuestionTheme : specific categories are not updated |
|
Quick question: Why this change? 5a095ff The issue seems to be because we are pulling the theme from the last saved value on the DB. The requested theme is not saved on the DB yet. That's why it is not bringing up the expected attributes |
|
ping @ollehar | |
I'm not sure I made that commit. Is it a merge? | |
Gah. I have to think. | |
Is it enough to change this line? https://github.com/LimeSurvey/LimeSurvey/commit/5a095ff627bce3793fdd082e141c157c6dd4ecab#diff-107f8c81519e24601f4b3670bb92ccb4daa2c444134d1c14b24cce40b1e4d722R1779 Or to change $oQuestion->question_theme or whatever attribute. |
|
My opinion : original issue is set theme as a attribute like all other one. Theme must be inside question table (I think i tell this before question theme was ready as stable) |
|
> Theme must be inside question table Oh yeah. Weird that it isn't. |
|
Move it (with DB upgrade) in 5.X ? | |
Which 5.x? We want to release 4.5 next week. :) Add it to 4.6 if it doesn't brake anything, maybe? | |
Oh yes , better if we are sure to didn't broke anything. Since QuestionTheme didn't really work for now (?unsure on this point) @tparner must check if it's OK with existing QuestionTheme |
|
All question theme issues are assigned to Gabriel right now. We'll try to involve tpartner when Gabriel is ready. | |
> Is it enough to change this line? https://github.com/LimeSurvey/LimeSurvey/commit/5a095ff627bce3793fdd082e141c157c6dd4ecab#diff-107f8c81519e24601f4b3670bb92ccb4daa2c444134d1c14b24cce40b1e4d722R1779 > > Or to change $oQuestion->question_theme or whatever attribute. Yes, we would need to change some of those lines as to get the theme not from the DB but from the user selection. OK, having reviewed that commit 5a095ff can be safely changed, we will work on it. Thanks! |
|
May the force be with you. ^^ | |
PR: https://github.com/LimeSurvey/LimeSurvey/pull/1842 Code reorganization. - Added some comments. - Have created helper functions as to be able to have a better outlook of what some function do. Solution Summary. The key of the solution is to be able to pickup theme attributes from a theme name and not from a the attribute set on the question. Then added a theme override parameter which will set which theme attributes should be picked up. So now, question attributes may contain the attributes of a temp theme, which is not set on the DB. |
|
Another remark about New question , I think except Question theme : all advanced settings can be removed from the enw form. It make it simpliest. My opinoon : when copy/create have - theme - question code - question text - question help - Group - order in this group All other part included in Question model are interesting buit not mandatory. Clearly : advanced settings make a simpliest form for user (and i think : more easy for dev). |
|
> My opinion : original issue is set theme as a attribute like all other one. > Theme must be inside question table > Move it (with DB upgrade) in 5.X ? >Which 5.x? We want to release 4.5 next week. :) Add it to 4.6 if it doesn't brake anything, maybe? Agree Let's create a different ticket for that. Lot of code to be reviewed I think. |
|
Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=31563 | |
LimeSurvey: master fe3a90d8 2021-04-08 17:11:06 Committer: GitHub Details Diff |
Fixed issue 17120: After picking a new question theme, the settings are not properly updated (#1842) Code reorganization. - Added some comments. - Have created helper functions as to be able to have a better outlook of what some function do. Solution Summary. The key of the solution is to be able to pickup theme attributes from a theme name and not from a the attribute set on the question. Then added a theme override parameter which will set which theme attributes should be picked up. So now, question attributes may contain the attributes of a temp theme, which is not set on the DB. * Move QuestionAttribute related static methods to a new service class * Move QuestionAttribute related static methods to a new service class - Missing class Co-authored-by: encuestabizdevgit <devgit@encuesta.biz> |
Affected Issues 17120 |
|
mod - application/controllers/QuestionAdministrationController.php | Diff File | ||
mod - application/models/Question.php | Diff File | ||
mod - application/models/QuestionAttribute.php | Diff File | ||
mod - application/models/QuestionTheme.php | Diff File | ||
add - application/models/services/QuestionAttributeHelper.php | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2021-02-22 15:21 | gabrieljenik | New Issue | |
2021-02-22 15:21 | gabrieljenik | Issue generated from: 17018 | |
2021-02-22 15:26 | DenisChenu | Note Added: 62438 | |
2021-02-23 15:47 | gabrieljenik | Assigned To | => gabrieljenik |
2021-02-23 15:47 | gabrieljenik | Status | new => assigned |
2021-03-05 14:52 | gabrieljenik | Note Added: 62763 | |
2021-03-05 14:53 | gabrieljenik | Note Edited: 62763 | View Revisions |
2021-03-11 16:27 | gabrieljenik | Relationship added | related to 17022 |
2021-03-30 20:27 | gabrieljenik | Note Added: 63728 | |
2021-03-31 11:03 | ollehar | Note Added: 63733 | |
2021-03-31 11:03 | ollehar | Note Added: 63734 | |
2021-03-31 11:21 | ollehar | Note Added: 63737 | |
2021-03-31 11:30 | DenisChenu | Note Added: 63738 | |
2021-03-31 11:32 | ollehar | Note Added: 63739 | |
2021-03-31 11:41 | DenisChenu | Note Added: 63740 | |
2021-03-31 11:42 | ollehar | Note Added: 63741 | |
2021-03-31 12:46 | DenisChenu | Note Added: 63742 | |
2021-03-31 12:47 | ollehar | Note Added: 63743 | |
2021-03-31 16:20 | gabrieljenik | Note Added: 63758 | |
2021-03-31 16:32 | ollehar | Note Added: 63761 | |
2021-04-07 21:15 | gabrieljenik | Note Added: 63863 | |
2021-04-07 21:18 | gabrieljenik | Note Edited: 63863 | View Revisions |
2021-04-08 08:39 | DenisChenu | Note Added: 63864 | |
2021-04-08 14:20 | gabrieljenik | Note Added: 63868 | |
2021-04-08 17:11 | gabrieljenik | Changeset attached | => LimeSurvey master fe3a90d8 |
2021-04-08 17:11 | gabrieljenik | Note Added: 63884 | |
2021-04-08 17:11 | gabrieljenik | Resolution | open => fixed |
2021-04-22 16:19 | gabrieljenik | Status | assigned => resolved |