Fix MDL-25617 urgently

[GoogleCodeExporter]

Начиная с Moodle 2.0 система backup/restore была резко 
переработана, но сломана прослойка между 
ней и нашим вопросом - extra_question_fields().

Был написан патч для 2.0, но замечания 
исправлены не были.

Необходимо:
1) загрузить Moodle через Git и отдельно 
настроить сайт девелоперской версии (ветка 
master). Качаем msysgit и TortoiseGit, читаем 
http://docs.moodle.org/dev/Git_for_developers и 
http://docs.moodle.org/20/en/Git_for_Administrators
2) разобраться что надо изменить, чтобы 
перенести этот код в ветку master - что-то 
устарело, поменяло положение и т.д.
3) исправить замечания Тима (см. 
http://tracker.moodle.org/browse/MDL-25617)
4) создать ветку для этой проблемы MDL-25617, в 
ней изменить код и оттестировать на своем 
сайте (тестируем нормальный backup/restore для 
shortanswer (поля usecase, answers) и preg, а также multichoice - 
проверяем что ничего не сломалось в 
обычных) Текущий код из shortanswer, который не 
использует extra_question_fields, должен быть удален 
перед тестированием (!) чтобы не замещать 
универсальный. Тестируем очень тщательно.
5) сделать коммит в эту ветку Git, 
коммит-сообщение по английски, за юзером 
следить. 
6) на основе коммита Git-ом сгенерировать 
патч, либо на GitHub клонировать репозиторий и 
вытолкнуть туда ветку
7) по возможности показать мне, 
прокомментировав сделанные изменения
8) поместить патч или ссылку на репозиторий 
на GitHub в http://tracker.moodle.org/browse/MDL-25617

Счет идет на дни. Не успеем - потеряем 
полгода.... :(

Original issue reported on code.google.com by oasyc...@gmail.com on 15 Nov 2011 at 11:55

[GoogleCodeExporter]

можете стучаться в чат на gmail'e если будут 
срочные проблемы, блокирующие работу. В 
первую очередь технического рода, по ним я 
быстро смогу ответить - если надолго не 
получается, не стесняйтесь. По внутреннему 
устройству backup/restore и патча мне самому 
придется разбираться, тут сначала 
попытайтесь разобраться сами и пишите если 
уж совсем неясно. 

По устройству backup/restore читайте 
http://docs.moodle.org/dev/Backup_2.0 и страницы, на которые 
идут ссылки справа в блочке - особенно 
раздел "For developers". Может быть немного 
устаревшее, но в целом нормально. Если 
плохо понятно, посмотрите как сейчас 
устроено это в shortanswer впрямую, хороший 
пример как должно это работать. shortanswer 
работает только с этой таблицей...

Original comment by oasyc...@gmail.com on 15 Nov 2011 at 1:32

[GoogleCodeExporter]

Я немного не понял, вы пытаетесь это 
сделать; считаете нерешаемой к 
понедельнику или боитесь взяться? Потому 
что часть изменений, которые вы вносите в 
матчер менее срочны (в течении недели) чем 
эта задача(в пн-вт. надо отослать...)...

Original comment by oasyc...@gmail.com on 20 Nov 2011 at 11:06

[GoogleCodeExporter]

Я много переделал в матчере - захват 
последней подмаски оказался сложнее, чем я 
думал. Обнаружились некоторые баги (были 
даже две ситуации в другой части кода, 
которые вызывали исключения). С этой точки 
зрения, думаю, хорошо, что я поправил 
функциональность матчера - кто его знает, 
нашёл бы ошибки потом я, или их нашли бы 
юзеры:)

А сейчас буду пытаться разбираться с этим 
issue...

Original comment by vostreltsov@gmail.com on 20 Nov 2011 at 1:58

[GoogleCodeExporter]

Меня просто удивил порядок работ - вы 
взялись сначала не за самую срочную...

Original comment by oasyc...@gmail.com on 20 Nov 2011 at 2:48

[GoogleCodeExporter]

Original comment by vostreltsov@gmail.com on 25 Nov 2011 at 10:40

Attachments:

