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
    • Rank:
      45991

      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

        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: