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

Api for updating calendar subscriptions

    Details

    • Testing Instructions:
      Hide
      1. run phpunit --verbose calendar/tests/calendarical_tests.php and make sure it passes
      2. A Regression testing with calendar subscriptions is required i.e (add, edit, update, delete etc)
        Test
      3. Goto site>calendar>manage subscription
      4. Add an ical subscription using url
        code}
        https://www.google.com/calendar/ical/en.australian%23holiday%40group.v.calendar.google.com/public/basic.ics

         

      5. Not the details in database (event_subscription).
      6. Goto the same page again and from the top list of subscription, update the time interval for the one that you just added
      7. Check in database pollinterval is updated accordingly
      Show
      run phpunit --verbose calendar/tests/calendarical_tests.php and make sure it passes A Regression testing with calendar subscriptions is required i.e (add, edit, update, delete etc) Test Goto site>calendar>manage subscription Add an ical subscription using url code} https://www.google.com/calendar/ical/en.australian%23holiday%40group.v.calendar.google.com/public/basic.ics   Not the details in database (event_subscription). Goto the same page again and from the top list of subscription, update the time interval for the one that you just added Check in database pollinterval is updated accordingly
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:
      MDL-37562-master

      Description

      while working on MDL-36621, I noticed we need an api to update ical subscriptions instead of doing update_record all over the place. Also would be easier to invalidate cache with an api.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              samhemelryk Sam Hemelryk added a comment -

              Hi Ankit,

              Thanks for the report.
              I've changed this to an improvement issue as it sounds like what you are talking about is introducing two new things:

              1. An API.
              2. Caching

              Both very much worth doing, but both things that likely can't be backported.

              Many thanks
              Sam

              Show
              samhemelryk Sam Hemelryk added a comment - Hi Ankit, Thanks for the report. I've changed this to an improvement issue as it sounds like what you are talking about is introducing two new things: An API. Caching Both very much worth doing, but both things that likely can't be backported. Many thanks Sam
              Hide
              rwijaya Rossiani Wijaya added a comment -

              Hi Ankit,

              Some feedback for the patch.

              calendar/lib.php
               
              line 2804: remove $cache declaration
              line 3063-3068: use || (or logic) since it is using the same string otherwise make the string different.
              line 3070: capitalize for 'Update'
              

              The rest of the patch looks great.

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

              Show
              rwijaya Rossiani Wijaya added a comment - Hi Ankit, Some feedback for the patch. calendar/lib.php   line 2804: remove $cache declaration line 3063-3068: use || (or logic) since it is using the same string otherwise make the string different. line 3070: capitalize for 'Update' The rest of the patch looks great. [y] Syntax [-] Output [y] Whitespace [-] Language [-] Databases [-] Testing [-] Security [-] Documentation [y] Git [y] Sanity check
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Thanks Rosie for the review,
              Patch has been updated.
              Submitting for integration.
              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - Thanks Rosie for the review, Patch has been updated. Submitting for integration. Thanks
              Hide
              poltawski Dan Poltawski 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
              poltawski Dan Poltawski 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
              damyon Damyon Wiese added a comment -

              Hi Ankit

              This change looks good - can you just add a line to calendar/upgrade.txt describing the new function?

              Thanks!

              Show
              damyon Damyon Wiese added a comment - Hi Ankit This change looks good - can you just add a line to calendar/upgrade.txt describing the new function? Thanks!
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Thanks Damyon,
              Changes made.

              Show
              ankit_frenz Ankit Agarwal added a comment - Thanks Damyon, Changes made.
              Hide
              damyon Damyon Wiese added a comment -

              Thanks Ankit - One more thing I spotted is that the test file is incorrectly named - it should be "calendarical_test.php" or this won't be run by a ./vendor/bin/phpunit with no arguments.

              The other file "externallib_tests.php" is wrong too (I'll add an issue about that and fix the check on the ci server to detect these).

              Show
              damyon Damyon Wiese added a comment - Thanks Ankit - One more thing I spotted is that the test file is incorrectly named - it should be "calendarical_test.php" or this won't be run by a ./vendor/bin/phpunit with no arguments. The other file "externallib_tests.php" is wrong too (I'll add an issue about that and fix the check on the ci server to detect these).
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Updated.
              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - Updated. Thanks
              Hide
              damyon Damyon Wiese added a comment -

              Thanks Ankit,

              This has been integrated to master only.

              Show
              damyon Damyon Wiese added a comment - Thanks Ankit, This has been integrated to master only.
              Hide
              andyjdavis Andrew Davis added a comment -

              Works as described. Passing.

              Show
              andyjdavis Andrew Davis added a comment - Works as described. Passing.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Your awesome contributions are now part of Moodle, your fav LMS out there.

              Closing this as fixed.

              Many thanks for all the hard work, ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Your awesome contributions are now part of Moodle, your fav LMS out there. Closing this as fixed. Many thanks for all the hard work, ciao

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    14/May/13