Skip to content

AA-335: Avoid due dates for ORA subsections - #24987

Merged
mikix merged 1 commit into
masterfrom
mikix/subsection-ora-cutoff
Sep 17, 2020
Merged

AA-335: Avoid due dates for ORA subsections#24987
mikix merged 1 commit into
masterfrom
mikix/subsection-ora-cutoff

Conversation

@mikix

@mikix mikix commented Sep 15, 2020

Copy link
Copy Markdown
Contributor

Previously, we'd been avoiding PLS due dates for ORA sections. That is, if a section had only ORA content, we'd not set a PLS due date for anything in that section.

If any content in that section had non-ORA graded content however, we would set dates on all subsections, including the ORA one.

This resulted in some ORA-only subsections showing up twice on the dates tab. So this patch simply brings down the ORA-only check to a subsection level, not a section one.

Once deployed, we'll want to republish any affected courses.

@mikix

mikix commented Sep 15, 2020

Copy link
Copy Markdown
Contributor Author

@cpennington does this PR make sense, or is there a use case where we do actually need to check this at the section level?

@Dillon-Dumesnil Dillon-Dumesnil left a comment

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.

LGTM. Just a small question about positioning

same identifier used in setup.py.
"""
return "content_type_gate"
return 'open_assessment_transformer'

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.

copy pasta error?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I assume so

'required': True
}]
valid_assessments = block_data.get_xblock_field(descendent, 'valid_assessments')
print(valid_assessments)

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.

All of the 👏


if section_date_items and section.graded:
date_items.append((section.location, weeks_to_complete))
date_items.extend(section_date_items)

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.

Why have this outside the if statement? Either the section_date_items are [] and there is nothing to add. Or they are not [], but if the section isn't graded, this would still add the subsection dates and I'm not sure we want that. Just pondering out loud here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just thought it looked cleaner this way. But I can be convinced it is too implicit or is confusing.

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.

My only concern would be for the case where section_date_items != [] and section.graded = False and we add subsection dates without adding in the section due date. I'm not sure this scenario can even happen, but might be good to be defensive about it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think that can happen (I believe graded is bubbled up the tree). But also, that is the behavior of the existing code. So it wouldn't be a regression at least.

Previously, we'd been avoiding PLS due dates for ORA *sections*.
That is, if a section had only ORA content, we'd not set a PLS
due date for anything in that section.

If any content in that section had non-ORA graded content however,
we would set dates on all subsections, including the ORA one.

This resulted in some ORA-only subsections showing up twice on the
dates tab. So this patch simply brings down the ORA-only check
to a *subsection* level, not a section one.
@mikix
mikix force-pushed the mikix/subsection-ora-cutoff branch from a2f1230 to 0c9efb7 Compare September 17, 2020 14:10
@mikix

mikix commented Sep 17, 2020

Copy link
Copy Markdown
Contributor Author

jenkins run python

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests. There were no failures.

@mikix
mikix merged commit 8e1ea3e into master Sep 17, 2020
@mikix
mikix deleted the mikix/subsection-ora-cutoff branch September 17, 2020 14:57
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

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.

5 participants