Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Calendar
    • Labels:
    • Testing Instructions:
      Hide
      1. Log in to Moodle as a student
      2. Go to Navigation > Site pages > Calendar.
      3. Click the button 'Manage subscriptions'.
      4. Try importing a calendar with the URL http://tinyurl.com/at55c97
      5. Make sure it is imported and all events are created (if the linked issue is fixed) else events wont be created.
      Show
      Log in to Moodle as a student Go to Navigation > Site pages > Calendar. Click the button 'Manage subscriptions'. Try importing a calendar with the URL http://tinyurl.com/at55c97 Make sure it is imported and all events are created (if the linked issue is fixed) else events wont be created.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:
      MDL-36525-master

      Description

      When trying to import a calendar using a URL to an ICS file (not a script that generates an iCal data), the import process throws an error.

      1. Log in to Moodle as a student
      2. Go to Navigation > Site pages > Calendar.
      3. Click the button 'Manage subscriptions'.
      4. Try importing a calendar with the URL http://tinyurl.com/at55c97

      Expected result: The file should be used as the basis of a calendar

      Actual result: An error is shown, but the calendar entry is still created.

      The given iCal URL is invalid.
       
      More information about this error
      Debug info:
      Error code: errorinvalidicalurl
      Stack trace:
       
          line 2873 of /calendar/lib.php: moodle_exception thrown
          line 2950 of /calendar/lib.php: call to calendar_get_icalendar()
          line 78 of /calendar/managesubscriptions.php: call to calendar_update_subscription_events()

      I suspect this has something to do with the call to $curl->get() failing at line 2871 of calendar/lib.php. This might be because the URL of the calendar above forces the user to download the file, rather than giving the content as a response. Trying the following URL works. Using the URLs directly in your browser highlights this distinction.

      http://www.mozilla.org/projects/calendar/caldata/AustraliaHolidays.ics

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              ankit_frenz Ankit Agarwal added a comment -

              While working on this I noticed some files dont work, while others did with the correct format. after spending quite some time debugging the problem I found was the "newline" character used in those files i.e "\n" instead of "\r\n". Which was breaking everything as $ical::unserialize couldnt find the event details as it does an explode on "\r\n" only. I will create a blocker for that .
              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - While working on this I noticed some files dont work, while others did with the correct format. after spending quite some time debugging the problem I found was the "newline" character used in those files i.e "\n" instead of "\r\n". Which was breaking everything as $ical::unserialize couldnt find the event details as it does an explode on "\r\n" only. I will create a blocker for that . Thanks
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Sending for peer review
              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - Sending for peer review Thanks
              Hide
              rwijaya Rossiani Wijaya added a comment -

              Hi Ankit,

              The import is working as expected. However it doesn't create any events.

              I had a discussion with Ankit and he explained that events creation will be fix through MDL-36592.

              [y] Syntax
              [y] Output
              [y] Whitespace
              [-] Language
              [-] Databases
              [y] Testing
              [-] Security
              [-] Documentation
              [y] Git
              [y] Sanity check

              Show
              rwijaya Rossiani Wijaya added a comment - Hi Ankit, The import is working as expected. However it doesn't create any events. I had a discussion with Ankit and he explained that events creation will be fix through MDL-36592 . [y] Syntax [y] Output [y] Whitespace [-] Language [-] Databases [y] Testing [-] Security [-] Documentation [y] Git [y] Sanity check
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Thanks Rosie for the review.
              Pushing this for integration.
              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - Thanks Rosie for the review. Pushing this for integration. Thanks
              Hide
              poltawski Dan Poltawski added a comment -

              Hi Ankit,

              Regarding:

              $calendar = $curl->get($url, '', array('CURLOPT_FOLLOWLOCATION' => 1, 'CURLOPT_MAXREDIRS' => 5));

              The second argument should be an array, an empty string isn't valid. So that is not right at all.

              I think it'd could be neater to use a setopt() call before doing the get, since we're already creating an instance and you can split that up easily avoiding sending the blank argument.

              Show
              poltawski Dan Poltawski added a comment - Hi Ankit, Regarding: $calendar = $curl->get($url, '', array('CURLOPT_FOLLOWLOCATION' => 1, 'CURLOPT_MAXREDIRS' => 5)); The second argument should be an array , an empty string isn't valid. So that is not right at all. I think it'd could be neater to use a setopt() call before doing the get, since we're already creating an instance and you can split that up easily avoiding sending the blank argument.
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Thanks Dan for the feedback
              I have made changes.
              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - Thanks Dan for the feedback I have made changes. Thanks
              Hide
              poltawski Dan Poltawski added a comment -

              Thanks Ankit, i've integrated this now.

              Show
              poltawski Dan Poltawski added a comment - Thanks Ankit, i've integrated this now.
              Hide
              poltawski Dan Poltawski added a comment -

              Passed (by creating a tinyurl to use a good redirect )

              Show
              poltawski Dan Poltawski added a comment - Passed (by creating a tinyurl to use a good redirect )
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Many, many thanks for your effort!

              Millions of people will enjoy the results of your work, yay!

              Closing as fixed. Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Many, many thanks for your effort! Millions of people will enjoy the results of your work, yay! Closing as fixed. Ciao

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    3/Dec/12