Skip to content

Using display_name_with_default instead of display_name. - #11550

Merged
symbolist merged 1 commit into
masterfrom
muzaffar/tnl4013
Feb 19, 2016
Merged

Using display_name_with_default instead of display_name.#11550
symbolist merged 1 commit into
masterfrom
muzaffar/tnl4013

Conversation

@muzaffaryousaf

Copy link
Copy Markdown

TNL-4103

@muzaffaryousaf

Copy link
Copy Markdown
Author

@symbolist @muhammad-ammar Quick look ?

@muhammad-ammar

Copy link
Copy Markdown
Contributor

@muzaffaryousaf lgtm 👍

@symbolist

Copy link
Copy Markdown
Contributor

@muzaffaryousaf I see two other usages of reading display_name from a block on line 27 and 182. Can we change them too?

@symbolist

Copy link
Copy Markdown
Contributor

In models.py.

@muzaffaryousaf

Copy link
Copy Markdown
Author

@symbolist we don't need default name at line 27 but i've made the change for other place.

"""
Test that the xblocks data is persisted correctly with display_name=None.
"""
ItemFactory.create(parent_location=self.sequential_2.location, category='vertical', display_name=None)

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.

@muzaffaryousaf We should check there is a valid XBlockCache item for this XBlock.

@muzaffaryousaf

Copy link
Copy Markdown
Author

@symbolist I've addressed your feedback. PTAL. Thanks.

@symbolist

Copy link
Copy Markdown
Contributor

👍 once tests pass.

@symbolist

Copy link
Copy Markdown
Contributor

@muzaffaryousaf Tests have passed. I am merging so that we can get it into todays RC.

symbolist added a commit that referenced this pull request Feb 19, 2016
Using display_name_with_default  instead of display_name.
@symbolist
symbolist merged commit 4be5aeb into master Feb 19, 2016
@symbolist
symbolist deleted the muzaffar/tnl4013 branch February 19, 2016 12:51
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