View Issue Details

IDProjectCategoryView StatusLast Update
06519Development Otherpublic2012-11-26 22:25
ReporterTMSWhite Assigned Touser16774 
PrioritynormalSeverityminor 
Status assignedResolutionopen 
Product Version2.1 
Target VersionFixed in Version 
Summary06519: problems with Question_Objects branch
Description

this is a placeholder 'bug' to discuss the Question_Objects branch

TagsNo tags attached.

Activities

TMSWhite

TMSWhite

2012-08-26 05:55

reporter   ~20582

Aaron-
Some issues:
(1) Array_Filter_Tests.lss is not working - if you try it in 1.92, you'll see that it uses the first question to implement array_filter and array_filter_exclude for each of the other question types. None of that is working in 2.1
(2) Survey import isn't working properly. This might be partly the source of the trouble with (1). I'm getting errors like 'Could not open temporary directory'

user16774

2012-08-26 06:00

  ~20583

With regards to 2,
Is this a clean install or an upgrade?
Which survey are you trying to import - if it's not in the docs folder, please attach it to the issue.
*What file/line is the error occurring on?

TMSWhite

TMSWhite

2012-08-26 06:15

reporter   ~20584

(3) It's worth moving the $this->sgqaNaming code to Question_Objects. It is a fairly static parameter. So, LimeExpressionManager can use $q->varname (or something like that) to get the sgqa or qcode name, as appropriate.

TMSWhite

TMSWhite

2012-08-26 06:27

reporter   ~20585

(2) was a clean install (I cloned your repo and did a fresh install). The survey is ls2_array_filter_tests.lss in the docs/demosurveys folder.
I'm not seeing an error on any line - rather the survey isn't working as designed.

TMSWhite

TMSWhite

2012-08-26 14:46

reporter   ~20586

Aaron, some more comments:
(4) Please add comments to QuestionModule (and derived classes too, when possible, but should start with QuestionModule)
(5) QuestionModule->generateQuestionModule() and QuestionModule->generateSQInfo() seem to duplicate a lot of the other functionality. Is that intentional? I'm guessing that you started with that and you are gradually refactoring
(6) Performance - seems quite slow.
(a) Are you sure that the database access to question attributes is optimized (getQuestionAttributes())?
(b) ->getChildren() - this does a separate database access per question. I recommend against that approach since the survey will get linearly slower with the number of questions on the page.

TMSWhite

TMSWhite

2012-08-26 15:19

reporter   ~20587

(6)(c) ->getAnswerHTML() - should not query database for answers or 'other' status. Furthermore, if you are trying to randomize questions or answers, rather using SQL to do the randomization, first load all of the answers (at question creation time) and use PHP randomization functions to randomize them.
(6)(d) ->getDataEntryView() - don't do a separate query
(6)(e) ->setAssessment() - don't do a separate query - assessment values should be populated at question creation time.
(6)(f) -> getSPSSAnswers() - don't do a separate database query
(6)(g) RankingQuestion->getAnswers() - avoid separate database query

TMSWhite

TMSWhite

2012-08-26 15:28

reporter   ~20588

(7) ->getAnswerHTML(). This is a good start, but all of these should be refactored into views or .pstpl templates so that people wanting to extend question functionality can do so via the addition of new views without needing to modify the core code.

user16774

2012-08-27 21:07

  ~20590

I'll be working on these things. Many of the database queries were copied out of the original code and I'm still working on optimization for those. Other than the array-filters thing, did you find any broken features?

user16774

2012-08-29 09:00

  ~20593

Ok. I think I resolved the import issue. I fixed at least one major bug with the filtering, but it appears to be very much broken still. I'll be working on it.

user16774

2012-08-30 02:13

  ~20594

I fixed another couple bugs with array filtering. As far as I can tell, it now works as expected compared to 2.0. I assume you're at your new job now, but when it's convenient if you have time to look for any more fatal (i.e. non-performance-related) bugs that would be great.

TMSWhite

TMSWhite

2012-08-30 02:49

reporter   ~20595

Aaron-

Although page 1 of ls2_array_filter_tests.lss works OK, I can't get to page 2. I just see a blank page. Also, I can't Show Logic File for page 2, or for the whole survey. In both cases, I get a blank page.

I did a fresh database install and re-imported the survey, so that can't be the problem.

However, you're comparing to 2.0, not 1.92; and 2.0 may not work either. I just don't know. So, when you do your feature regression tests, please compare against 1.92, since I'm sure that works properly.

user16774

2012-08-30 04:02

  ~20596

Ok. I'm officially an idiot some days. While fixing the first page, I actually managed to introduce a new bug in the second page. That should be resolved now.

TMSWhite

TMSWhite

2012-08-30 06:31

reporter   ~20597

Aaron-

That fixed that issue. However, you (I'd say we, but I don't have much time), really need to regression test all of the survey in docs/demosurveys/*, comparing them to the 1.92 functionality.

I only loaded one additional survey to start testing (ls2_cascading_array_filter.lss), and already see two severe problems:
(1) Show Logic File shows several questions with ".NAOK" instead of "qcode.NAOK". So, the ConvertConditions2Relevance() functionality is broken. The content of the conditions editor seems OK, but the conversion to relevance is failing.
(2) the Show Logic File is not showing the proper sub-questions and answers for each question. There is duplication of some questions, and missing sub-questions/answers for others.

So, the fact that you're doing this refactoring is wonderful; but with any such refactoring, there is a need for rigorous testing. The surveys in docs/demosurveys/* are specifically designed to test all of the LimeSurvey functionality, so I recommend getting familiar with them in 1.92 and confirming that you can get the same behavior in your branch.

Good luck!

c_schmitz

c_schmitz

2012-11-26 22:25

administrator   ~22565

Fix me! ;)

Issue History

Date Modified Username Field Change
2012-08-26 05:43 TMSWhite New Issue
2012-08-26 05:43 TMSWhite Status new => assigned
2012-08-26 05:43 TMSWhite Assigned To => user16774
2012-08-26 05:55 TMSWhite Note Added: 20582
2012-08-26 06:00 user16774 Note Added: 20583
2012-08-26 06:15 TMSWhite Note Added: 20584
2012-08-26 06:27 TMSWhite Note Added: 20585
2012-08-26 14:46 TMSWhite Note Added: 20586
2012-08-26 15:19 TMSWhite Note Added: 20587
2012-08-26 15:28 TMSWhite Note Added: 20588
2012-08-27 21:07 user16774 Note Added: 20590
2012-08-29 09:00 user16774 Note Added: 20593
2012-08-30 02:13 user16774 Note Added: 20594
2012-08-30 02:49 TMSWhite Note Added: 20595
2012-08-30 04:02 user16774 Note Added: 20596
2012-08-30 06:31 TMSWhite Note Added: 20597
2012-11-26 22:25 c_schmitz Note Added: 22565