Skip to content

Conversation

@sefk
Copy link
Contributor

@sefk sefk commented Dec 1, 2013

For @rocha since this is adds onto your work (?). @jinpa if you care to comment on usefulness and such, please do.

This adds two options to the existing management command for dumping out a course structure, "csv" and "flat". The "flat" option is another format for listing out a course that we found helpful for ad-hoc reporting. It has convenient module_id to description mappings, and has the level so you have some notion of where

This is the output of this command:
./manage.py lms --settings=cms.dev dump_course_structure --flat Medicine/SciWrite/Fall2013 --csv | head

in my dev environment:

course_id,position,level,module_id,type,displayname,path,parent
Medicine/SciWrite/Fall2013,0,0,Medicine/SciWrite/Fall2013,course,Writing in the Sciences,,
Medicine/SciWrite/Fall2013,1,1,i4x://Medicine/SciWrite/chapter/bfb45ea5331b4fa8946e4a4c2f60add1,chapter,Getting Started,Getting Started,Medicine/SciWrite/Fall2013
Medicine/SciWrite/Fall2013,2,2,i4x://Medicine/SciWrite/sequential/eb0ff3535f98436993e0504bccc0704d,sequential,Pre-course assessment,"Getting Started,Pre-course assessment",i4x://Medicine/SciWrite/chapter/bfb45ea5331b4fa8946e4a4c2f60add1
Medicine/SciWrite/Fall2013,3,3,i4x://Medicine/SciWrite/vertical/174d22b53c8e4bda9cfefe4a11342ff2,vertical,Starting point assessment,"Getting Started,Pre-course assessment,Starting point assessment",i4x://Medicine/SciWrite/sequential/eb0ff3535f98436993e0504bccc0704d
Medicine/SciWrite/Fall2013,4,4,i4x://Medicine/SciWrite/html/aff2a837288e4ca482d1a1ea6c781f85,html,Text,"Getting Started,Pre-course assessment,Starting point assessment,Text",i4x://Medicine/SciWrite/vertical/174d22b53c8e4bda9cfefe4a11342ff2
Medicine/SciWrite/Fall2013,5,4,i4x://Medicine/SciWrite/html/0a027e1ef69c4a2fbd60281300e00e69,html,Starting point assessment,"Getting Started,Pre-course assessment,Starting point assessment,Starting point assessment",i4x://Medicine/SciWrite/vertical/174d22b53c8e4bda9cfefe4a11342ff2
Medicine/SciWrite/Fall2013,6,2,i4x://Medicine/SciWrite/sequential/9b3ac8e639fc46888e9ef3603765c636,sequential,Pre-course survey,"Getting Started,Pre-course survey",i4x://Medicine/SciWrite/chapter/bfb45ea5331b4fa8946e4a4c2f60add1
Medicine/SciWrite/Fall2013,7,3,i4x://Medicine/SciWrite/vertical/c5aacc093d124126991d9bf41f76fe36,vertical,Survey,"Getting Started,Pre-course survey,Survey",i4x://Medicine/SciWrite/sequential/9b3ac8e639fc46888e9ef3603765c636
Medicine/SciWrite/Fall2013,8,4,i4x://Medicine/SciWrite/html/589be58d3353441a8bf50e657c7aee8b,html,Survey,"Getting Started,Pre-course survey,Survey,Survey",i4x://Medicine/SciWrite/vertical/c5aacc093d124126991d9bf41f76fe36

The "position" is so you can sort reports in the order that they would be presented in the course.

@ghost ghost assigned rocha Dec 1, 2013
@jinpa
Copy link
Contributor

jinpa commented Dec 2, 2013

This will be useful for reporting stats to teaching staff - they don't love getting data organized by module_id - having an easy way to lookup the display name for each module_id, as well as the order in the course, will be handy for putting reports together.

@chrisndodge
Copy link
Contributor

@sefk I can't remember, does @rocha's original implementation have test which assert on the resultant output?

@rocha
Copy link
Contributor

rocha commented Dec 4, 2013

@chrisndodge tests are at lms/djangoapps/courseware/tests/test_commands.py

@sefk we should add tests for the new functionality too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be convenient to add an example of the CSV output too.

@sefk
Copy link
Contributor Author

sefk commented Dec 4, 2013

You're both right on tests and docs, I'll do that, sorry about that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll prefer to use the fieldnames parameter of the cvs writer, with the name of the fields in a global. Each entry in destination will then be a dictionary with the fields like so:

destination.append({
  'course_id': course_id,
  'position': pos,
  'level': level,
  ...
})

That way we don't have to keep the to fields and the entries in sync, and the --flat output in json works too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't know about that option, indeed that sounds nicer. I'll try that thanks.

@rocha
Copy link
Contributor

rocha commented Dec 4, 2013

@sefk I added some comments.

⚠️ The PR also needs a test.

@singingwolfboy
Copy link
Contributor

What's the status of this pull request? Is it actively being worked on, or can it be closed?

@rocha
Copy link
Contributor

rocha commented Dec 31, 2013

@sefk ?

@brianhw
Copy link
Contributor

brianhw commented Dec 31, 2013

Note that https://github.com/edx/edx-platform/pull/2008 makes modifications to the same management command, and includes moving the tests to a more approved location (i.e. in management/commands/tests).

@sefk
Copy link
Contributor Author

sefk commented Jan 1, 2014

I'll add tests so this can land, it slipped my mind. Thanks for following up.

@brianhw
Copy link
Contributor

brianhw commented Jan 3, 2014

@sefk I've merged #2008, so the location of the tests have moved. Hopefully you can just write your tests in the new location.

@sarina
Copy link
Contributor

sarina commented Feb 13, 2014

Hi - just checking on the status of this PR as it hasn't seen any action in a month.

@sefk
Copy link
Contributor Author

sefk commented Feb 13, 2014

Sorry, it's not good to leave a PR languishing like this. I'm going to close for now and will reopen another when I've rebased and written a test. It's not hard, just keeps getting bumped down my stack.

Thanks for following up @sarina and @singingwolfboy

@sefk sefk closed this Feb 13, 2014
@benpatterson benpatterson deleted the sef/course_structure_for_reporting branch January 21, 2015 13:13
@sefk sefk unassigned rocha Feb 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants