View Issue Details

This bug affects 1 person(s).
 6
IDProjectCategoryView StatusLast Update
17346Bug reportsQuestion editorpublic2021-07-05 17:39
Reportergabrieljenik Assigned Togabrieljenik  
PrioritynormalSeveritypartial_block 
Status acknowledgedResolutionopen 
Product Version5.x 
Target Version5.0.0dev 
Summary17346: Personal setting default question type is ignored when creating questions
DescriptionWhen selecting a default question type, ex shortfreetext, the array question type is shown as the selected question type.
Nevertheless the internal settings in memory do consider shortfreetext as the selected type.

This only happens with the simple selector.
Steps To Reproduce- Set default to short free text
- Create question.
- The default selected type is not short free text
Additional Information`QuestionTheme::findAllQuestionMetaDataForSelector` is setting name attribute as core for core types. That's usefull for the Personal Settings screen, although not good for the question editor.

We believe the "setting as core" should be not on the model.
TagsNo tags attached.
Bug heat6
Complete LimeSurvey version number (& build)5.0
I will donate to the project if issue is resolvedNo
Browser
Database & DB-VersionMysql
Server OS (if known)
Webserver software & version (if known)
PHP Version7

Relationships

related to 17310 confirmedgabrieljenik Simple question type selector didn't open with the current question type 

Users monitoring this issue

User List There are no users monitoring this issue.

Activities

DenisChenu

DenisChenu

2021-05-31 15:03

developer   ~64672

Are you sure it's saved ? Last time i check : nothing seems saved on user settings.
gabrieljenik

gabrieljenik

2021-05-31 15:09

manager   ~64674

Yes, that's what we are seeing
DenisChenu

DenisChenu

2021-05-31 15:13

developer   ~64675

Yes, see the other mantis after (i think i already reported it :( )
gabrieljenik

gabrieljenik

2021-05-31 15:17

manager   ~64676

Somehow there is a missmatch in between the core question theme and the names of the core question themes.
Ex: Short free text question theme is core and its name is shortfreetext. Usually core is used on the code, but on the data and the model the name is shortfreetext.
gabrieljenik

gabrieljenik

2021-06-07 17:42

manager   ~64770

Question themes have a name, like 'arrays/array' or 'bootstrap_buttons'.
There are base (core) question themes and extended question themes (custom or included with Lime).

Some parts of Lime only handle the question themes by name, and treat the core themes as "core" (instead of by their name).

For example, when handling question attributes, it's usually done by question type and question theme name, and 'core' is used as theme name for the basic ones.
That also applies to the question type/theme selector.

The specific problem with this ticket is that question type/theme metadata for the selected question (to mark the selected question type/theme in the selector) is retrieved with QuestionTheme::findQuestionMetaData(), which returns the "real" question theme data, including the saved name.
Since the selector has a list of question type + theme pairs where the theme is 'core' for the basic ones, but the question metadata has the actual theme name ('arrays/array', 'longfreetext', etc), no question type/theme is marked as selected (defaulting to the first one.

It's temporarily solved in T656 by replacing the theme name after retrieving the metadata.

Alternative better approaches follow:

Alternative A) Make the replacement inside findQuestionMetadata(). Other parts of Lime using that function don't rely on the theme name, so it wouldn't have an impact. But, the function is public, so it may be used by third party code.

Alternative B) Make a clone of findQuestionMetadata() adapted for returning "core" sometimes.

Alternative C) Review the usage for findAllQuestionMetaDataForSelector() and make it match to findQuestionMetadata().

Thoughts? @ollehar
DenisChenu

DenisChenu

2021-06-07 17:49

developer   ~64771

> Alternative A) … … so it may be used by third party code.

Young code, out of any API : don't really care, no ?

If you think of « it may be used by third party code» : you don't create i10n function like this.
ollehar

ollehar

2021-06-14 17:06

administrator   ~64898

