Skip to content

Commit

Permalink
New feature #15246: Allow fixed function (only PHP) in expression man…
Browse files Browse the repository at this point in the history 
…ager (#1320)

New feature #15246: Allow fixed function (only PHP) in expression manager
New feature #13175: Ability to show calculated values based on all users
  • Loading branch information
@Shnoulle @thedirtypanda
Shnoulle authored and thedirtypanda committed Jan 28, 2020
1 parent 86d2e13 commit ee4f069
Show file tree
Hide file tree
Showing 10 changed files with 485 additions and 92 deletions.
25 changes: 25 additions & 0 deletions application/core/plugins/statFunctions/config.xml
View file
@@ -0,0 +1,25 @@
<?xml version="1.0" encoding="UTF-8"?>
<config>
<metadata>
<name>statFunctions</name>
<type>plugin</type>
<creationDate>2019-09-19</creationDate>
<last_update>2019-09-19</last_update>
<author>Denis Chenu</author>
<authorUrl>https://www.limesurvey.org</authorUrl>
<version>0.1.0</version>
<license>GNU General Public License version 2 or later</license>
<description><![CDATA[New function for expression manager to count some statictics data : <ul>
<li>statCountIf(QuestionCode.sgqa, value[, submitted = true])</li>
<li>statCount(QuestionCode.sgqa[, submitted = true])</li>
</ul>
]]></description>
</metadata>

<compatibility>
<version>4.0</version>
</compatibility>

<updaters disabled="disabled">
</updaters>
</config>
77 changes: 77 additions & 0 deletions application/core/plugins/statFunctions/countFunctions.php
View file
@@ -0,0 +1,77 @@
<?php
/**
* This file is part of statFunctions plugin
*/
namespace statFunctions;

use Yii;
use CHtml;
use LimeExpressionManager;
use Survey;
use SurveyDynamic;
use CDbCriteria;
use Permission;

class countFunctions
{
/**
* Return the count of reponse on current Expression Manager survey equal to a specific value
* @param string $qCode : code of question, currently must be existing sgqa. Sample Q01.sgqa.
* @param string $comparaison : comparre with value. Can use < or > … see https://www.yiiframework.com/doc/api/1.1/CDbCriteria#compare-detail
* @param boolean $submitted (or not) response
* @return integer|string
*/
public static function statCountIf($qCode, $comparaison, $submitted = true)
{
$surveyId = LimeExpressionManager::getLEMsurveyId();
if (!Survey::model()->findByPk($surveyId)->getIsActive()) {
return 0;
}
$questionCodeHelper = new \statFunctions\questionCodeHelper($surveyId);
$column = $questionCodeHelper->getColumnByQCode($qCode);
if (is_null($column)) {
if (Permission::model()->hasSurveyPermission($surveyId, 'surveycontent')) { // update ???
return sprintf(gT("Invalid question code %s"), CHtml::encode($qCode));
}
return "";
}
$sQuotedColumn=Yii::app()->db->quoteColumnName($column);
$oCriteria = new CDbCriteria;
$oCriteria->condition= "$sQuotedColumn IS NOT NULL";
if ($submitted) {
$oCriteria->addCondition("submitdate IS NOT NULL");
}
$oCriteria->compare($sQuotedColumn, $comparaison);
return intval(SurveyDynamic::model($surveyId)->count($oCriteria));
}

/**
* Return the count of reponse on current Expression Manager survey equal to a specific value
* @param string $qCode : code of question, currently must be existing sgqa. Sample Q01.sgqa.
* @param boolean $submitted (or not) response
* @return integer|string
*/
public static function statCount($qCode, $submitted = true)
{
$surveyId = LimeExpressionManager::getLEMsurveyId();
if (!Survey::model()->findByPk($surveyId)->getIsActive()) {
return 0;
}
$questionCodeHelper = new \statFunctions\questionCodeHelper($surveyId);
$column = $questionCodeHelper->getColumnByQCode($qCode);
if (is_null($column)) {
if (Permission::model()->hasSurveyPermission($surveyId, 'surveycontent')) { // update ???
return sprintf(gT("Invalid question code %s"), CHtml::encode($qCode));
}
return "";
}

$sQuotedColumn=Yii::app()->db->quoteColumnName($column);
$oCriteria = new CDbCriteria;
$oCriteria->condition= "$sQuotedColumn IS NOT NULL and $sQuotedColumn <> ''";
if ($submitted) {
$oCriteria->addCondition("submitdate IS NOT NULL");
}
return intval(SurveyDynamic::model($surveyId)->count($oCriteria));
}
}
46 changes: 46 additions & 0 deletions application/core/plugins/statFunctions/questionCodeHelper.php
View file
@@ -0,0 +1,46 @@
<?php
/**
* This file is part of statFunctions plugin
*/
namespace statFunctions;

use Yii;
use Survey;
use SurveyDynamic;
use CDbCriteria;

class questionCodeHelper
{
/** @var integer $surveyId **/
public $surveyId = 0;

/**
* @param integer $surveyId
*/
public function __construct($surveyId)
{
$this->surveyId = $surveyId;
/* Throw error if surveyid is invalid ? */
}

/**
* Check the survey
* @param string $qCode question SGQA
* @return string|null : the final column name, null if not found
*/
public function getColumnByQCode($qCode)
{
$availableColumns = SurveyDynamic::model($this->surveyId)->getAttributes();
/* Sample : Q01.sgqa Q01_SQ01.sgqa */
if (array_key_exists($qCode, $availableColumns)) {
return $qCode;
}

/* @todo : allow "Q0" and "Q0_SQ0" …
* But without using LimeExpressionManager::ProcessString or LimeExpressionManager::getLEMqcode2sgqa
* Because break logic file
* Wait for OK to merge to start it …
*/
return null;
}
}
56 changes: 56 additions & 0 deletions application/core/plugins/statFunctions/statFunctions.php
View file
@@ -0,0 +1,56 @@
<?php
/**
* @author Denis Chenu <denis@sondages.pro>
* @copyright 2019 Denis Chenu <http://www.sondages.pro>
* @license GPL version 3
* @version 0.0.1
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
class statFunctions extends PluginBase
{
protected static $description = 'Add some function in expression manager to get count from other responses';
protected static $name = 'statCountFunctions';

public function init()
{
$this->subscribe('ExpressionManagerStart', 'newValidFunctions');
}

public function newValidFunctions()
{
Yii::setPathOfAlias(get_class($this), dirname(__FILE__));
$newFunctions = array(
'statCountIf' => array(
'\statFunctions\countFunctions::statCountIf',
null, // No javascript function : set as static function
$this->gT("Count the response done with value equal to a specific value"), // Description for admin
'integer statCountIf(QuestionCode.sgqa, value[, submitted = true])', // Extra description
'https://www.limesurvey.org', // Help url
2, // Number of argument unsure it work here … , minimum 2, allow 3
3
),
'statCount' => array(
'\statFunctions\countFunctions::statCount',
null, // No javascript function : set as static function
$this->gT("Count previous response done not empty"), // Description for admin
'integer statCount(QuestionCode.sgqa[, submitted = true])', // Extra description
'https://www.limesurvey.org', // Help url
1, // Number of argument (time to make a good description of EM …) minimum 1, allow 2
2,
),
);
$this->getEvent()->append('functions', $newFunctions);
}
}
156 changes: 96 additions & 60 deletions application/helpers/expressions/em_core_helper.php
View file
Expand Up @@ -1240,70 +1240,106 @@ public function GetJavaScriptEquivalentOfExpression()
$tokens = $this->RDP_tokens;
$stringParts = array();
$numTokens = count($tokens);

/* Static function management */
$bracket = 0;
$staticStringToParse = "";
for ($i = 0; $i < $numTokens; ++$i) {
$token = $tokens[$i];
// When do these need to be quoted?
$token = $tokens[$i]; // When do these need to be quoted?
if(!empty($staticStringToParse)) { /* Currently inside a static function */

switch ($token[2]) {
case 'DQ_STRING':
$stringParts[] = '"'.addcslashes($token[0], '\"').'"'; // htmlspecialchars($token[0],ENT_QUOTES,'UTF-8',false) . "'";
break;
case 'SQ_STRING':
$stringParts[] = "'".addcslashes($token[0], "\'")."'"; // htmlspecialchars($token[0],ENT_QUOTES,'UTF-8',false) . "'";
break;
case 'SGQA':
case 'WORD':
if ($i + 1 < $numTokens && $tokens[$i + 1][2] == 'LP') {
// then word is a function name
$funcInfo = $this->RDP_ValidFunctions[$token[0]];
if ($funcInfo[1] == 'NA') {
return ''; // to indicate that this is trying to use a undefined function. Need more graceful solution
}
$stringParts[] = $funcInfo[1]; // the PHP function name
} elseif ($i + 1 < $numTokens && $tokens[$i + 1][2] == 'ASSIGN') {
$jsName = $this->GetVarAttribute($token[0], 'jsName', '');
$stringParts[] = "document.getElementById('".$jsName."').value";
if ($tokens[$i + 1][0] == '+=') {
// Javascript does concatenation unless both left and right side are numbers, so refactor the equation
$varName = $this->GetVarAttribute($token[0], 'varName', $token[0]);
$stringParts[] = " = LEMval('".$varName."') + ";
++$i;
}
} else {
$jsName = $this->GetVarAttribute($token[0], 'jsName', '');
$code = $this->GetVarAttribute($token[0], 'code', '');
if ($jsName != '') {
$varName = $this->GetVarAttribute($token[0], 'varName', $token[0]);
$stringParts[] = "LEMval('".$varName."') ";
switch ($token[2]) {
case 'LP':
$staticStringToParse .= $token[0];
$bracket ++;
break;
case 'RP':
$staticStringToParse .= $token[0];
$bracket --;
break;
case 'DQ_STRING':
// A string inside double quote : add double quote again
$staticStringToParse .= '"'.$token[0].'"';
break;
case 'SQ_STRING':
// A string inside single quote : add single quote again
$staticStringToParse .= "'".$token[0]."'";
break;
default:
// This set whole string inside function as a static var : must document clearly.
$staticStringToParse .= $token[0];
}
if($bracket == 0) { // Last close bracket : get the static final function and reset
//~ $staticString = LimeExpressionManager::ProcessStepString("{".$staticStringToParse."}",array(),3,true);
$staticString = $this->sProcessStringContainingExpressions("{".$staticStringToParse."}",0,3,1,-1,-1,true); // As static : no gseq,qseq etc …
$stringParts[] = $staticString;
$staticStringToParse = "";
}
} else {
switch ($token[2]) {
case 'DQ_STRING':
$stringParts[] = '"'.addcslashes($token[0], '\"').'"'; // htmlspecialchars($token[0],ENT_QUOTES,'UTF-8',false) . "'";
break;
case 'SQ_STRING':
$stringParts[] = "'".addcslashes($token[0], "\'")."'"; // htmlspecialchars($token[0],ENT_QUOTES,'UTF-8',false) . "'";
break;
case 'SGQA':
case 'WORD':
if ($i + 1 < $numTokens && $tokens[$i + 1][2] == 'LP') {
// then word is a function name
$funcInfo = $this->RDP_ValidFunctions[$token[0]];
if ($funcInfo[1] === null ) {
/* start a static function */
$staticStringToParse = $token[0]; // The function name
$bracket = 0; // Reset bracket (again)
} else {
$stringParts[] = $funcInfo[1]; // the PHP function name
}
} elseif ($i + 1 < $numTokens && $tokens[$i + 1][2] == 'ASSIGN') {
$jsName = $this->GetVarAttribute($token[0], 'jsName', '');
$stringParts[] = "document.getElementById('".$jsName."').value";
if ($tokens[$i + 1][0] == '+=') {
// Javascript does concatenation unless both left and right side are numbers, so refactor the equation
$varName = $this->GetVarAttribute($token[0], 'varName', $token[0]);
$stringParts[] = " = LEMval('".$varName."') + ";
++$i;
}
} else {
$stringParts[] = "'".addcslashes($code, "'")."'";
$jsName = $this->GetVarAttribute($token[0], 'jsName', '');
$code = $this->GetVarAttribute($token[0], 'code', '');
if ($jsName != '') {
$varName = $this->GetVarAttribute($token[0], 'varName', $token[0]);
$stringParts[] = "LEMval('".$varName."') ";
} else {
$stringParts[] = "'".addcslashes($code, "'")."'";
}
}
}
break;
case 'LP':
case 'RP':
$stringParts[] = $token[0];
break;
case 'NUMBER':
$stringParts[] = is_numeric($token[0]) ? $token[0] : ("'".$token[0]."'");
break;
case 'COMMA':
$stringParts[] = $token[0].' ';
break;
default:
// don't need to check type of $token[2] here since already handling SQ_STRING and DQ_STRING above
switch (strtolower($token[0])) {
case 'and': $stringParts[] = ' && '; break;
case 'or': $stringParts[] = ' || '; break;
case 'lt': $stringParts[] = ' < '; break;
case 'le': $stringParts[] = ' <= '; break;
case 'gt': $stringParts[] = ' > '; break;
case 'ge': $stringParts[] = ' >= '; break;
case 'eq': case '==': $stringParts[] = ' == '; break;
case 'ne': case '!=': $stringParts[] = ' != '; break;
default: $stringParts[] = ' '.$token[0].' '; break;
}
break;
break;
case 'LP':
case 'RP':
$stringParts[] = $token[0];
break;
case 'NUMBER':
$stringParts[] = is_numeric($token[0]) ? $token[0] : ("'".$token[0]."'");
break;
case 'COMMA':
$stringParts[] = $token[0].' ';
break;
default:
// don't need to check type of $token[2] here since already handling SQ_STRING and DQ_STRING above
switch (strtolower($token[0])) {
case 'and': $stringParts[] = ' && '; break;
case 'or': $stringParts[] = ' || '; break;
case 'lt': $stringParts[] = ' < '; break;
case 'le': $stringParts[] = ' <= '; break;
case 'gt': $stringParts[] = ' > '; break;
case 'ge': $stringParts[] = ' >= '; break;
case 'eq': case '==': $stringParts[] = ' == '; break;
case 'ne': case '!=': $stringParts[] = ' != '; break;
default: $stringParts[] = ' '.$token[0].' '; break;
}
break;
}
}
}
// for each variable that does not have a default value, add clause to throw error if any of them are NA
Expand Down

35 comments on commit ee4f069

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in dev, not master. Sigh. The fuck is going on?

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original pull request was done when 4.X are develop … not master …

@maziminke
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a great new feature. Do we have some documentation at the manual on how to exactly use that?

A demo survey would also be very helpful, best as an LSA file with some test data. Can you provide that/add it to the manual?

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maziminke : there are a plugin with that feature … and a demo survey as lsa …

@maziminke
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Shnoulle And where exactly can I find that?

We should never forget that adding great new features is useless for the users if we do not a) document them at the manual and b) provide some examples on how to use such features.

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://manual.limesurvey.org/ExpressionManagerStart#Example

And please : i the only one to update manual when i add a event … then … …

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaks on Postgres T_T

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"CDbCommand failed to execute the SQL statement: SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for type numeric: "" LINE 1: ...("823512X19X81" IS NOT NULL and "823512X19X81" <> '') AND (s... ^. The SQL statement executed was: SELECT COUNT(*) FROM "lime_survey_823512" "t" WHERE ("823512X19X81" IS NOT NULL and "823512X19X81" <> '') AND (submitdate IS NOT NULL); Count the number of complete responses which are not empty; integer statCount(QuestionCode.sgqa[, submitted = true])"

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't understand why this error happens. :d

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

statCount ? What is the 81 question type ? DECIMAL or DATE ?

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe must test DB type for comparaison : add <> '' only of text or varchar or char ?

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T_T My native psql is not same version as my docker postgres.

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😢

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

823512X19X81 | numeric(30,10) | | |

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, potential solution :

  1. check column type ?
  2. Cast ?

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should compare with 0 or "is null".

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not with empty string.

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No : because we need this (<>'')for "short text" and for "single choice" question type.
And in LimleSurvey : 0 mean answered

PS : my computer with my 8 or 10 LimeSurvey instance is broken since Friday .... i' am on my little laptop .... 😭

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check column type : DECIMAL or DATE NOT IS NULL

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, then need to adapt it to question type. 🎉

@olleharstedt
Copy link
Contributor

@olleharstedt olleharstedt commented on ee4f069 Feb 12, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CAST(column AS varchar) also works.

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will push tomorrow.

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CAST(column AS varchar) for long/short text ?

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't matter, doesn't work on MySQL anyway.

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't matter, doesn't work on MySQL anyway.

? What that doesn't work ? Cast ?

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CAST(column AS varchar) is not valid MySQL syntax.

@Shnoulle
Copy link
Collaborator Author

@Shnoulle Shnoulle commented on ee4f069 Feb 13, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without checking type, maybe


        $oCriteria = new CDbCriteria;
        $oCriteria->condition= "$sQuotedColumn IS NOT NULL and $sQuotedColumn <> :empty";
        $oCriteria->params = array(":empty"=>'');
        if ($submitted) {
            $oCriteria->addCondition("submitdate IS NOT NULL");
        }

?

@Shnoulle
Copy link
Collaborator Author

@Shnoulle Shnoulle commented on ee4f069 Feb 13, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or ....

    switch (Yii::app()->db->driverName){
        case 'sqlsrv':
        case 'dblib':
        case 'mssql':
            $lengthWord='LEN';
            break;
        default:
            $lengthWord='LENGTH';
    }
    
	$oCriteria = new CDbCriteria;
	$oCriteria->condition= "$sQuotedColumn IS NOT NULL and $lengthWord($sQuotedColumn) = 0";
	if ($submitted) {
		$oCriteria->addCondition("submitdate IS NOT NULL");
	}

?

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but this should be part of the driver classes, not a switch in a random place. Can I assign you the bug, since it's in your code?

@olleharstedt
Copy link
Contributor

@olleharstedt olleharstedt commented on ee4f069 Feb 13, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: In MySQL, it's CAST(foo AS char). Don't know about MSSQL.

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I assign you the bug, since it's in your code?

Yes, sure , but i didn't see it in mantis.

@Shnoulle
Copy link
Collaborator Author

@Shnoulle Shnoulle commented on ee4f069 Feb 13, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but this should be part of the driver classes, not a switch in a random place.

You mean create a function for CDBCriteria ->isempty($columnname) ?

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See

LimeSurvey/application/helpers/export_helper.php

Lines 2311 to 2316 in 03840d3

/**
* Find the string size according DB size for existing question
* Column name must be SGQA currently
* @param string sColumn column
* @return integer
**/

Else : maybe better to check column type ? Unsure on the best way ....

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See

LimeSurvey/application/helpers/export_helper.php

Lines 2369 to 2371 in 03840d3

if(Yii::app()->db->driverName == 'pgsql') {
$castedColumnString = "CAST($sColumn as text)";
}

Maybe the quickest way .....

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maziminke

Looks like a great new feature. Do we have some documentation at the manual on how to exactly use that?

A demo survey would also be very helpful, best as an LSA file with some test data. Can you provide that/add it to the manual?

Else : plugin documentation included
Capture d’écran du 2020-02-13 14-22-13

Please sign in to comment.