1. Oleg Sychev Avatar Oleg Sychev
  2. POAS Moodle plugins
  3. Moodle Plugins
  4. Issues

Fix MDL-25617 urgently

Issue #50 resolved
Oleg Sychev repo owner created an issue

Originally reported on Google Code with ID 50

Начиная с 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

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

Reported by oasychev on 2011-11-15 11:55:12

Comments (37)

  1. Oleg Sychev reporter

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

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

    Reported by `oasychev` on 2011-11-15 13:32:27

  2. Oleg Sychev reporter

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

    Reported by `oasychev` on 2011-11-20 11:06:40

  3. Valeriy Streltsov

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

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

    ```

    Reported by `vostreltsov` on 2011-11-20 13:58:48

  4. Oleg Sychev reporter

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

    Reported by `oasychev` on 2011-11-20 14:48:14

  6. Oleg Sychev reporter

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

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

    Reported by `oasychev` on 2011-11-25 23:05:34

  7. Oleg Sychev reporter

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

    Reported by `oasychev` on 2011-11-26 17:08:21

  8. Oleg Sychev reporter

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

    Reported by `oasychev` on 2011-11-26 17:11:36

  10. Oleg Sychev reporter

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

    Reported by `oasychev` on 2011-11-26 21:24:59

  11. Oleg Sychev reporter

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

    Reported by `oasychev` on 2011-11-27 21:26:31

  13. Oleg Sychev reporter

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

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

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

    Reported by `oasychev` on 2011-11-29 08:07:03 - Status changed: `InProgress`

  14. Oleg Sychev reporter

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

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

    Reported by `oasychev` on 2011-12-02 20:15:39

  15. Valeriy Streltsov

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

    Reported by `vostreltsov` on 2011-12-03 03:06:32

  16. Oleg Sychev reporter

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

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

    Reported by `oasychev` on 2011-12-03 14:25:16

  17. Valeriy Streltsov

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

    Reported by `vostreltsov` on 2011-12-03 20:20:23

  18. Oleg Sychev reporter

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

    Reported by `oasychev` on 2011-12-04 12:42:30

  19. Oleg Sychev reporter

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

    Reported by `oasychev` on 2011-12-14 11:13:51

  20. Oleg Sychev reporter

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

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

    Reported by `oasychev` on 2011-12-14 11:23:43

  21. Oleg Sychev reporter

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

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

    Reported by `oasychev` on 2011-12-14 14:16:51

  22. Oleg Sychev reporter

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

    Reported by `oasychev` on 2011-12-23 10:48:06

  23. Oleg Sychev reporter

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

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

    Reported by `oasychev` on 2012-06-06 16:28:55 - Labels added: Milestone-Release2.2 - Labels removed: Milestone-Release2.1

  24. Valeriy Streltsov

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

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

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

    Reported by `vostreltsov` on 2012-06-07 12:17:50

  25. Oleg Sychev reporter

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

    Reported by `oasychev` on 2012-06-07 12:22:21

  26. Valeriy Streltsov

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

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

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

    Reported by `vostreltsov` on 2012-06-08 18:28:37

  27. Valeriy Streltsov

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

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

    Reported by `vostreltsov` on 2012-06-09 13:24:50

  30. Valeriy Streltsov

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

    Reported by `vostreltsov` on 2012-09-13 15:39:43

  31. Oleg Sychev reporter

    ``` А в самой MDL-25617?

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

    Reported by `oasychev` on 2012-09-14 04:36:28

  32. Oleg Sychev reporter

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

    Reported by `oasychev` on 2012-09-25 09:24:54

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

    Reported by mamontov.dp on 2013-01-21 11:13:11

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

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

    Reported by oasychev on 2013-01-21 12:25:42

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

    Reported by oasychev on 2013-01-21 13:03:30

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

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

    Reported by oasychev on 2013-01-26 18:56:44

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

    Reported by oasychev on 2014-04-05 16:42:24

  38. Log in to comment
Assignee
Type
bug
Priority
critical
Status
resolved
Component
–
Votes
0
Watchers
0
Jira: the preferred issue tracker for Bitbucket. Join the team!