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

          Attachments

            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