feat: [FC-0056] Add calculation for percentage of the completions by … #34816
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @NiedielnitsevIvan! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
| if navigation_sidebar_caching_is_disabled := courseware_disable_navigation_sidebar_blocks_caching(): | ||
| cached = False | ||
| course_blocks = None | ||
| else: | ||
| course_blocks = cache.get(cache_key) | ||
| cached = course_blocks is not None | ||
| else: | ||
| course_blocks = None |
There was a problem hiding this comment.
Is this conditional reversed? It looks like we're doing the cache.get if navigation_sidebar_caching_is_disabled? It also looks like navigation_sidebar_caching_is_disabled is never referenced later?
There was a problem hiding this comment.
Oh, thanks for noticing, it was most likely due to a cherry-pick from our repository. Fixed it.
| if block_type in ['course', 'sequential']: | ||
| completion = sum(child['complete'] for child in completable_children) | ||
| elif block_type == 'chapter': | ||
| # For sections, we have to count the status on the number of completed units | ||
| # and, accordingly, the number of units in it. | ||
| completion = sum(child['completion_stat']['completion'] for child in completable_children) | ||
| completable_children_num = sum( | ||
| child['completion_stat']['completable_children'] for child in completable_children | ||
| ) | ||
| elif block_type == 'vertical': | ||
| completion = sum(child['completion_stat']['completion'] for child in completable_children) |
There was a problem hiding this comment.
I think this whole piece needs better comments as to why we're treating these types differently. What is complete vs. completion_stat:completion? Why are course and sequential treated the same way, but chapter is so different?
There was a problem hiding this comment.
I've updated the docsting for this method.
completion(float from 0 to 1 meaning the percentage of completion) and complete(bool value of completion) are the names that are used by default in the BlockCompletionTransformer.
The chapter has a different calculating, due to a request from the product team, you can read more here:
https://openedx.slack.com/archives/C06A9ES6QUU/p1714586420150249?thread_ts=1714058363.124469&cid=C06A9ES6QUU
| elif block_type == 'vertical': | ||
| completion = sum(child['completion_stat']['completion'] for child in completable_children) | ||
| else: | ||
| completion = self.completions_dict.get(block['id'], 0) |
There was a problem hiding this comment.
Does this handle the case where there are other container blocks inside vertical? For instance, split_test nests multiple verticals inside of it.
There was a problem hiding this comment.
Yes, this works provided that the user has received a completion for all available blocks in the "split_test" block.
That is, if the user belongs to group 1 and has only 2 blocks available in the "split_test", then completing both will be counted as a completed. But if the user is staff and has access to all blocks for all groups, then the completion will be counted only when he passes ALL the blocks in the "split_test".
|
@ormsbee thanks for approving, could you please merge this PR (and squash commits), so we can open a backport PR to Redwood? |
|
Right, sorry, I keep forgetting who has and doesn't have merge rights on this repo. I put in a notice in #cc-edx-platform and I'll merge this in about half an hour. |
And the number of people is growing each year, which is cool😝 Thank you |
|
@NiedielnitsevIvan 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
…4816) This is an enhancement to the API used for the courseware navigation sidebar.
This is an enhancement to the API used for the courseware navigation sidebar.
Description
This PR draws attention to the need to add the calculation of user progress to the API for navigating the sidebar that returns the course structure.
It is worth noting that the progress for sections is calculated by the state of the units' improvements in this section, and not directly by their subsections
Testing instructions
completion_statfield) on the compliments for the user.