core_course_delete_courses

We have to work in this function definition. What about associated ressources, roles ... Must be destroyed?

/**

• Deletes a course from the course table
• @private ?
• @param string $field
• Possible values:
• <ul>
• <li>id</li>
• <li>name</li>
• <li>idnumber</li>
• <li>shortname</li>
• </ul>
• @param string $value
• @return boolean
  */
  function delete_course($value, $field){
  }

/**

• Deletes some courses from the course table
• @param string $field
• Possible values:
• <ul>
• <li>id</li>
• <li>name</li>
• <li>idnumber</li>
• <li>shortname</li>
• </ul>
• @param array $values
• @uses delete_course
• @return boolean
  */
  function delete_courses($field, $values){
  }

This funcionality is completed in CVS.
We have commit a simpletest.

Will not be resolve for 2.0 as it's a pretty sensitive area, moving fix version to 2.0.1

We have funding for implementing this function:

This function will perform a full delete of the specified courses

Parameters:
----------

array of course ids

Return value:
-----------
Array of course ids deleted

array [id]

Jerome, are we in time for getting this in 2.3? I will have the code for review the 20th of this month

Adding the main function code for a peer review:

https://gist.github.com/2399550

Sorry for not adding the link to the full branch, but I'm developing this as a local plugin so once reviewed I will create the pull Moodle DEV branch (this is mainly for avoid rebasing)

Thanks Juan. Can you also add in the test instruction: a link to the demo client (https://github.com/moodlehq/sample-ws-clients/tree/master/PHP-REST) with a parameters section example:

/// PARAMETERS
$params = array('2','3') //pick some course ids you want to delete ;)

I'll test it before submitting otherwise the code seems all good to me.

Jerome,

I've added the pull URLs and testing instructions for your tests

Regards

it's good to me, submitting to integration.

yay, 99% ok:

• get_context_instance() is deprecated since 2.2. context_course::instance() is the new way.
• moodle_exception(get_string(...)) seems incorrect. moodle_exception() itself performs the get_string() for you (yes, I've seen the file is plagued of them, grrr).

Also, I don't know if I returning nulls on success is the best of the ideas, but I've seen the same approach used in other delete operations... so, np.

So, reopening... thanks and ciao

Errors fixed:

Here is the diff with the previous version for a quick review of the changes made:

https://github.com/jleyva/moodle/commit/4222ed6b259f689abe6423000e5260fb8174a94d#course/externallib.php

I resubmitted, thanks Juan.

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

Thanks Juan/Jerome i've integrated this.

I squashed the two commits together because the second one didn't have the bug url in and I think it makes sense as an atomic commit.

Note I noticed that trying to delete the site course silently fails. I think that the solution here is to make delete_course throw an exception in this case since this is an exceptional circumstance.

Created MDL-32696

Attempting to add function to external service produce "Not Found" error. This is due to a typo for $PAGE->set_url().

Raj has created an issue with patch to fix this. MDL-32732

Test failed.

The fix to the URL bug has been integrated, please restart testing

Agree with Dan to create an exception when attempting to delete site course.

Other than that, this looks great.

Test passed.

UPDATE tracker_issues
   SET status = 'Closed',
      comment = 'Thanks!'
WHEN participants = 'Did a gorgeous work'

This landed upstream some hours ago (some - me - developer fell slept in the sofa yesterday before spamming this).