Define TrackedCommand for use as base class for tracking management commands

[brianhw]

Use in create_user command instead of inheriting from BaseCommand.

@rocha @mulby This seems like the least intrusive way to get command information into context.

[rocha]

Can you add some documentation about the edx.mgmt.command context? And example of the generated context would also be useful.

[rocha]

Following django conventions, I would spec command.py to be located in common/djangoapps/track/management/

[brianhw]

In looking at the doc (e.g. https://docs.djangoproject.com/en/dev/howto/custom-management-commands/), I find nothing about where to define code to be used when defining management commands elsewhere. I believe that only management commands should be put in management/commands (and this is not a management command). I haven't seen any example about adding code to management. This might be a reasonable place, but it's not based on any precedence.

[rocha]

Note that I am not saying it to put it in managent/commands, just in management. I have seen that pattern in other django projects. In addition, BaseCommand lives in django.core.management.base.

[brianhw]

Okay, I'll do that, and I'll move the test to a tests subdirectory of management.

[mulby]

I would leave these two entries (course_id, org_id) out until they are actually being added by the create_user command

[brianhw]

They are added by the enrollment call itself. Only "command" is now being added by the TrackedCommand class.

[mulby]

Ah yes, I suppose they would be. Awesome.

[rocha]

Close the PR accidentally, sorry.

[rocha]

:LGTM: One thing we have to watch out is events that assume the context contains the course_id. If that is the case then we can provide a mechanism for users of TrackedCommand to set the course_id and related fields in the context.

However, maybe events that semantically require a course_id should put it in the event data and not the context.

[brianhw]

Those events that require a course_id can put it in their context themselves as well as in their event data. At least that's what the enrollment code does. (See CourseEnrollment.emit_event() for particulars.)

[rocha]

My concern is that the enrollment event is currently the only one (from what I remember) that emitted while setting the context at the same time. The other events assume the context is set up for them. I think we should only deal with this problem if we find an example. Right now it is just hypothetical.

[rocha]

🚀