Moodle

LOCAL: course deletion hook

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9.3, 2.0
  • Fix Version/s: 2.0
  • Component/s: Course
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE, MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

I didn't add t his, but it's there. This one is just a docs bug. local_course_delete.

Activity

Hide
Petr Škoda (skodak) added a comment -

maybe we could use events instead - seems much cleaner solution

Show
Petr Škoda (skodak) added a comment - maybe we could use events instead - seems much cleaner solution
Hide
Penny Leach added a comment -

Yep, agree. I will have a look at changing what's currently there to use events instead.

Show
Penny Leach added a comment - Yep, agree. I will have a look at changing what's currently there to use events instead.
Hide
Penny Leach added a comment -

It seems this function is actually called from remove_course_contents, so the name is a little weird.

How do we feel about changing it to an event? This function has been there for a long time (I wonder if the OU people added it, I don't think Catalyst did) - potentially we'd b breaking backwards compatibility to just change it over.

Show
Penny Leach added a comment - It seems this function is actually called from remove_course_contents, so the name is a little weird. How do we feel about changing it to an event? This function has been there for a long time (I wonder if the OU people added it, I don't think Catalyst did) - potentially we'd b breaking backwards compatibility to just change it over.
Hide
Petr Škoda (skodak) added a comment -

hmm, I personally think that now is the right time to refactor/cleanup all APIs - no old code will be working, there are few places properly documented, we have to make a clear distinction what is public x private

Show
Petr Škoda (skodak) added a comment - hmm, I personally think that now is the right time to refactor/cleanup all APIs - no old code will be working, there are few places properly documented, we have to make a clear distinction what is public x private
Hide
Penny Leach added a comment -

Okie, I guess just as long as it's in the release notes it's ok, huh.

Show
Penny Leach added a comment - Okie, I guess just as long as it's in the release notes it's ok, huh.
Hide
Penny Leach added a comment -

Interesting. The notify_local_course_delete function is actually called in remove_course_contents, not in delete_course function.

I propose the following:

1. Throw a new event, course_emptied in remove_course_contents
2. delete the notify_local_course_delete function

delete_course does actually throw a 'course_deleted' event as well.

Show
Penny Leach added a comment - Interesting. The notify_local_course_delete function is actually called in remove_course_contents, not in delete_course function. I propose the following: 1. Throw a new event, course_emptied in remove_course_contents 2. delete the notify_local_course_delete function delete_course does actually throw a 'course_deleted' event as well.
Hide
Petr Škoda (skodak) added a comment -

I think this is ok for HEAD
-1 for 1.9.x

Show
Petr Škoda (skodak) added a comment - I think this is ok for HEAD -1 for 1.9.x
Hide
Petr Škoda (skodak) added a comment -

I have cleaned up the course delete code a bit and added the new event that is triggered after removing course content, thanks

Show
Petr Škoda (skodak) added a comment - I have cleaned up the course delete code a bit and added the new event that is triggered after removing course content, thanks

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: