Патч по extra_answer_fields для Тима с юнит-тестами

[Oleg Sychev]

Originally reported on Google Code with ID 138

Реализовать интерфейс тестирования регулярного выражения автором: 
  * строка где показывается цветная строка;
  * тесты которые должны совпадать (отмечать цветными строками давшие  неполное совпадение)
  * тесты, которые не должны совпадать (выводить цветными строками все)

Сначала интерфейс, потом уже сохранение в БД...

Reported by oasychev on 2012-07-31 14:10:19

[Oleg Sychev]

Напоминаю, что это нам очень даже нужно к релизу!

Кто отвечает за эту часть? По взаимодействию с БД обращаться ко мне за консультациями,
по бэкап/рестор - к Валерию.

Reported by oasychev on 2013-06-21 15:20:47

[Oleg Sychev]

На всякий случай напоминаю, что необходимо сделать:
1) завести саму таблицу используя install.xml и upgrade.php и обновить версию в version.php
2) обеспечить работу с данными о вопросе, для чего в questiontype preg'а прописать
работу с этими данными в get_question_options() и save_question_options() (не забыть
вызвать родительские экземпляры, если функция не была ранее перегружена) - теперь данны
смогут попасть в вопрос, на форму и в экспорт/импорт;
3) сделать поле в классе question - массив, аналогичный массиву answers -  и обеспечить
его заполнение в initialise_question_instance
4) сделать скрытые поля для запоминания тестов в форме - get_per_answer_fields - и
сделать их заполнение, перегрузив ту функцию, которая заполняет ответы в форме; ваше
окошко с инструментами должно сохранять тесты именно в форму
5) сделать импорт/экспорт в Moodle XML формат - через функции в questiontype.php
6) сделать backup/restore
7) все протестировать:
7.1) создание нового вопроса с тестами
7.2) редактирование существующего вопроса с тестами - что тесты не исчезают при пересохранении,
даже если окошко не вызывалось
7.3) импорт/экспорт в Moodle XML формат - в question bank есть для этого ссылки - что
после импорта тесты на месте;
7.4) backup/restore - ссылки есть в блоке управления курсом, если в курсе есть хоть
один тест то бэкапятся все вопросы - проверяем, что тесты бэкапятся и восстанавливаются
нормально, тут два варианта восстановления тестируем - с восстановлением в тот же курс
и созданием нового.

Пишу подробно, чтобы ничего не забыли - соответственно спрашивайте и отмечайте сделанное/протестированное
тоже подробно...

Reported by oasychev on 2013-07-12 14:57:34

[Oleg Sychev]

 from Tim Hunt:
Sounds like what you are doing is similar to question_tests in STACK. If you
look there, you will see that we only load it when necessary. (There are some
extra methods on the question_type class.) Don't get the data in
get_question_options. That will be an unnecessary performance hit during quiz
attempts.

Т.е. стратегию работы с тестами немного меняем: тесты подгружаются дополнительно только
когда нужны, а не через get_question_options. Вместо этого получаем их прямо из БД/сохраняем
в БД при открытии/закрытии окна инструментов авторинга, а также при экспорте/импорте
и backup/restore. Это даже проще по коду... Можете посмотреть на код вопроса STACK
если есть желание...

Reported by oasychev on 2013-08-14 19:33:07

[Former user]

По ходу работы возникла проблема. Для вытаскивания из БД тестов я перегрузил в edit_preg_form
метод data_preprocessing_answers (как вы и советовали). Но после того, как я это сделал,
перестали создаваться вопросы в Банке вопросов с вот таким объяснением http://prntscr.com/1qn2f3

Reported by ZluMYO on 2013-09-11 14:30:12

[Oleg Sychev]

Надо смотреть на код, что вы там делали; вызывали ли родительский метод и т.д.
И как с БД работали....

Reported by oasychev on 2013-09-11 15:36:44

[Former user]

protected function data_preprocessing_answers($question, $withanswerfiles = false) {
        parent::data_preprocessing_answers($question, $withanswerfiles);

        global $DB;

        $testsdata = $DB->get_records('qtype_preg_regex_tests', array('questionid'
=> $questionid), 'testcase, regextests');
        foreach ($testsdata as $data) {
            $question->regextests[] = $data->regextests;
        }
    }

Reported by ZluMYO on 2013-09-11 17:21:29

[Oleg Sychev]

Если вы смотрели на код родительских функций, то могли заметить что они возвращают измененный
объект.
А у вас ни return $question; нет
ни 
$question = parent::data_preprocessing_answers($question, $withanswerfiles);

Будьте внимательнее!

Reported by oasychev on 2013-09-11 19:22:11

[Former user]

Реализовал работу с БД.

Reported by ZluMYO on 2013-09-18 08:59:59 - Status changed: Fixed

[Oleg Sychev]

Ну до fixed тут еще далековато.

Во-первых, сохранение реализовано примитивно - надо обеспечить максимальное использование
существующих записей. Во-вторых, там есть функции связанные с удалением вопроса - данные
надо корректно очистить.

А еще выше написано было про импорт/экспорт и бэкап/рестор.

Reported by oasychev on 2013-09-18 09:12:08 - Status changed: InProgress

[Oleg Sychev]

Иванов, чего вы там наделали с полями?

Во-первых, зачем-то убрали нормальную схему tableid/tablename которую мы еще с лета
обсуждали - если забыли, так надо спрашивать. Затем зачем-то quiestionid уже совсем
непонятно - у вопроса может быть несколько регексов-ответов, и к ним - разные тесты...

Давайте возвращать как было и заполнять нормально. tablename для вас будет question_answers
(или что-то типа того, посмотрите название таблицы), а tableid будет id answer'а...

Reported by oasychev on 2013-09-18 18:15:29

[Oleg Sychev]

И еще Иванов - уберите все табуляции (отдельным коммиттом) и настройте редактор чтобы
он их больше не генерировал!

Reported by oasychev on 2013-09-18 19:05:24

[Former user]

"tablename для вас будет question_answers (или что-то типа того, посмотрите название
таблицы), а tableid будет id answer'а... "
Объясните пожалуйста подробнее - я не понимаю этой схемы.
<FIELD NAME="id" TYPE="int"/>
<FIELD NAME="tablename"/>
<FIELD NAME="tableid" TYPE="int"/>
<FIELD NAME="regextests"/>

Решительно не понимаю, что будет в tablename.
tableid это id вопроса? Разве такое имя не сбивает с толку?
И, наконец, как можно в одном regextests хранить тесты для разных ответов, если мы
уже используем переводы строк в рамках тестов для одного ответа?

Reported by ZluMYO on 2013-09-20 14:35:58

[Oleg Sychev]

Во, первых в нашей таблице мы будем привязывать тесты не к вопросам, а к ответам (сами
ответы привязаны к вопросам), в одной строке таблицы regextests хранятся тесты к одному
ответу. Соответственно и прописывать ей надо answerid, а вовсе не questionid, чтобы
строка соответствовала ответу. Один вопрос может содержать несколько регулярных выражений-ответов,
с разными тестами и оценками. 

Схема tablename/tableid используется в тех случаях, когда таблица (и код) привязаны
не всегда к одному объекту. Вы еще помните, что блок надо сделать? Но он чисто для
составления регекса, там тесты не запоминаются. А потихоньку я уже начинаю работы над
вопросом ContructRegex, где надо составить регекс в качестве ответа, в котором те же
инструменты авторинга будут использоваться как подсказки. И в нем - если подумаете
- тесты окажутся привязаны уже к вопросу, а не ответу. Так что получается что для Preg
надо хранить answerid, а для ContructRegex - questionid. А id могут и совпасть в разных
таблицах. Поэтому ключ состоит из двух полей - имя таблицы, откуда взят id (tablename)
и id-шник записи в этой таблице, к которой привязываются тесты (tableid). Такая комбинация
будет всегда уникальна.  Для Preg tablename всегда будет question_answers а tableid
- это answerid. А для других клиентов инструментов авторинга он может быть другим...

Так понятно? Я кому-то из вас - Терехову наверное - объяснял уже эту схему, кто первоначально
должен был делать...


Кстати, поэтому сам код сохранения тестов лучше расположить в классе testing tool,
пусть даже в статической функции. А из формы передать ей имя таблицы - question_answers
 - для tablename - и массив где ключами будут id ответов (для tableid), а значениями
- строки с тестами. Но это если не поняли - сам отрефакторю, вы основу сделайте...

P.S. Вот как раз пытаясь прописать туда questionid вы и пытались засунуть несколько
ответов в одну строку, а используя answerid получаем отдельную строку таблицы с тестами
для каждого ответа...

Reported by oasychev on 2013-09-20 15:03:47

[Former user]

Теперь стало яснее...
Теперь вопрос по поводу импорта/экспорта в XML.
Я посмотрел код стандартного экспортёра и файл, который он формирует.
https://gist.github.com/zlumyo/6639357
Для того, чтобы тесты были внутри <answer> придётся очень переписать стандартные import_answer
и export_answer. Подскажите, как лучше сделать?

Reported by ZluMYO on 2013-09-20 15:36:39

[Oleg Sychev]

Вы по таблице/табуляциям сделайте побыстрее, чтобы ваш код слить можно было...

С импортом проще - там из массива по ключу в любой момент взять можно (я надеюсь вы
смотрели import_from_xml и export_to_xml в questiontypebase.php), вызвать сначала родительский
вариант, потом добавить данные самим в возвращаемый объект. Он будет сохраняться через
save_question_options так что если там код прописан, то все нормально.

Экспорт возвращает строку, тут немного хуже. Хотя можно по идее найти в ней все теги
</answer> и вставлять перед ними... Но это делать надо аккуратно - может же и в тексте
вопроса такое сочетание символов встретится...

Можно вместо этого сделать по другому. export_to_xml использует для вывода ответов
$format->write_answer . Вы можете создать свой наследник от класса формата - qformat_xml
вроде question/format/xml/format.php  - qtype_preg_qformat_xml и  перегрузить в нем
write_answer чтобы она сохраняла тесты и подменить объект перед вызовом родительской
функции на ваш класс - полей в нем вроде нет (проверьте внимательно), одни функции
- никто не заметит...

Только учтите что export_to_xml получает данные вопроса из get_question_options - и
тестов содержать не будет. Их надо запросить отдельно из БД, как и в случае с формой...

Reported by oasychev on 2013-09-20 16:14:51

[Former user]

Не получается "нормально" сохранить в базу данных. Код: https://gist.github.com/zlumyo/6705049
Создаю новый вопрос, забиваю его данными, сохраняю. При этом в таблице сохранилось
следующее.
mysql> select * from  mdl_qtype_preg_regex_tests;
+----+-----------+---------+------------+
| id | tablename | tableid | regextests |
+----+-----------+---------+------------+
| 31 | NULL      |       0 |            |
| 32 | NULL      |       0 |            |
| 33 | NULL      |       0 |            |
+----+-----------+---------+------------+
3 rows in set (0.00 sec)

Как быть? Не получается найти решение.

Reported by ZluMYO on 2013-09-25 19:50:38

[Oleg Sychev]

Явная ошибка - $test = array_shift($oldhints);
Аргумент в скобках не исправлен после копипаста.
Я бы также предпочел count() чем sizeof().

Дальше надо будет разбираться при отладке - например имена таблиц так задавать нельзя,
когда на них делается ссылка в SQL-операторах, они пишутся в фигурных скобках и без
префикса - не факт, что он будет именно mdl - почитайте правила.
Не знаю, умеет ли get_records_select вложенные подзапросы - есть get_records_sql.

Reported by oasychev on 2013-09-25 20:03:20

[Former user]

Как так не заметили! Спасибо. По справке sizeof это псевдоним count. Я просто привык
к C++ стилю...

Reported by ZluMYO on 2013-09-25 20:11:18

[Oleg Sychev]

В С++ у sizeof другой смысл; поэтому может как раз сбивать с толку.

Reported by oasychev on 2013-09-25 20:13:52

[Former user]

Вы говорили сегодня, что есть функция в Database API , которая проверяет совпадение
со списком, чтобы не делать вложенный подзапрос. Вы об этой http://prntscr.com/1tkpoa
?
К сожалению придётся сделать 2 запроса, т.к. мой исходный запрос она не покроет
$oldtests = $DB->get_records_select('qtype_preg_regex_tests',
            'tablename = \'question_answers\' AND tableid IN (SELECT question FROM
mdl_question_answers WHERE question = ' . $questionid . ')');

Reported by ZluMYO on 2013-09-26 14:53:34

[Oleg Sychev]

Я говорил о функции get_in_or_equal - или около того. Она не делает запрос, но она генерирует
наиболее эффективную форму IN для любой СУБД. Лучше, чем вы это пропишете сами.

А для сложных запросов есть get_records_sql. 

Reported by oasychev on 2013-09-26 15:52:29

[Former user]

Я переделал с get_records_sql https://gist.github.com/zlumyo/6717631
Но результат тот же
mysql> select * from  mdl_qtype_preg_regex_tests;
+----+-----------+---------+------------+
| id | tablename | tableid | regextests |
+----+-----------+---------+------------+
| 1  | NULL      |       0 |            |
| 2  | NULL      |       0 |            |
+----+-----------+---------+------------+

Как видите, я хотел задействовать отладочную печать, но на странице ничего такого не
появляется. Похоже мудл блокирует такие вещи. Как быть?

Reported by ZluMYO on 2013-09-26 17:34:18

[Oleg Sychev]

Из цитатника по стилям:
All SQL queries and fragments should be enclosed in double quotes.

Ну в принципе print_r может не печатать ничего если пусто, попробуйте var_dump. И заодно
$questionid и $regextests распечатайте, проверьте нормально ли они передаются.

Reported by oasychev on 2013-09-26 21:18:07

[Former user]

var_dump также ничего не печатает (пробовал с разными типами переменных), т.е. на HTML
я не нашёл вкраплений отладочной печати... 

Reported by ZluMYO on 2013-09-27 03:51:40

[Oleg Sychev]

Обычно печатает. Вы функцию то вызвать не забыли из save_question_options?

Reported by oasychev on 2013-09-27 14:34:20

[Oleg Sychev]

Если не печатает - возможно вопрос на самом деле не пытается сохраняться - чтобы это
произошло, там должно быть заполнено имя вопроса, хотя бы один регекс без ошибок и
со 100% оценкой.

У меня ругается на NULL в качестве tableid. Вы откуда берете третий параметр для своей
функции? Если из объекта $question в save_question_options то там нет id - только сами
ответы, это данные поля answer с формы. id - если нужны - надо получить из базы (Заодно
и вложенные запросы не понадобятся). Только учтите, что сами ответы сохранятся только
после вызова parent::save_question_options, так что вам надо вызывать свою функцию
после этого. 

Reported by oasychev on 2013-09-27 15:03:55

[Oleg Sychev]

И уберите из файлов все табуляции наконец! Отдельным коммитом...

Reported by oasychev on 2013-09-27 15:04:21

[Oleg Sychev]

Если не печатает и выскакивает форма редактирования обратно - значит на ней что-то не
так для сохранения вопроса. Если же он возвращается в банк вопросов, то печатает -
если конечно функция вообще вызывается...

Reported by oasychev on 2013-09-27 15:06:41

[Oleg Sychev]

Иванов - если не получится, будет легче если вы вытолкните недоделанный код, чтобы видеть
не только функцию, но и как она вызывается и т.д. Только табуляции все вычистите -
они из-за вас появились!

Но скорее всего проблема в том, что в ваших $answers нет id - как вы хотели. А у вас
поле not null.

Reported by oasychev on 2013-09-27 21:18:59

[Oleg Sychev]

Вытолкнул первый раунд приведения в чувство save_question_options. Сейчас она создает
строки для каждого ответа чтобы работало (о том, что будет с пропушенными тестами при
обратной загрузке из БД в форму тоже, похоже, никто не подумал - они съезжали), позже
выправлю.

Закомиченная Ивановым версия в сложных случаях обладала массой проблем, например если
исчезал ответ к которому были тесты, тесты могли в базе остаться, т.к. перечень answerid
для отлова тестов получался из базы уже после удаления лишних ответов. А возможное
наличие пустого ответа посредине списка (который не сохраняется в БД - см код от shortanswer)
тоже никто не подумал учесть.

После дискуссии с Тимом пришел к выводу что будем использовать extra_answer_fields,
поэтому перевел таблицу на answerid все-таки. Пока код будет у нас, впоследствии войдет
в ядро и от нас удалится...

