Skip to content

fix: section.due datetime version issue#30848

Merged
leangseu-edx merged 1 commit intomasterfrom
leangseu-edx/section-due-date-fix
Aug 11, 2022
Merged

fix: section.due datetime version issue#30848
leangseu-edx merged 1 commit intomasterfrom
leangseu-edx/section-due-date-fix

Conversation

@leangseu-edx
Copy link
Contributor

Ticket: AU-782

Description

There seems to be issue with datetime version compatibilities especially if the datetime data being stored in Cache. It is a wack a mole situation at the moment.

Reference:

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this is mostly copy/pasted from https://github.com/openedx/edx-platform/blob/3492bede446e4523ee09f965b7639b55af0472d0/openedx/core/djangoapps/content/block_structure/block_structure.py#L453-L483, is there a reason to redefine this instead of importing or using that function?

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like this is moving that part for reuse in block_structure and here, but maybe it could just use get_xblock_field directly as you say

Copy link
Contributor Author

@leangseu-edx leangseu-edx Aug 11, 2022

Choose a reason for hiding this comment

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

How do I re-use it? self.due = subsection.get_xblock_field('due')?

Copy link
Contributor

Choose a reason for hiding this comment

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

Doing a quick repo search shows some examples. If I had to guess:

self.get_xblock_field(usage_key, "due")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

neither self and subsection was a subclass of BlockStructureBlockData.

Copy link
Contributor

Choose a reason for hiding this comment

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

After a discussion, decided that these are slightly different use-cases so it's okay to have a duplication, but should comment so it's clear that if one function changes both will probably have to change.

@leangseu-edx leangseu-edx force-pushed the leangseu-edx/section-due-date-fix branch from c446410 to 0155a0a Compare August 11, 2022 15:17
@jansenk jansenk self-requested a review August 11, 2022 16:00
@leangseu-edx leangseu-edx force-pushed the leangseu-edx/section-due-date-fix branch from 0155a0a to 86fdfbd Compare August 11, 2022 16:31
@leangseu-edx leangseu-edx force-pushed the leangseu-edx/section-due-date-fix branch from 86fdfbd to e25dfe7 Compare August 11, 2022 17:17
Copy link
Contributor

@ashultz0 ashultz0 left a comment

Choose a reason for hiding this comment

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

not really authoritative for Aurora tickets but this is the sort of fix that I was expecting and you've discussed the details so 👍

@leangseu-edx leangseu-edx merged commit 719ddff into master Aug 11, 2022
@leangseu-edx leangseu-edx deleted the leangseu-edx/section-due-date-fix branch August 11, 2022 17:53
@edx-pipeline-bot
Copy link
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
Contributor

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

@edx-pipeline-bot
Copy link
Contributor

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

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.

4 participants

Comments