Ok, I'll address some of the code/style-related issues you've raised and
add in the docstrings, then we can keep going.  Thinking more about this
specific operation, the use case that immediately comes to mind is the
check to see if a relationship already exists -- I'll add that to the
docstring.



On Thu, May 8, 2014 at 11:37 AM, David Ormsbee <notifications@github.com>wrote:

> In lms/djangoapps/api_manager/courses/views.py:
>
> > +        try:
> > +            course_descriptor = get_course(course_id)
> > +        except ValueError:
> > +            return Response({}, status=status.HTTP_404_NOT_FOUND)
> > +        store = modulestore()
> > +        try:
> > +            existing_module = store.get_instance(course_id, Location(module_id))
> > +        except InvalidLocationError:
> > +            return Response({}, status=status.HTTP_404_NOT_FOUND)
> > +        groups = ModuleGroupRelationship.objects.filter(course_id=course_id, module_id=module_id, group=group_id)
> > +        if len(groups):
> > +            response_data = {
> > +                'course_id': course_id,
> > +                'module_id': module_id,
> > +                'group_id': group_id,
> > +            }
>
> I don't really know what it should return because I don't know what the
> use case is for it being called. Actually, since you're going to be writing
> docstrings for the different API calls, maybe I'll just wait for those to
> get updated before continuing my review?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/edx-solutions/edx-platform/pull/42/files#r12435294>
> .
>



-- 
Matthew Drayer
Solutions Architect
edX | MIT
mattdrayer@edx.org