extra_answer_fields даст нормальный импорт и экспорт, сохранение и считывание я сделаю;
но мне понадобится помощь в двух местах
1) бэкап-рестор - или через extra_answer_fields, или просто так... - это можете делать
прямо сейчас
2) юнит-тестирование этого безобразия - для варианта, когда не к каждому ответу есть
тесты, тестовых случаев очень много...

Reported by oasychev on 2013-10-04 23:36:06

[Oleg Sychev]

Сохранение, считывание, импорт/экспорт в первом приближении я сделал. Подробнее напишу
юнит-тесты.

Ждем бэкап/рестор от Валерия.

Господа ИВТ-460, поскольку мы с Валерием взяли на себя срочное решение вопросов с БД
(хотя нам есть чем заняться), вам лучше уже не задерживая исправить оставшиеся на вас
проблемы. В понедельник вечером я должен протестировать предрелизную версию!!!

Терехов - выверяйте строки и посмотрите еще раз на вид дерева с разными узлами;
Иванов - зеленеющую рамку уберите, и займитесь исправлением проблем с < и т.д. сущностями,
а также универсализацией отображения серых узлов - в дереве по идее это сделано нормально;
Пахомов - ждем настройки стилей от Moodle, исправлений переноса на сервер если будут
замечания от Стрельцова, выделения серым добавленного exact.

Reported by oasychev on 2013-10-05 22:07:43

[Former user]

Валера вам не рассказывал о моём тупике с экранированием символов? Я поделился своими
наблюдениями со всеми, но никто ничего не сказал. Вообщем суть такая:
/* тестик небольшой */
$graph = new qtype_preg_explaining_graph_tool('<');
$result = $graph->create_graph();
$dot = $result->create_dot();
echo $dot;

На инструменте рисует <.
phpunit даёт ответ:
Moodle 2.5.1+ (Build: 20130823), mysqli
PHPUnit 3.7.24 by Sebastian Bergmann.

Configuration read from D:\MyWebSites\Moodle\phpunit.xml

digraph { compound=true; rankdir = LR;"nd2" [label="&
#38;lt;"];"nd3" [shape=box, style=filled, color=purple, label="begin"];"nd4" [sh
ape=box, style=filled, color=purple, label="end"];"nd3" -> "nd2" [label="", arro
whead=normal];"nd2" -> "nd4" [label="", arrowhead=normal];}

Reported by ZluMYO on 2013-10-06 15:29:29

[Former user]

У меня аналогичная ситуация. Заменяю [<] на [
&
l
t
;
]
 и всё равно в узле отображается [<].

Reported by grvlter on 2013-10-06 16:00:56

[Former user]

Похоже дот делает 2 прохода замен мнемоник!

Reported by ZluMYO on 2013-10-06 16:06:56

[Valeriy Streltsov]

Бэкап\рестор вроде бы сделал, прошу всех подключиться к тестированию.

Reported by vostreltsov on 2013-10-06 23:42:08

[Valeriy Streltsov]

Reported by vostreltsov on 2013-10-06 23:54:02 - Status changed: Fixed

[Former user]

Проверил. Создал курс с 3-мя нашими вопросами. Заполнили их тестами для регулярных выражений.
Сделал бэкап. Очистил курс и сделал рестор. Всё стало как нужно.

Reported by ZluMYO on 2013-10-07 19:37:24

[Oleg Sychev]

Пока после фикса багов, выявлено странное поведение при ресторе в тот же курс с мержингом
- отредактированные вопросы могут дублироваться. По результатам разговора с Тимом -
возможны ошибки в самом Moodle, откладываем как минимум на 2.6

Reported by oasychev on 2013-10-07 23:19:19

[Valeriy Streltsov]

Обнаружил, что shortanswer тоже дублируется, если вопрос изменить перед рестором.

Reported by vostreltsov on 2013-10-08 11:31:26

[Oleg Sychev]

Бэкап-рестор в первом приближении работает.
Перевожу задачу на пропихивание кода в ядро Moodle по extra_answer_fields

Reported by oasychev on 2013-10-08 19:11:59 - Status changed: InProgress

[Oleg Sychev]

Тима наконец-то прошел, жду внимания интеграторов.

Reported by oasychev on 2014-03-28 22:45:40 - Status changed: Fixed

[Oleg Sychev]

В Moodle! Осталось только убрать лишний код из самого Preg...

Reported by oasychev on 2014-07-10 18:14:22 - Status changed: Done