Skip to content

Commit

Permalink
Fixed issue : possible page broke when import TSV survey
Browse files Browse the repository at this point in the history
Dev : key are not validated
Dev : Yii::log replace tracevar
  • Loading branch information
Shnoulle committed Jun 16, 2017
1 parent 3d373df commit 2838ab4
Showing 1 changed file with 19 additions and 5 deletions.
24 changes: 19 additions & 5 deletions application/models/Answer.php
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?php if ( ! defined('BASEPATH')) exit('No direct script access allowed');
/*
* LimeSurvey
* Copyright (C) 2007-2011 The LimeSurvey Project Team / Carsten Schmitz
* Copyright (C) 2007-2017 The LimeSurvey Project Team / Carsten Schmitz
* All rights reserved.
* License: GNU/GPL License v2 or later, see LICENSE.php
* LimeSurvey is free software. This version may have been modified pursuant
Expand Down Expand Up @@ -77,10 +77,22 @@ public function rules()
return array(
array('qid','numerical', 'integerOnly'=>true),
array('code','length', 'min' => 1, 'max'=>5),
array('language','length', 'min' => 2, 'max'=>20),// in array languages ?
// Unicity of key
array(
'code', 'unique', 'caseSensitive'=>false, 'criteria'=>array(
'condition' => 'language=:language AND qid=:qid AND scale_id=:scale_id',
'params' => array(
':language' => $this->language,
':qid' => $this->qid,
':scale_id' => $this->scale_id
)
),
'message' => gT('Answer codes must be unique by question.')
),
array('answer','LSYii_Validators'),
array('sortorder','numerical', 'integerOnly'=>true,'allowEmpty'=>true),
array('assessment_value','numerical', 'integerOnly'=>true,'allowEmpty'=>true),
array('language','length', 'min' => 2, 'max'=>20),// in array languages ?
array('scale_id','numerical', 'integerOnly'=>true,'allowEmpty'=>true),
);
}
Expand Down Expand Up @@ -146,11 +158,13 @@ public function updateRecord($data, $condition=FALSE)
function insertRecords($data)
{
$oRecord = new self;
foreach ($data as $k => $v)
foreach ($data as $k => $v) {
$oRecord->$k = $v;
if($oRecord->validate())
}
if($oRecord->validate()) {
return $oRecord->save();
tracevar($oRecord->getErrors());
}
Yii::log(\CVarDumper::dumpAsString($oRecord->getErrors()),'warning','application.models.Answer.insertRecords');

This comment has been minimized.

Copy link
@LouisGac

LouisGac Jun 19, 2017

Contributor

does it depend on debug mode?

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Jun 19, 2017

Author Collaborator

No. But see https://github.com/LimeSurvey/LimeSurvey/blob/master/application/models/Question.php#L326 using insert recordds : now way to get error in a clean way.

log as warning seems the good way. tracevar its juste used for develop.

This comment has been minimized.

Copy link
@LouisGac

LouisGac Jun 19, 2017

Contributor

I think we should extend the log class to makes it dependent of debug mode state

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Jun 19, 2017

Author Collaborator

Why ? To set debug, user must access to config.php .

https://manual.limesurvey.org/Optional_settings#Logging_settings

This comment has been minimized.

Copy link
@LouisGac

LouisGac Jun 19, 2017

Contributor

because log are always a huge performance issue + security issue.
application should log only if admin wants it.

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Jun 19, 2017

Author Collaborator

Then WHY extend it ??? Yii already offer great system only accessible by FTP access. No need to extend it !

This comment has been minimized.

Copy link
@LouisGac

LouisGac Jun 19, 2017

Contributor

I'm just saying it should be possible to turn off all logs in the application, in a way or another (debug mode, switch in admin interface, etc). To integrate this on/off behaviour everywhere, extending the "log" class would be by far the easiest and more elegant way to do.

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Jun 19, 2017

Author Collaborator

? Currently : Yii can log in web/sql/file : what do you want more ? GUI settings for log ?

This comment has been minimized.

Copy link
@LouisGac

LouisGac Jun 19, 2017

Contributor

on/off

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Jun 19, 2017

Author Collaborator

Think it can be done via plugin when init . And think core don't need it .

This comment has been minimized.

Copy link
@LouisGac

LouisGac Jun 19, 2017

Contributor

well you're making the log core here. So we need a way to turn it off.

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Jun 19, 2017

Author Collaborator

But it's OFF in 99% of installation. Really don't understand your point here.

This comment has been minimized.

Copy link
@LouisGac

LouisGac Jun 19, 2017

Contributor

My point is that if we start to add Yii::log everywhere in the core code, we should start thinking about how it will be used by admin, how it could be turned on and off, what should be the different categories, etc
But not a big deal for now. Just curious, and just giving my first thought about it for now (should be part of a complete debug system, configurable by the admin, a little bit like in Joomla)

This comment has been minimized.

Copy link
@maziminke

maziminke Jun 19, 2017

Collaborator

If we add some logging at the background by such commits, we should indeed offer a way for superadmins to turn this on (should be off by default).
My suggestion is to add a new setting to Global Settings -> Security which allows superadmins to enable such logging.

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Jun 19, 2017

Author Collaborator

Open a feature request or a mantis and remind : super admin can want to use syslog or anything else.

}

/**
Expand Down

0 comments on commit 2838ab4

Please sign in to comment.