View Issue Details

This bug affects 1 person(s).
 8
IDProjectCategoryView StatusLast Update
17120Bug reportsSurvey editingpublic2021-07-12 11:53
Reportergabrieljenik Assigned Togabrieljenik  
PrioritynoneSeverityminor 
Status closedResolutionfixed 
Product Version4.4.0-RC3 
Summary17120: After picking a new question theme, the settings are not properly updated
DescriptionIf you change from theme "A" (saved) to theme "B", the settings displayed are the ones from theme "A".
Additional InformationThis happens using simple selector as well as full selector
TagsNo tags attached.
Bug heat8
Complete LimeSurvey version number (& build)4.4.0-RC5
I will donate to the project if issue is resolvedNo
Browsernot relevant
Database & DB-Versionnot relevant
Server OS (if known)not relevant
Webserver software & version (if known)not relevant
PHP Versionnot relevant

Relationships

related to 17022 closedgabrieljenik QuestionTheme : specific categories are not updated 

Activities

DenisChenu

DenisChenu

2021-02-22 15:26

developer   ~62438

Near this one : https://bugs.limesurvey.org/view.php?id=17022

QuestionTheme : specific categories are not updated
gabrieljenik

gabrieljenik

2021-03-05 14:52

manager   ~62763

Last edited: 2021-04-07 21:21

View 2 revisions

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
gabrieljenik

gabrieljenik

2021-03-30 20:27

manager   ~63728

Last edited: 2021-04-07 21:21

ping @ollehar
ollehar

ollehar

2021-03-31 11:03

administrator   ~63733

Last edited: 2021-04-07 21:21

I'm not sure I made that commit. Is it a merge?
ollehar

ollehar

2021-03-31 11:03

administrator   ~63734

Last edited: 2021-04-07 21:21

Gah. I have to think.
ollehar

ollehar

2021-03-31 11:21

administrator   ~63737

Last edited: 2021-04-07 21:21

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.
DenisChenu

DenisChenu

2021-03-31 11:30

developer   ~63738

Last edited: 2021-04-07 21:21

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)
ollehar

ollehar

2021-03-31 11:32

administrator   ~63739

Last edited: 2021-04-07 21:21

> Theme must be inside question table

Oh yeah. Weird that it isn't.
DenisChenu

DenisChenu

2021-03-31 11:41

developer   ~63740

Last edited: 2021-04-07 21:21

Move it (with DB upgrade) in 5.X ?
ollehar

ollehar

2021-03-31 11:42

administrator   ~63741

Last edited: 2021-04-07 21:21

Which 5.x? We want to release 4.5 next week. :) Add it to 4.6 if it doesn't brake anything, maybe?
DenisChenu

DenisChenu

2021-03-31 12:46

developer   ~63742

Last edited: 2021-04-07 21:21

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
ollehar

ollehar

2021-03-31 12:47

administrator   ~63743

Last edited: 2021-04-07 21:21

All question theme issues are assigned to Gabriel right now. We'll try to involve tpartner when Gabriel is ready.
gabrieljenik

gabrieljenik

2021-03-31 16:20

manager   ~63758

Last edited: 2021-04-07 21:21

> 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!
ollehar

ollehar

2021-03-31 16:32

administrator   ~63761

Last edited: 2021-04-07 21:21

May the force be with you. ^^
gabrieljenik

gabrieljenik

2021-04-07 21:15

manager   ~63863

Last edited: 2021-04-07 21:21

View 2 revisions

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.
DenisChenu

DenisChenu

2021-04-08 08:39

developer   ~63864

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).
gabrieljenik

gabrieljenik

2021-04-08 14:20

manager   ~63868

> 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.
gabrieljenik

gabrieljenik

2021-04-08 17:11

manager   ~63884

Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=31563
c_schmitz

c_schmitz

2021-07-12 11:53

administrator   ~65321

Release done.

Related Changesets

LimeSurvey: master fe3a90d8

2021-04-08 17:11:06

gabrieljenik


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

Issue History

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
2021-07-12 11:53 c_schmitz Note Added: 65321
2021-07-12 11:53 c_schmitz Bug heat 6 => 8
2021-07-12 11:53 c_schmitz Status resolved => closed