Skip to content

Rewrite Reference and ReferenceList fields on import - #2543

Merged
cahrens merged 1 commit into
masterfrom
christina/reference_bug
Feb 12, 2014
Merged

Rewrite Reference and ReferenceList fields on import#2543
cahrens merged 1 commit into
masterfrom
christina/reference_bug

Conversation

@cahrens

@cahrens cahrens commented Feb 10, 2014

Copy link
Copy Markdown

Fixes STUD-149.

@dmitchell @singingwolfboy Please review. This branch is based off of #2503, so the first commit can be ignored (assuming you reviewed the other one!).

@cahrens

cahrens commented Feb 10, 2014

Copy link
Copy Markdown
Author

Diffcover is lying-- the "Reference" code is covered by test_rewrite_reference (verified with breakpoint in debugger).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The spacing makes this hard to read. I'd format this more like:

if (original_location.tag == ref.tag and 
        original_location.org == ref.org and
        original_location.course == ref.course):
    new_ref = ref.replace(
        tag=target_location_namespace.tag,
        org=target_location_namespace.org,
        course=target_location_namespace.course
    ).url()

or

def locations_equal(a, b):
    return (a.tag == b.tag and a.org == b.org and a.course == b.course)

if locations_equal(original_location, ref):
    new_ref = ref.replace(
        tag=target_location_namespace.tag,
        org=target_location_namespace.org,
        course=target_location_namespace.course
    ).url()

@singingwolfboy

Copy link
Copy Markdown
Contributor

Again, only nitpicks, so 👍. As for diffcover, you should probably file a bug report with @wedaly.

@dmitchell

Copy link
Copy Markdown
Contributor

👍

cahrens pushed a commit that referenced this pull request Feb 12, 2014
Rewrite Reference and ReferenceList fields on import
@cahrens
cahrens merged commit b0b8257 into master Feb 12, 2014
@cahrens
cahrens deleted the christina/reference_bug branch February 12, 2014 21:07
@cahrens

cahrens commented Feb 12, 2014

Copy link
Copy Markdown
Author

Filed https://edx-wiki.atlassian.net/browse/TE-346 for diffcover bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants