Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Blocker 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

          Issue Links

            Activity

            Hide
            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 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 Agarwal added a comment -

            Sending for peer review
            Thanks

            Show
            Ankit Agarwal added a comment - Sending for peer review Thanks
            Hide
            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
            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 Agarwal added a comment -

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

            Show
            Ankit Agarwal added a comment - Thanks Rosie for the review. Pushing this for integration. Thanks
            Hide
            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
            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 Agarwal added a comment -

            Thanks Dan for the feedback
            I have made changes.
            Thanks

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

            Thanks Ankit, i've integrated this now.

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

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

            Show
            Dan Poltawski added a comment - Passed (by creating a tinyurl to use a good redirect )
            Hide
            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
            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: