Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed issue #16669: getQuestionAttributes function don't get the plugins attribute #1843

Conversation

gabrieljenik
Copy link
Collaborator

Picking up plugin attributes and merging them with theme and std ones.

…gins attribute

- Added new function 'getAttributesFromPlugin' to QuestionAttributeHelper, which calls the old 'getOwnQuestionAttributesViaPlugin' function and filters by question type.
- Use the new function in Question::getAdvancedSettingsWithValues()
@Shnoulle Shnoulle self-requested a review April 10, 2021 08:08
@olleharstedt
Copy link
Collaborator

olleharstedt commented Apr 12, 2021

This has the same issue as before - the plugin event is run from two different places. Maybe it's the only way in LS4, since, sadly, the code lost a lot of quality compared to LS3. Still I wonder if it's possible to use ONLY the method "getQuestionThemeAttributeValues" to fetch question theme attributes.

Or, is it possible to have ONE function to get question theme attributes (together with their values, if any)? And fire the event there?

Since getQuestionThemeAttributeValues() is static, it can be moved to a service class, e.g. QuestionAttributeHelper.

getAdditionalAttrFromExtendedTheme - why is this needed? Additional to what?

@gabrieljenik
Copy link
Collaborator Author

Or, is it possible to have ONE function to get question theme attributes (together with their values, if any)? And fire the event there?

Why do you wan't fir it from there? I was actually thinking about removing it from there.
Let's review on a call?

@olleharstedt
Copy link
Collaborator

Sure, ping me tomorrow.

@gabrieljenik
Copy link
Collaborator Author

After call, this ticket was created: https://bugs.limesurvey.org/view.php?id=17242

@olleharstedt olleharstedt merged commit a028dce into LimeSurvey:master Apr 13, 2021
@olleharstedt
Copy link
Collaborator

Merged after deciding to continue with more clean-up PRs.

Ticket for unit-test: https://bugs.limesurvey.org/view.php?id=17241

Duplicate ticket for clean-up: https://bugs.limesurvey.org/view.php?id=17240

@Shnoulle
Copy link
Collaborator

@olleharstedt About issue with https://manual.limesurvey.org/NewQuestionAttributes

For example :

        'inputtype' => 'switch',
        'options'   => array(
          0 => gT('No'),
          1 => gT('Yes'),
        ),

Nothing is shown

        'inputtype'=>'singleselect',
        'options'=>array(
          'afteranswer'=>$this->_translate("The script is inserted just after answer part."),
          CClientScript::POS_HEAD=>$this->_translate("The script is inserted in the head section right before the title element (POS_HEAD)."),
          CClientScript::POS_BEGIN=>$this->_translate("The script is inserted at the beginning of the body section (POS_BEGIN)."),
          CClientScript::POS_END=>$this->_translate("The script is inserted at the end of the body section (POS_END)."),
          CClientScript::POS_LOAD=>$this->_translate("The script is inserted in the window.onload() function (POS_LOAD)."),
          CClientScript::POS_READY=>$this->_translate("The script is inserted in the jQuery's ready function (POS_READY)."),
        ),

Show a dropdown with 'array' (and break with debug)

Report as issue or API upgrade need updating manual ?

@Shnoulle
Copy link
Collaborator

Oh no … ok : before 4.5.0 : need

          'option'=> array(
            array('value'=>0, 'text'=> gT("No")),
            array('value'=>1, 'text'=> gT("Yes")),
          ),

for 4.5 : come back with compatibility of 3.X …

Still some issue with help and with switch

@olleharstedt
Copy link
Collaborator

Report everything, assign to Gabriel. :) 🥳

@olleharstedt
Copy link
Collaborator

Or tell me which function that needs unit-test to stay correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants