Moodle
  1. Moodle
  2. MDL-40214

Custom Scale does not restore when associated with Assignments.

    Details

    • Rank:
      50968

      Description

      Summary: An Assignment activity which uses a custom scale, when backed up and restored to a new course will not use that custom scale.

      Steps to Reproduce:

      1. Create a custom scale in a course;
      2. Create an Assignment activity in the course which uses that custom scale.
      3. Backup that course and restore it to a new course.
      4. The same activity in the restored course does not use the custom scale.

      Actual Results: An assignment activity which uses a custom scale, when backed up and restored to a new course does not use the custom scale.

      Expected Results: An assignment activity which uses a custom scale, when backed up and restored to a new course should use the same custom scale.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that.

          Feel free to help us work on this issue.

          Show
          Michael de Raadt added a comment - Thanks for reporting that. Feel free to help us work on this issue.
          Hide
          Colin Campbell added a comment -

          This is an easy fix. Looks like somebody just forgot to put in a few lines when coding the new assign module restore logic.

          In mod/assign/backup/moodle2/restore_assign_stepslib.php, function process_assign needs these lines:

              if ($data->grade < 0) { // scale found, get mapping
                  $data->grade = -($this->get_mappingid('scale', abs($data->grade)));
              }   
          
          Show
          Colin Campbell added a comment - This is an easy fix. Looks like somebody just forgot to put in a few lines when coding the new assign module restore logic. In mod/assign/backup/moodle2/restore_assign_stepslib.php, function process_assign needs these lines: if ($data->grade < 0) { // scale found, get mapping $data->grade = -($ this ->get_mappingid('scale', abs($data->grade))); }
          Hide
          Mark Nelson added a comment -

          Hi Colin, would you be able to put this patch onto github.com? That way we can pull it in and credit your work.

          Show
          Mark Nelson added a comment - Hi Colin, would you be able to put this patch onto github.com? That way we can pull it in and credit your work.
          Hide
          Colin Campbell added a comment -

          Hi Mark. I added the 2.4 pull information. Please let me know if I need to do anything different procedurally.

          Show
          Colin Campbell added a comment - Hi Mark. I added the 2.4 pull information. Please let me know if I need to do anything different procedurally.
          Hide
          Mark Nelson added a comment -

          Whoops, assigned to the wrong Colin. Sorry about that! Since I was unable to assign this issue to you Colin I have assigned this to myself. Also, I have taken your commit and have made some changes to the commit message and comment.

          Show
          Mark Nelson added a comment - Whoops, assigned to the wrong Colin. Sorry about that! Since I was unable to assign this issue to you Colin I have assigned this to myself. Also, I have taken your commit and have made some changes to the commit message and comment.
          Hide
          Mark Nelson added a comment -

          There was also conflicts on 2.5 and master which I fixed.

          Show
          Mark Nelson added a comment - There was also conflicts on 2.5 and master which I fixed.
          Hide
          Mark Nelson added a comment -

          Damyon Wiese since this is your area of expertise would you mind peer reviewing this?

          Show
          Mark Nelson added a comment - Damyon Wiese since this is your area of expertise would you mind peer reviewing this?
          Hide
          Damyon Wiese added a comment -

          +1 from me

          I just added to the test instructions.

          Sending for integration.

          Thankyou!

          Show
          Damyon Wiese added a comment - +1 from me I just added to the test instructions. Sending for integration. Thankyou!
          Hide
          Dan Poltawski added a comment -

          Thanks Colin.

          Looks good and i've integrated to master, 25 and 24

          Show
          Dan Poltawski added a comment - Thanks Colin. Looks good and i've integrated to master, 25 and 24
          Hide
          Mark Nelson added a comment -

          Sorry, accidentally unassigned myself.

          Show
          Mark Nelson added a comment - Sorry, accidentally unassigned myself.
          Hide
          Rossiani Wijaya added a comment -

          Hi guys,

          During the restore process, it produced the following error message:

          Did you remember to make the first column something unique in your call to get_records? Duplicate value 'server.xyz+110517111136+s8n0Dl server.xyz+110517111136+ysBliJ' found in column 'concat(stamp, ' ', version)'.
          
              line 1005 of /lib/dml/mysqli_native_moodle_database.php: call to debugging()
              line 1334 of /lib/dml/moodle_database.php: call to mysqli_native_moodle_database->get_records_sql()
              line 620 of /backup/util/dbops/restore_dbops.class.php: call to moodle_database->get_records_sql_menu()
              line 484 of /backup/util/dbops/restore_dbops.class.php: call to restore_dbops::prechek_precheck_qbanks_by_level()
              line 135 of /backup/util/helper/restore_prechecks_helper.class.php: call to restore_dbops::precheck_categories_and_questions()
              line 366 of /backup/controller/restore_controller.class.php: call to restore_prechecks_helper::execute_prechecks()
              line 638 of /backup/util/ui/restore_ui_stage.class.php: call to restore_controller->execute_precheck()
              line 284 of /backup/util/ui/restore_ui.class.php: call to restore_ui_stage_process->process()
              line 60 of /backup/restore.php: call to restore_ui->requires_substage()
          

          I don't think this is related to the patch, but will do the test without the patch and report back.

          Show
          Rossiani Wijaya added a comment - Hi guys, During the restore process, it produced the following error message: Did you remember to make the first column something unique in your call to get_records? Duplicate value 'server.xyz+110517111136+s8n0Dl server.xyz+110517111136+ysBliJ' found in column 'concat(stamp, ' ', version)'. line 1005 of /lib/dml/mysqli_native_moodle_database.php: call to debugging() line 1334 of /lib/dml/moodle_database.php: call to mysqli_native_moodle_database->get_records_sql() line 620 of /backup/util/dbops/restore_dbops.class.php: call to moodle_database->get_records_sql_menu() line 484 of /backup/util/dbops/restore_dbops.class.php: call to restore_dbops::prechek_precheck_qbanks_by_level() line 135 of /backup/util/helper/restore_prechecks_helper.class.php: call to restore_dbops::precheck_categories_and_questions() line 366 of /backup/controller/restore_controller.class.php: call to restore_prechecks_helper::execute_prechecks() line 638 of /backup/util/ui/restore_ui_stage.class.php: call to restore_controller->execute_precheck() line 284 of /backup/util/ui/restore_ui.class.php: call to restore_ui_stage_process->process() line 60 of /backup/restore.php: call to restore_ui->requires_substage() I don't think this is related to the patch, but will do the test without the patch and report back.
          Hide
          Rossiani Wijaya added a comment - - edited

          Update: The errors exists without the patch.

          The outcome from restoring a course is not the same with the backuped course. I'm not sure if it happen because of the errors during restore. (attaching backup file)

          Show
          Rossiani Wijaya added a comment - - edited Update: The errors exists without the patch. The outcome from restoring a course is not the same with the backuped course. I'm not sure if it happen because of the errors during restore. (attaching backup file)
          Hide
          Mark Nelson added a comment -

          Hi Rosie,

          If it exists w/o the patch I suggest creating another issue with your backup attached and link to this as 'testing discovered'. Looking at the code it's an issue with questions, so may be completely unrelated. I suggest creating a completely new course and ensure there are no questions that are backed up, then repeat the testing instructions.

          Regards,

          Mark

          Show
          Mark Nelson added a comment - Hi Rosie, If it exists w/o the patch I suggest creating another issue with your backup attached and link to this as 'testing discovered'. Looking at the code it's an issue with questions, so may be completely unrelated. I suggest creating a completely new course and ensure there are no questions that are backed up, then repeat the testing instructions. Regards, Mark
          Hide
          Eloy Lafuente (stronk7) added a comment -

          +1 to separate issue.

          I think I've got that error sometimes when a given restore has failed in the middle (timeout or whatever). Then, the second attempt to restore the same course, in the same category, leads to that error.

          Just in case it helps to reproduce it constantly.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - +1 to separate issue. I think I've got that error sometimes when a given restore has failed in the middle (timeout or whatever). Then, the second attempt to restore the same course, in the same category, leads to that error. Just in case it helps to reproduce it constantly. Ciao
          Hide
          Rossiani Wijaya added a comment -

          Thanks Mark and Eloy for the feedbacks.

          Retested this again with new course and 1 assignment activity, it works as expected.

          I'm creating issue to fix the above error messages in MDL-41601.

          Tested this on 2.4, 2.5 and master.

          Test passed.

          Show
          Rossiani Wijaya added a comment - Thanks Mark and Eloy for the feedbacks. Retested this again with new course and 1 assignment activity, it works as expected. I'm creating issue to fix the above error messages in MDL-41601 . Tested this on 2.4, 2.5 and master. Test passed.
          Hide
          Dan Poltawski added a comment -

          Congratulations! This change has been integrated upstream and is now available from our git and download mirrors. To celebrate, here is a joke:

          A SQL query goes into a bar, walks up to two tables and asks, "Can I join you?"

          Show
          Dan Poltawski added a comment - Congratulations! This change has been integrated upstream and is now available from our git and download mirrors. To celebrate, here is a joke: A SQL query goes into a bar, walks up to two tables and asks, "Can I join you?"

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: