Project

General

Profile

Bug #966

A transaction creates a problem (Learnpath class was renamed)

Added by Ivan Tcholakov over 9 years ago. Updated over 9 years ago.

Status:
Feature implemented
Priority:
Normal
Category:
-
Target version:
Start date:
09/04/2010
Due date:
% Done:

100%

Estimated time:
Complexity:
Normal
SCRUM pts - complexity:
?

Description

I have an expectation that I think is normal: when some feature has been introduced and it works, it is duty of all developers not to break it. Nevertheless that the product is free, we have the obligation to provide some basic level of quality. This product is created to provide people fun and benefit, it is not created to waste peoples' time.

In the light of above thoughts I would like to express my objection about the transaction http://code.google.com/p/chamilo/source/detail?r=0142f6c410f36368427bdcffa3cb9334e46a0f3d&repo=classic , made by Julio. This transaction is unethical. I am against the approach of solving problems by making problems to the others (me for example) and breaking compatibility.

Technical details: A class has been renamed. As a result, course archives with learning path inside and created on prior systems can not be read. Fatal error occurs, see the attached picture.

I appeal to Julio Montoya to solve this additional problem too, because he created it. Please, maintain compatibility.


Files

History

#2

Updated by Julio Montoya over 9 years ago

  • Subject changed from A transaction creates a problem intentionally (Learnpath class was renamed) to A transaction creates a problem (Learnpath class was renamed)
#3

Updated by Ivan Tcholakov over 9 years ago

I did a test, now importing works. Thank you for understanding. :-)

#4

Updated by Yannick Warnier over 9 years ago

  • Status changed from New to Bug resolved
  • % Done changed from 0 to 100

I understand this task can be closed. Thanks to all for your contributions.

#5

Updated by Ivan Tcholakov over 9 years ago

  • Status changed from Bug resolved to Needs more info
  • % Done changed from 100 to 90

I am sorry but I suspect that the same problem appeared again. See the transaction http://code.google.com/p/chamilo/source/detail?r=ce38c5e7bd7b3d97e6f3764aa1c34b4bee49a77c&repo=classic

Could you find another way to solve the initial problem?

Anyway, I simply want my archived content (for which I invested time) to load.

#6

Updated by Ivan Tcholakov over 9 years ago

  • Target version set to 1.8.7 beta
#7

Updated by Ivan Tcholakov over 9 years ago

I have the same fatal error again, see the picture.

#8

Updated by Ivan Tcholakov over 9 years ago

Guillaume, I don't know whether it is important renaming the class Learnpath (copy course tool), because I am not able to find the task #869 .

Why renaming is important? If so, then please, test the following fix for the newly created problem:

File: chamilo/main/coursecopy/classes/CourseArchiver.class.php
Line 209 (within the method read_course()):

        $course = unserialize(base64_decode($contents));

to be replaced with these two lines:
        // For backward comaptibility: Before unserialization, if class name "Learnpath" was found, it should be renamed as "CourseCopyLearnpath".
        $course = unserialize(str_replace('O:9:"Learnpath":', 'O:19:"CourseCopyLearnpath":', base64_decode($contents)));

#9

Updated by Anonymous over 9 years ago

Hello Ivan,

First of all, you're not the only one who wants to maintain compatibility, so there is no need for messages such as "please, maintain compatibility" written in bold letters. We all do some changes which can, potentially, without us being aware of it, break compatibility or introduce new bugs (including, for example, changes to the chamilo installer which break compatibility with http://testing.beeznest.com).

Concerning the task 869, you probably can't see it because it's part of the Chamilo quality assurance project instead of being part of the Chamilo 1.8 project. But basically, if you look in the Chamilo's code, you'll see that there are (were) two classes named Learnpath (one in main/coursecopy/classes and another one in main/newscorm). This creates a fatal error when you go to Administration > configuration settings > Training and select "Icons and text" under "Display training navigation menu", and then go to course maintenance and try to create a backup of a course, you'll see (with the code before I modified it) a fatal error saying that the Learnpath class can not be redeclared.

I believe your bug appears when you're trying to restore a course that has been previously backed up right ? In order for me to test the changes you are proposing, could you please send me the file you're trying to restore ?

Thank you.

#10

Updated by Ivan Tcholakov over 9 years ago

I think that the appeal Please, maintain compatibility should stay in big bold letters in some of the Wiki pages. The same problem appears twice. To my regret, I feel that I am the only one who concerns about compatibility at the moment.

You are telling me that there is "Chamilo quality assurance project" that is not part of "Chamilo 1.8 project". Why this nothing-to-do project creates a problem to me then? Could you not introduce bugs from the "Chamilo quality assurance project" here?

You may create (actually, you should create) a file for testing purposes, because you are the one who introduces the problem this time. You may export a file from Chamilo 1.8.6.2 that contains a learning path, then try to import it in Chamilo 1.8.7.

I work with a real course, I can not send it to you, sorry.

If you are reluctant to dedicate some of your time on this, no problem, just tell me. I will take this task then. I want to see my problem solved.

#11

Updated by Anonymous over 9 years ago

Hi Ivan,

Yes, I'm sure you're the only one who is concerned about compatibility, and you're also probably the only one good enough to handle properly the chamilo project. However, just as a reminder, some changes made to the Chamilo installer broke compatibility with testing.beeznest.com, so please, maintain compatibility.

Concerning the "Chamilo quality assurance project", it was created mainly for the testers, and the bug I'm talking about, for some reason, appears in this "Chamilo quality assurance project". If you want to have access to the "Chamilo quality assurance", please, ask Yannick.

Concerning the course, if I create a course right now with a learning path, it will use the new code (with the CourseCopyLearnpath class), so having a course that's been created BEFORE the code was modified would really help.

#12

Updated by Ivan Tcholakov over 9 years ago

  • Status changed from Needs more info to Assigned
  • Assignee set to Ivan Tcholakov

I am taking this task.

#13

Updated by Ivan Tcholakov over 9 years ago

Guillaume Viguier-Just wrote:

Hi Ivan,

Yes, I'm sure you're the only one who is concerned about compatibility, and you're also probably the only one good enough to handle properly the chamilo project. However, just as a reminder, some changes made to the Chamilo installer broke compatibility with testing.beeznest.com, so please, maintain compatibility.
...

Hello Guillaume,

I see, there are some changes in the installer that attracted your attention and that you are not agree with. Are these changes made by me? If so, is there a task opened, so I can see the details about the problem? I probably missed to read it.

Thank you.

#15

Updated by Ivan Tcholakov over 9 years ago

  • % Done changed from 0 to 80

11556:2542a9e19d85 Bug #966 - Importing learning paths from archives created with Chamilo 1.8.6.2 has been recovered.
http://code.google.com/p/chamilo/source/detail?r=2542a9e19d855cbbc6b18e678f2c250a7943bd51&repo=classic

#16

Updated by Ivan Tcholakov over 9 years ago

11557:01aaf964647f Bug #966 - Testing: Minor change, adding a closing bracket.
http://code.google.com/p/chamilo/source/detail?r=01aaf964647fb89f674b16becdcbdccb1faa3a70&repo=classic

#17

Updated by Ivan Tcholakov over 9 years ago

I think, due to the class renaming, there are some additional (minor) changes to be done in the Course copy tool.

Here is a test for Chamilo 1.8.7 alpha (between 2 and 3): I created an archive with learning path inside, then I imported this archive in another course in the same system. As a result, in the destination course the learning path was missing.

I am researching code to figure out these additional changes to be done.

#18

Updated by Ivan Tcholakov over 9 years ago

11560:c718c75a7733 Bug #966 - Testing, a trivial fix: The former Learnpath class had a PHP4-style constructor. In the renamed class CourseCopyLearnpath we also have to rename the constructor accordingly.
http://code.google.com/p/chamilo/source/detail?r=c718c75a7733feb002114c8fa159716c52fcdf86&repo=classic

#19

Updated by Ivan Tcholakov over 9 years ago

  • Status changed from Assigned to Feature implemented
  • % Done changed from 80 to 100

Tested by me, importing learning paths from Chamilo 1.8.x works.

Also available in: Atom PDF