Hide course versions #26678
Hide course versions #26678
Conversation
| @@ -69,7 +69,8 @@ export default class CourseOverview extends Component { | |||
| hasVerifiedResources: PropTypes.bool.isRequired, | |||
| versions: PropTypes.arrayOf(PropTypes.shape({ | |||
| name: PropTypes.string.isRequired, | |||
| version_year: PropTypes.string.isRequired | |||
| version_year: PropTypes.string.isRequired, | |||
| can_view_version: PropTypes.bool.isRequired, | |||
There was a problem hiding this comment.
while you are in here, please add version_title as a required part of this shape, because it is depended on below.
| @@ -74,6 +74,7 @@ class ScriptOverviewHeader extends Component { | |||
| name: PropTypes.string.isRequired, | |||
| version_year: PropTypes.string.isRequired, | |||
| version_title: PropTypes.string.isRequired, | |||
| can_view_version: PropTypes.bool.isRequired, | |||
There was a problem hiding this comment.
I am going to suggest that you extract a scriptVersionShape, shared between here and ScriptOverview. A scriptVersionShape would contain all the information needed to display a row in the version dropdown on the script overview page. This will help make it clear that this PropType is distinct from the similar-but-different assignmentVersionShape.
I'm also going to recommend that you do not (yet) share this shape with the CourseOverview. This is because course versions and script versions are generated in different places (see two summarize_versions methods), and we do want to create a hard dependency between these implementations.
It may sound like I'm going off the deep end here re: not over-deduplicating. I blame these articles:
There was a problem hiding this comment.
i don't think you're going off the deep end! this is a good thing to DRY up
| @@ -143,9 +143,10 @@ describe('CourseOverview', () => { | |||
| expect(utils.navigateToHref).to.have.been.calledOnce; | |||
| }); | |||
|
|
|||
| it('does not appear when only one version is present', () => { | |||
| it('does not appear when only one version is viewable', () => { | |||
There was a problem hiding this comment.
Thank you for keeping these test case names up to date! 💯
Hide course/script versions in version dropdown on course/script overview pages if
can_view_version?(user)is false.