• 0001-MDL-25616-fix-backup-restore-questions-using-extra_q.patch

[GoogleCodeExporter]

мне вот в патче на первых двух строчках require 
не нравятся
один через ..
другой $CFG->dirroot
лучше оба через $CFG->dirroot

Более подробно посмотрю уже днем...

Original comment by oasyc...@gmail.com on 25 Nov 2011 at 11:05

[GoogleCodeExporter]

Смотря юнит-тесты:
1) во всех тестах формат идет после поля к 
которому он относится; нужен тест где 
формат употребляется раньше своего поля;
2) вы как-то не подумали когда копировали 
комментарий к функции. Было странно 
увидеть добавленную вами строку
@copyright  2011 The Open University
Тут уж нашему универу стоило бы стоять, но 
это может создать проблемы. Уберите ее от 
греха подальше. Если очень хотите - можете 
написать @author Valeriy Streltsov - но гордится там 
особо нечем...

Original comment by oasyc...@gmail.com on 26 Nov 2011 at 5:08

[GoogleCodeExporter]

 @return null if ok, else - notice message.
Не очень грамотно. Я бы написал notice message 
otherwise.

Original comment by oasyc...@gmail.com on 26 Nov 2011 at 5:11

[GoogleCodeExporter]

Поправил.

Original comment by vostreltsov@gmail.com on 26 Nov 2011 at 8:46

Attachments:

• fix.zip

[GoogleCodeExporter]

А где коммит с вариантом transform с одним 
циклом?

Original comment by oasyc...@gmail.com on 26 Nov 2011 at 9:24

[GoogleCodeExporter]

Если выполнили задачу - переводите 
состояние в Fixed. Я после проверки перевожу в 
Done.

Original comment by oasyc...@gmail.com on 27 Nov 2011 at 9:26

[GoogleCodeExporter]

Original comment by vostreltsov@gmail.com on 28 Nov 2011 at 2:24

• Changed state: Fixed

[GoogleCodeExporter]

Между прочим, Тим подал нам в замечании 4 
интересную идею, которую можно творчески 
развить. Вы сказали что выделить классы 
несложно.  Так почему бы нам не попробовать 
разместить такие же классы для начала в 
нашем вопросе - preg, ведь классы 
бэкапа/рестора отдельные и не мешают нам 
наследоваться от SA по другим направлениям. 
Это вариант "обеспечить работу бэкап к 
этому релизу минимальными усилиями".

Если это сработает, мы можем занять 
следующую позицию. Замечания 1 и 4 мы Тиму 
исправим, по 2 я еще уточню - он может сам 
отказаться. И когда в начале декабря выйдет 
2.2 перебазируем наши патчи на начало 
мастера (будущая 2.3) - учитывая что все почти 
стабилизировано это почти наверняка будет 
без конфликтов. Дальше по замечанию 3 мы 
говорим - ежели хочешь убедиться, вот есть 6 
месяцев тестирования до выхода 2.3, вполне 
достаточно для уверенности. Не нравится - в 
нашем вопросе это уже есть отдельно, тогда 
занимайся этой проблемой сам, нас она 
больше не волнует... Если он займется сам - 
перебазируем к 2.2 в абстрактный вопрос.

Как вам такой вариант? Мне кажется 
оптимально по трудозатратам в текущий 
момент.
Но тогда мне нужно быстро узнать, пройдет 
ли фокус с переносом, аналогичным 4, в наши 
классы бэкапа...  Должен пройти - но надо 
быть уверенным прежде чем разругиваться с 
Тимом ;)

Original comment by oasyc...@gmail.com on 29 Nov 2011 at 8:07

• Changed state: InProgress

[GoogleCodeExporter]

Спасибо за коммиты бэкапа и слияния.

Теперь надо подготовить Тиму ветки для 
http://tracker.moodle.org/browse/MDL-30562 - там других 
изменений не нужно в классах ядра кроме 
того чтобы сделать эти функции public'ами?

Original comment by oasyc...@gmail.com on 2 Dec 2011 at 8:15

[GoogleCodeExporter]

Нет, только две функции public.