> Somehow there is a missmatch in between the core question theme and the names of the core question themes.
> Ex: Short free text question theme is core and its name is shortfreetext. Usually core is used on the code, but on the data and the model the name is shortfreetext.

Can you give some example where this happens? There might be a hack present, not sure.

There are three types of question themes:

* core - the basic core themes included in the github repo
* custom - question theme included in repo but which extends a core theme
* user - question theme NOT included in repo but which extends a core/custom theme (so installed by the user)

Whenever you see "core", the variable should probably be called $questionThemeType.
ollehar

ollehar

2021-06-14 17:09

administrator   ~64899

Last edited: 2021-06-14 17:10

View 3 revisions

Just a NB about "findQuestionMetadata" - this function shouldn't exist. It should be $question->question_theme->config->metadata.

Questions have no metadata, only question themes do (and all other extensions like survey theme and plugins). There's some conceptual confusion here I want to sort out together with Patrick as soon as he's finished with encryption (again).
ollehar

ollehar

2021-06-14 17:16

administrator   ~64900

Last edited: 2021-06-14 17:18

View 3 revisions

About "findAllQuestionMetaDataForSelector", not sure this should exist either. :) More like

array_map($questions, function($q) { return $q->question_theme->config->metadata; });

Much could fall into place if we add a new class Metadata and a method "getMetadata" to ExtensionConfig class.

About this line:

            $baseQuestion['settings'] = json_decode($baseQuestion['settings']);

This is a sign of lack of abstraction between view, logic and data. There should be a method "getSettings" that does the conversion. Actually, settings should not be part of metadata at all, because metadata should be identical between all extension types (plugins etc). Again some domain confusion.

This lack of separation is throughout the entire question theme system, sadly.
ollehar

ollehar

2021-06-14 17:19

administrator   ~64901

Possibly a new class QuestionThemeExtensionConfig extending ExtensionConfig could have method getSettings.
gabrieljenik

gabrieljenik

2021-06-14 17:22

manager   ~64902

From Olle:
Maybe the question_theme column (17350) to lime_questions table could be done before?
Then we could have proper ORM code like: $question->question_theme->config->metadata
gabrieljenik

gabrieljenik

2021-07-05 17:39

manager   ~65205

Waiting for 17350 to be finished to reassess this

Issue History

Date Modified Username Field Change
2021-05-31 14:58 gabrieljenik New Issue
2021-05-31 15:01 DenisChenu Relationship added related to 17310
2021-05-31 15:03 DenisChenu Note Added: 64672
2021-05-31 15:08 gabrieljenik Additional Information Updated View Revisions
2021-05-31 15:09 gabrieljenik Note Added: 64674
2021-05-31 15:13 DenisChenu Note Added: 64675
2021-05-31 15:17 gabrieljenik Note Added: 64676
2021-06-01 10:04 ollehar Product Version => 5.x
2021-06-01 19:53 ollehar Priority none => normal
2021-06-03 16:22 gabrieljenik Assigned To => gabrieljenik
2021-06-03 16:22 gabrieljenik Status new => assigned
2021-06-07 17:42 gabrieljenik Note Added: 64770
2021-06-07 17:49 DenisChenu Note Added: 64771
2021-06-14 17:06 ollehar Note Added: 64898
2021-06-14 17:09 ollehar Note Added: 64899
2021-06-14 17:10 ollehar Note Edited: 64899 View Revisions
2021-06-14 17:10 ollehar Note Edited: 64899 View Revisions
2021-06-14 17:16 ollehar Note Added: 64900
2021-06-14 17:18 ollehar Note Edited: 64900 View Revisions
2021-06-14 17:18 ollehar Note Edited: 64900 View Revisions
2021-06-14 17:19 ollehar Note Added: 64901
2021-06-14 17:22 gabrieljenik Note Added: 64902
2021-07-05 17:39 gabrieljenik Status assigned => acknowledged
2021-07-05 17:39 gabrieljenik Note Added: 65205