Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-32285

Added "Continue" button under Error notification message (IMS CP)

    Details

    • Testing Instructions:
      Hide
      1. Add a new IMS-CP resource to a course, using the "gitta_ims_bad-manifest.zip" attached package.
      2. Click to view the package.
      3. When error page is displayed, click the "Continue" button and make sure you are redirected to the course main page.
      Show
      Add a new IMS-CP resource to a course, using the "gitta_ims_bad-manifest.zip" attached package. Click to view the package. When error page is displayed, click the "Continue" button and make sure you are redirected to the course main page.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_28_STABLE, MOODLE_29_STABLE
    • Fixed Branches:
      MOODLE_29_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-32285_master

      Description

      After uploading a problematic (bad or no manifest.xml file) IMS CP file (package)
      I am getting an error message with no continue button.
      It seems like a minor usability issue.

      Adding a "Continue" button with a link to the course's main page could be very helpful to novice teachers.
      Attached, is a proposed fix.

      EDIT:
      For testing purposes, I am attaching two IMS CP packages, One with bad (missing) manifest file and one which is perfectly OK.

        Gliffy Diagrams

        1. imscp-error.png
          29 kB

          Activity

          Hide
          salvetore Michael de Raadt added a comment -

          Thanks for suggesting that.

          As always, feel free to work on this with us.

          Show
          salvetore Michael de Raadt added a comment - Thanks for suggesting that. As always, feel free to work on this with us.
          Hide
          marina Marina Glancy added a comment -

          We have detected that this issue has been inactive for over two years and also did not collect many votes. It is possible that it has been already implemented in a more recent version of Moodle, or it is not highly demanded. There are unlimited number of ways Moodle functinality can be expanded and improved but we would like to concentrate on the features that will benefit majority of users, and which can not be implemented as plugins. If you have a suggestion for improving Moodle core, and there is no open issue for it in the tracker, please start a new forum discussion to see how many other users agree with you, and then create a new issue providing as many details as possible.

          ==BLK2YIMP20141121==

          Show
          marina Marina Glancy added a comment - We have detected that this issue has been inactive for over two years and also did not collect many votes. It is possible that it has been already implemented in a more recent version of Moodle, or it is not highly demanded. There are unlimited number of ways Moodle functinality can be expanded and improved but we would like to concentrate on the features that will benefit majority of users, and which can not be implemented as plugins. If you have a suggestion for improving Moodle core, and there is no open issue for it in the tracker, please start a new forum discussion to see how many other users agree with you, and then create a new issue providing as many details as possible. ==BLK2YIMP20141121==
          Hide
          nadavkav Nadav Kavalerchik added a comment -

          Hi Marina Glancy, I think you were too quick to close this one.
          This is some kind of a "bug" in the UI workflow.

          The suggested 1 line patch, can solve it nicely by adding a button.
          Please see if you can reopen and apply.

          Show
          nadavkav Nadav Kavalerchik added a comment - Hi Marina Glancy , I think you were too quick to close this one. This is some kind of a "bug" in the UI workflow. The suggested 1 line patch, can solve it nicely by adding a button. Please see if you can reopen and apply.
          Hide
          marina Marina Glancy added a comment -

          Sure, here you go. Added the missing 'patch' label as well.
          If anybody wants to submit the patch as a git branch with testing instructions - you are more than welcome.

          Show
          marina Marina Glancy added a comment - Sure, here you go. Added the missing 'patch' label as well. If anybody wants to submit the patch as a git branch with testing instructions - you are more than welcome.
          Hide
          nadavkav Nadav Kavalerchik added a comment -

          Original IMS CP packages are from: http://www.elml.org/website/en/html/output_cp.html

          Show
          nadavkav Nadav Kavalerchik added a comment - Original IMS CP packages are from: http://www.elml.org/website/en/html/output_cp.html
          Hide
          nadavkav Nadav Kavalerchik added a comment -

          Ready for peer review.

          Show
          nadavkav Nadav Kavalerchik added a comment - Ready for peer review.
          Hide
          cibot CiBoT added a comment -
          Show
          cibot CiBoT added a comment - Fails against automated checks. Checked MDL-32285 using repository: https://github.com/nadavkav/moodle/ master (1 errors / 0 warnings) [branch: MDL-32285_master | CI Job ] phplint (0/0) , php (0/0) , js (0/0) , css (0/0) , phpdoc (0/0) , commit (1/0) , savepoint (0/0) , thirdparty (0/0) , More information about this report
          Hide
          marina Marina Glancy added a comment -

          weird, this issue was assigned to me for review, I don't remember doing this... Removing myself as a peer reviewer so the issue goes back in the HQ peer review dashboard

          Show
          marina Marina Glancy added a comment - weird, this issue was assigned to me for review, I don't remember doing this... Removing myself as a peer reviewer so the issue goes back in the HQ peer review dashboard
          Hide
          nadavkav Nadav Kavalerchik added a comment -

          Hi Marina Glancy, As far as I remember, I assigned you as a peer reviewer, probably not according to Moodle HQ flow. Sorry. Thought it was OK thing to do.

          It's a one line code change, maybe you can still review it (and I will rebase it too) and we could move this tiny issue forward

          Show
          nadavkav Nadav Kavalerchik added a comment - Hi Marina Glancy , As far as I remember, I assigned you as a peer reviewer, probably not according to Moodle HQ flow. Sorry. Thought it was OK thing to do. It's a one line code change, maybe you can still review it (and I will rebase it too) and we could move this tiny issue forward
          Hide
          marina Marina Glancy added a comment -

          sure, it's good code-wise. The only thing is that commit message is not up to the standard - do you want to correct it? First line should be under 72 chars, otherwise it looks cropped anyway but hard to understand in the commits list

          I'm sending it for integration anyway, but if you can change the commit message it'd be cool.

          P.S. just fyi, there is an easy way to build course url, even returning to the proper seciton course_get_url($course->id, $cm->section)
          But your link also works fine

          Show
          marina Marina Glancy added a comment - sure, it's good code-wise. The only thing is that commit message is not up to the standard - do you want to correct it? First line should be under 72 chars, otherwise it looks cropped anyway but hard to understand in the commits list I'm sending it for integration anyway, but if you can change the commit message it'd be cool. P.S. just fyi, there is an easy way to build course url, even returning to the proper seciton course_get_url($course->id, $cm->section) But your link also works fine
          Hide
          nadavkav Nadav Kavalerchik added a comment -

          Loooong commit messages was (is) always my big change. I am not a native English speaker and I tend to over describe the subject
          I rephrased it. hope it is still clear.

          And thank you for the course_get_url tip. I was not familiar with this function.

          I have changed the fix to include the course_get_url function and shortened the commit message.
          Thanks

          Show
          nadavkav Nadav Kavalerchik added a comment - Loooong commit messages was (is) always my big change. I am not a native English speaker and I tend to over describe the subject I rephrased it. hope it is still clear. And thank you for the course_get_url tip. I was not familiar with this function. I have changed the fix to include the course_get_url function and shortened the commit message. Thanks
          Hide
          marina Marina Glancy added a comment -

          Thank you Nadav

          btw, commit message can be multiline if you need to add more information, it's just the first line should be very compact.

          Show
          marina Marina Glancy added a comment - Thank you Nadav btw, commit message can be multiline if you need to add more information, it's just the first line should be very compact.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          nadavkav Nadav Kavalerchik added a comment -

          Rebased (20/3)

          Show
          nadavkav Nadav Kavalerchik added a comment - Rebased (20/3)
          Hide
          cibot CiBoT added a comment -

          Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

          Show
          cibot CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
          Hide
          cibot CiBoT added a comment -

          Code verified against automated checks.

          Checked MDL-32285 using repository: https://github.com/nadavkav/moodle/

          More information about this report

          Show
          cibot CiBoT added a comment - Code verified against automated checks. Checked MDL-32285 using repository: https://github.com/nadavkav/moodle/ master (0 errors / 0 warnings) [branch: MDL-32285_master | CI Job ] More information about this report
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Integrated (master only), thanks!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (master only), thanks!
          Hide
          markn Mark Nelson added a comment -

          Works as expected, thanks Nadav.

          Show
          markn Mark Nelson added a comment - Works as expected, thanks Nadav.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Your amazing work is now part of Moodle, the mother of all LMSs ! As a reward, be sure that Moodle, the Mum, loves you with all her heart and soul! Many thanks!

          Closing as fixed, ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Your amazing work is now part of Moodle, the mother of all LMSs ! As a reward, be sure that Moodle, the Mum, loves you with all her heart and soul! Many thanks! Closing as fixed, ciao

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                11/May/15