Original comment by vostreltsov@gmail.com on 3 Dec 2011 at 3:06

[GoogleCodeExporter]

Видели сообщение Тима?

Надо подготовить ветки на Гитхабе 20, 21 и 
мастер с этими исправлениями...  Должно быть 
недолго...

Original comment by oasyc...@gmail.com on 3 Dec 2011 at 2:25

[GoogleCodeExporter]

https://github.com/vostreltsov/moodle/branches
Создал 3 ветки. Для 2.0 ветка, правда, 
оказалась лишней - там не указано protected, а по 
умолчанию функции и так public. Я ничего не 
стал туда коммитить.

Original comment by vostreltsov@gmail.com on 3 Dec 2011 at 8:20

[GoogleCodeExporter]

Выложил данные Тиму, ждем-с.
В своем вопросе поправим когда будут 
интегрированы в Мудл...

Original comment by oasyc...@gmail.com on 4 Dec 2011 at 12:42

[GoogleCodeExporter]

Видели последнее письмо Тима?  Сможете в 
ближайшее время сделать?

Original comment by oasyc...@gmail.com on 14 Dec 2011 at 11:13

[GoogleCodeExporter]

Т.е. перебазируем и выправляем замечания...

Лучше не тянуть, т.к. европа уходит на 
новогодние праздники раньше нашего -  у них 
Рождество 25.12.  Неплохо было бы пропустить 
это до Рождества - хотя бы через Тима.

Original comment by oasyc...@gmail.com on 14 Dec 2011 at 11:23

[GoogleCodeExporter]

Покажите мне результат прежде чем 
выкладывать Тиму...

Проблема в том что в выходные не факт что 
будет интернет - сейчас только на работе...

Original comment by oasyc...@gmail.com on 14 Dec 2011 at 2:16

[GoogleCodeExporter]

MDL-30562 интегрирована.
Пора перебазировать, делать последние 
исправления и переваливать эту проблему на 
Тима...

Original comment by oasyc...@gmail.com on 23 Dec 2011 at 10:48

[GoogleCodeExporter]

В общем от Тима толку пока мало, в 2.3 этот 
код не попадает. А у нас подрастает второй 
вопрос, которым Мамонтов занимается.

Поэтому надо таки брать весь этот код под 
свою опеку. Сделать абстрактный вопрос 
poasquestion, где разместить весь код от 
extra_question_fields - и наследовать все от него. К 
релизу который этим летом

Original comment by oasyc...@gmail.com on 6 Jun 2012 at 4:28

• Added labels: Milestone-Release2.2
• Removed labels: Milestone-Release2.1

[GoogleCodeExporter]

Судя по всему, не получится сделать 
абстрактный вопрос в чистом виде. Функция 
get_plugin_list из moodlelib возвращает все, для чего 
создана отдельная папка в question\type.

Зато у question_type'ов есть функция is_real_question_type(). 
Если она возвращает false, то при создании 
нового вопроса этот тип вопроса будет 
отображен ниже обычных вопросов, после 
черты. Так работает, например, 
question\type\description.

Так что предлагаю наш абстрактный класс 
унаследовать от short answer'а, перегрузить эту 
функцию на возврат false, а потом в остальных 
наследниках снова на true. Ну и написать 
юзерам комментарий к poasquestion, что это не 
вопрос, а такая вот абстрактная штуковина.

Original comment by vostreltsov@gmail.com on 7 Jun 2012 at 12:17

[GoogleCodeExporter]

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

Original comment by oasyc...@gmail.com on 7 Jun 2012 at 12:22

[GoogleCodeExporter]

У меня тут внезапный результат. Я посмотрел 
как сделано в shortanswer'е, попробовал так же - и 
оно работает! Чудесным образом все 
восстанавливается - нотация, движок, 
генерация подсказок и т. д.

Что интересно - я грепнул функцию 
extra_answer_fields(), и оказалось, что она 
реализована только в shortanswer'е и у нас. Без 
нее backup/restore не работает. Судя по всему, 
shortanswer и preg - единственные 
восстанавливающиеся вопросы:)

Я создал клон vostreltsov-nfa-preg-22-backup-restore, 
изменение в нем.

Original comment by vostreltsov@gmail.com on 8 Jun 2012 at 6:28

[GoogleCodeExporter]

Добавлен абстрактный poasquestion, содержащий 
backup/restore, preg сделан зависимым от него.

Остается проблема с transform_extra_fields: в 
оригинальном коде оно было размещено в 
question_type и вызывалось из него и edit_question_form. У 
нас же не гарантировано наследование от 
shortanswera, поэтому наследовать наш 
абстрактный тип вопроса от него и 
определить эту функцию в нем нельзя (та же 
история и с формами). Но задача, 
поставленная в issue решена - сейчас все 
работает через extra_question_fields(). Так что 
предлагаю issue 
переименовать\закрыть\отложить 
разбирательство с оставшейся проблемой.

Original comment by vostreltsov@gmail.com on 9 Jun 2012 at 1:24

[GoogleCodeExporter]

Original comment by vostreltsov@gmail.com on 20 Jul 2012 at 7:15

• Changed state: Fixed

[GoogleCodeExporter]

Тим написал, что в нашем патче есть ошибки 
кодинг стайла и есть штука для их проверки
https://github.com/moodlehq/moodle-local_codechecker/

Я бы поставил/поправил.

Original comment by oasyc...@gmail.com on 11 Sep 2012 at 12:04

[GoogleCodeExporter]

Исправил код в poasquestion (не трогал только 
jlex.php - там он все равно не исправим на 100% - в 
именах функций есть большие буквы).
В preg исправил часть кода, надо подождать 
пока Дмитрий восстановит ДКА, тогда 
доделаю остальное.

Original comment by vostreltsov@gmail.com on 13 Sep 2012 at 3:39

[GoogleCodeExporter]

А в самой MDL-25617?

Дмитрий вроде восстанавливал ДКА....

Original comment by oasyc...@gmail.com on 14 Sep 2012 at 4:36

[GoogleCodeExporter]

Все-таки сам патч желательно поправить. 
Врядли там много работы...

Original comment by oasyc...@gmail.com on 25 Sep 2012 at 9:24

[GoogleCodeExporter]

На текущий момент в строке 62 backup poasquestion 
есть ошибка, которая связана с тем, что имя 
поля 'question', указывающее на объект вопроса в 
БД зависит от типа вопроса. Также в restore 
поле answers затирается, что мешает 
восстанавливать вопросы в сложных случаях.

Original comment by mamontov...@gmail.com on 21 Jan 2013 at 11:13

[GoogleCodeExporter]

Имя поля зависит от типа вопроса, т.к. в 
старых вопросах оно question (и их наследниках), 
но в новых по правилам должно быть questionid. 
Поэтому тип вопроса говорит, какое у него 
поле... Это нормально

Поле answers устаревшее, оно давно не 
используется и его было лень прибить Тиму, 
но в 2.5 его наконец не будет :)

Original comment by oasyc...@gmail.com on 21 Jan 2013 at 12:25

[GoogleCodeExporter]

О судьбе поля answers см 
https://tracker.moodle.org/browse/MDL-17812

Original comment by oasyc...@gmail.com on 21 Jan 2013 at 1:03

[GoogleCodeExporter]

Когда будем делать версию под 2.5, можно 
будет убрать весь кода касающийся answers из 
poasquestion. Но это не скоро.

Дмитрий, пишите сюда свои непонятки по коду 
в бэкапе poasassignment, я тоже с ним частично 
знаком - посмотрим все втроем, что там вам 
не понравилось...

Original comment by oasyc...@gmail.com on 26 Jan 2013 at 6:56

[GoogleCodeExporter]

На данный момент код extra_answer_fields наконец 
принят в Moodle и с версии 2.7 может быть удален 
из preg. Валерий - можно теперь код из backup/restore 
poasquestion выложить  (попутно конвертнув 
shortanswer, чтобы они могли протестировать) - 
если есть желание спихнуть его на Тима со 
своей головы. Спешки нет - можно в пределах 
месяца - двух это сделать...

Original comment by oasyc...@gmail.com on 5 Apr 2014 at 4:42