Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed subsection gating on course outline to consider rounded off g… #16837

Conversation

ammarnaqvi
Copy link
Contributor

…rade instead of absolute grade to decide whether to show a subsection which has a prerequisite or not.

@openedx-webhooks
Copy link

Thanks for the pull request, @ammarnaqvi! I've created OSPR-2041 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

  • supporting documentation
  • edx-code email threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here.

We can't start reviewing your pull request until you've submitted a signed contributor agreement or indicated your institutional affiliation. If you like, you can add yourself to the AUTHORS file for this repo, though that isn't required. Please see the CONTRIBUTING file for more information.

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Dec 8, 2017
@@ -61,7 +61,7 @@ def _get_subsection_percentage(subsection_grade):
"""
Returns the percentage value of the given subsection_grade.
"""
return _calculate_ratio(subsection_grade.graded_total.earned, subsection_grade.graded_total.possible) * 100.0
return int(round(_calculate_ratio(subsection_grade.graded_total.earned, subsection_grade.graded_total.possible) * 100.0))
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we keeping in mind that we need to make sure that rounding is consistent on all 3 views?
Secondly, this PR needs tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to ensure consistent rounding across all 3 views.

@nedbat
Copy link
Contributor

nedbat commented Dec 11, 2017

@ammarnaqvi, thanks! Let me know when you've submitted your contributor agreement so that we can start the review.

@nedbat
Copy link
Contributor

nedbat commented Dec 11, 2017

jenkins ok to test

@openedx-webhooks openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs triage labels Dec 11, 2017
@unpack
def test_min_score_achieved(self, module_score, result, mock_score):
self._setup_gating_milestone(50)
mock_score.return_value = module_score
mock_score.return_value = int("{0:.0f}".format(module_score))
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats going on here is you are mocking _get_subsection_percentage which means the change will you made in _get_subsection_percentage will not execute and mocked value will be returned.

ad Outdated
+
@attr(shard=3)
@ddt
class TestEvaluatePrerequisite(GatingTestCase, MilestonesTestCaseMixin):
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this file?

self.subsection_grade = Mock(graded_total=Mock(earned=mock_earned, possible=mock_possible))
result = _get_subsection_percentage(self.subsection_grade)

self.assertEqual(60, result)
Copy link
Contributor

Choose a reason for hiding this comment

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

need a new line here.

@data((59.50, 100), (60.50, 100))
@unpack
def test_subsection_percentage_rounding(self, mock_earned, mock_possible):
self.subsection_grade = Mock(graded_total=Mock(earned=mock_earned, possible=mock_possible))
Copy link
Contributor

Choose a reason for hiding this comment

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

this test needs a docstring.

@data((59.50, 100), (60.50, 100))
@unpack
def test_subsection_percentage_rounding(self, mock_earned, mock_possible):
self.subsection_grade = Mock(graded_total=Mock(earned=mock_earned, possible=mock_possible))
Copy link
Contributor

Choose a reason for hiding this comment

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

do you want to update self.subsection_grade or you can use the local var here?

@@ -111,3 +111,14 @@ def test_no_gated_content(self, mock_score):

evaluate_prerequisite(self.course, self.subsection_grade, self.user)
self.assertFalse(mock_score.called)

@data((59.50, 100, 60), (22.25, 50, 44))
Copy link
Contributor

Choose a reason for hiding this comment

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

(22.25, 50, 44) 22.25 to 44?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

22.25 is 44.5 percent of 50, hence 44.

@awaisdar001
Copy link
Contributor

this looks good to me, can you please squash the commits?

@ammarnaqvi ammarnaqvi force-pushed the EDUCATOR-1676-progress-page-and-prerequisite-rounding branch from 51a82d4 to e720d0b Compare December 13, 2017 15:20
@nedbat
Copy link
Contributor

nedbat commented Dec 14, 2017

@ammarnaqvi I now have you recorded as a contributor under the Arbisoft contribution agreement. In the future, please respond to comments on pull requests so the process can proceed smoothly.

@@ -61,7 +61,7 @@ def _get_subsection_percentage(subsection_grade):
"""
Returns the percentage value of the given subsection_grade.
"""
return _calculate_ratio(subsection_grade.graded_total.earned, subsection_grade.graded_total.possible) * 100.0
return int("{0:.0f}".format(_calculate_ratio(subsection_grade.graded_total.earned, subsection_grade.graded_total.possible) * 100.0))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the .format here? We shouldn't need to convert a float to a string just to convert it to an integer. Are we rounding? Or flooring? There are functions for those operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For exact halfway cases (e.g. 2.50); "{0:.0f}".format() rounds the number to the nearest even number, and python 2.7's round() rounds the number to the next whole number. "{0:.0f}".format() would round 2.50 to 2 and 1.50 to 2. On the other hand round() would round 2.50 to 3 and 1.50 to 2.

In the two other places, subsection percentage is displayed, "{0:.0%}".format() is doing the rounding, for sake of consistency, we use the same method of rounding. And since we need an int instead of a string, we have to convert.

Copy link
Contributor

Choose a reason for hiding this comment

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

You say there are other places where "{0:.0%}" is used, but I can't find them. If there are multiple places that need specific rounding, we should definitely write and use a tested helper function to get it right.

I'm also confused why we want 2.5 to round to 2, but 3.5 to round to 4? I understand the need for that in some domains, but why for grading? Is that what people will expect?

Copy link
Contributor Author

@ammarnaqvi ammarnaqvi Dec 15, 2017

Choose a reason for hiding this comment

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

https:/edx/edx-platform/blob/master/lms/templates/courseware/progress.html#L161
this code is used in progress page chapters section
and the following is used in graph
https:/edx/edx-platform/blob/master/lms/templates/courseware/progress_graph.js#L292
both use format for rounding. As the format rounding value is different from round() method rounding. To make it consistent with other values we need to use the same method for all 3.

We can use either method of rouding, we just have to make sure same method of rounding is used in all 3 places (progress graph, chapter section below the graph and in subsection gating).
Generally people expect rounding to go to next integer (2.5 to 3) because that's what is taught in schools, and is more generous towards the student while rounding to even number wouldn't be as generous to the student, sometimes it would favor the student and sometimes it would not.

Copy link
Contributor

Choose a reason for hiding this comment

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

@edx/educator-devs Please look into this rounding issue, and address it in a definitive way that will prevent confusion among future developers. I suspect that the original author of "{0:.0%}".format() didn't use it because of the even-up odd-down rounding behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nasthagiri It is not actually a bug, since python 3.x round() method follows the same method for rounding.
https://docs.python.org/3/library/functions.html#round
https://docs.python.org/2/library/functions.html#round

Copy link
Contributor Author

@ammarnaqvi ammarnaqvi Dec 15, 2017

Choose a reason for hiding this comment

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

I agree, using NonZeroSubsection.percent_graded seems like a better idea. UI code can be updated accordingly as well. But NonZeroSubsection.percent_graded method isn't implemented till @bill-filler 's PR is merged. I could copy that into this PR, if Bill's PR will take some time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, copying that into this PR sounds good in order to unblock you. @bill-filler has offered to resolve any merge conflicts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I suggest making sure Product (@sstack22) is aware that we are changing the code to Round to the Nearest Even, per https://en.wikipedia.org/wiki/IEEE_754#Rounding_rules.

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you thought through if there are any backward compatibility issues with this change?

@nedbat
Copy link
Contributor

nedbat commented Dec 14, 2017

@edx/educator-devs Does this need a product review? If not please take a look.

@openedx-webhooks openedx-webhooks added awaiting prioritization and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Dec 14, 2017
@nasthagiri
Copy link
Contributor

Please note that this change will conflict with this pending PR: https:/edx/edx-platform/pull/16876

I also suggest having @bill-filler review this PR since he's actively working on this. Thanks.

@nedbat nedbat removed awaiting prioritization open-source-contribution PR author is not from Axim or 2U labels Dec 14, 2017
@awaisdar001
Copy link
Contributor

@nasthagiri:
Just a quick question: As mentioned by Ammar, there are two other places where "{}".format rounding is used, do you think we should keep those as it is or they should also need to use math.round for consistency?

@nasthagiri
Copy link
Contributor

@awaisdar001 per the latest thread, we're planning to update the code in a single place (within subsection_grades.py) and have all other places rely on that single implementation. The working assumption is that we'll go with Rounding-to-the-nearest-Even, pending Product's approval.

We can't use round as @ammarnaqvi points out that its behavior has changed from Python-2 to Python-3 and we're currently leaning towards the Python-3 implementation.

@sstack22
Copy link

@awaisdar001 - what's the product question on this one?

@awaisdar001
Copy link
Contributor

@sstack22 Its the FYI from @nasthagiri. Let me quote here.

nasthagiri: Also, I suggest making sure Product (@sstack22) is aware that we are changing the code to Round to the Nearest Even, per https://en.wikipedia.org/wiki/IEEE_754#Rounding_rules.

@ammarnaqvi
Copy link
Contributor Author

@nasthagiri As per the latest commit, the change meets the acceptance criteria defined on the ticket. Just confirm from @awaisdar001 that criteria was defined with mutual understanding with Product.
So can you please review my PR again?

@@ -61,13 +61,13 @@ def _get_subsection_percentage(subsection_grade):
"""
Returns the percentage value of the given subsection_grade.
"""
return _calculate_ratio(subsection_grade.graded_total.earned, subsection_grade.graded_total.possible) * 100.0
return subsection_grade.percent_graded * 100.0


def _calculate_ratio(earned, possible):
Copy link
Contributor

Choose a reason for hiding this comment

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

May I please suggest the following in order to close out all loose ends on this issue? Remember that we would like to move all rounding logic of grades into a single place.

  1. Create a new function called compute_percent, which takes earned and possible and returns the rounded value. Put that function in https:/edx/edx-platform/blob/master/lms/djangoapps/grades/scores.py
  2. Change score_for_chapter to be chapter_percentage. And update it to return the rounded percentage by calling compute_percent.
  3. Update get_entrance_exam_score_ratio in this file to use the updated chapter_percentage function.
  4. You can now eliminate _calculate_ratio.
  5. Update the front-end code (in progress.html and progress_graph.js) to use the public functions in the Grades layer to get rounded percentage values - rather than duplicating that logic.

Whether these changes are made within this PR or in a subsequent PR, it's up to the team. However, in order for the ticket to be considered complete, the code needs to be refactored as such for better future maintenance. Thanks.

Copy link
Contributor Author

@ammarnaqvi ammarnaqvi Dec 26, 2017

Choose a reason for hiding this comment

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

@nasthagiri
The changes you suggested have been made in subsequent commits to this PR.
Can you please review the changes?

@ammarnaqvi ammarnaqvi force-pushed the EDUCATOR-1676-progress-page-and-prerequisite-rounding branch from aab3187 to dc64759 Compare December 26, 2017 13:01
@ammarnaqvi
Copy link
Contributor Author

jenkins run bokchoy

1 similar comment
@noraiz-anwar
Copy link
Contributor

jenkins run bokchoy

Changed subsection gating on course outline to consider rounded off grade instead of absolute grade to decide whether to show a subsection which has a prerequisite or not.
EDUCATOR-1676
@ammarnaqvi ammarnaqvi force-pushed the EDUCATOR-1676-progress-page-and-prerequisite-rounding branch from 39099b5 to fde830b Compare January 8, 2018 10:30
@Rabia23 Rabia23 merged commit 4fa0a38 into openedx:master Jan 8, 2018
@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 on Tuesday, January 09, 2018.

@edx-pipeline-bot
Copy link
Contributor

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

Agrendalath added a commit to open-craft/edx-platform that referenced this pull request Nov 8, 2021
Enabling the rounding in openedx#16837 has been causing noticeable (up to 1 percentage point) differences between non-rounded subsection grades and a total grade for a course. This increases the grade precision to reduce the negative implications of double rounding.
Agrendalath added a commit to open-craft/edx-platform that referenced this pull request May 26, 2024
Enabling the rounding in openedx#16837 has been causing noticeable (up to 1 percentage
point) differences between non-rounded subsection grades and a total grade for
a course. This increases the grade precision to reduce the negative
implications of double rounding.
Agrendalath added a commit to open-craft/edx-platform that referenced this pull request Sep 23, 2024
Enabling the rounding in openedx#16837 has been causing noticeable (up to 1 percentage
point) differences between non-rounded subsection grades and a total grade for
a course. This increases the grade precision to reduce the negative
implications of double rounding.
Agrendalath added a commit that referenced this pull request Sep 24, 2024
Enabling the rounding in #16837 has been causing noticeable (up to 1 percentage
point) differences between non-rounded subsection grades and a total grade for
a course. This increases the grade precision to reduce the negative
implications of double rounding.
Agrendalath added a commit to open-craft/edx-platform that referenced this pull request Sep 24, 2024
Enabling the rounding in openedx#16837 has been causing noticeable (up to 1 percentage
point) differences between non-rounded subsection grades and a total grade for
a course. This increases the grade precision to reduce the negative
implications of double rounding.

(cherry picked from commit 84d2ad9)
Agrendalath added a commit to open-craft/edx-platform that referenced this pull request Sep 24, 2024
Enabling the rounding in openedx#16837 has been causing noticeable (up to 1 percentage
point) differences between non-rounded subsection grades and a total grade for
a course. This increases the grade precision to reduce the negative
implications of double rounding.

(cherry picked from commit 84d2ad9)
Agrendalath added a commit to open-craft/edx-platform that referenced this pull request Sep 25, 2024
Enabling the rounding in openedx#16837 has been causing noticeable (up to 1 percentage
point) differences between non-rounded subsection grades and a total grade for
a course. This increases the grade precision to reduce the negative
implications of double rounding.

(cherry picked from commit 84d2ad9)
Agrendalath added a commit to open-craft/edx-platform that referenced this pull request Sep 25, 2024
Enabling the rounding in openedx#16837 has been causing noticeable (up to 1 percentage
point) differences between non-rounded subsection grades and a total grade for
a course. This increases the grade precision to reduce the negative
implications of double rounding.

(cherry picked from commit 84d2ad9)
Faraz32123 pushed a commit to edly-io/edx-platform that referenced this pull request Oct 18, 2024
- directly call python native APIs from forum v2 for pin, unpin thread,
commentables count_stats and get user's data by user_id
- add forum to the edx-platform requirements

feat: migrate some APIs to native python
- directly call python native APIs from forum v2 for get parent comment,
create parent comment and create child comment.
- rename retrieve_commentables_stats method to get_commentables_stats and
retrieve_user to get_user.

feat: pass params to python native APIs
- refactored code and now pass proper parameters to python native APIs
instead of a single dict

feat: code refactor and migrate delete comment API

fix: user tests

feat: fix tests for get_user API
- get_user API tests are now passing in test_views.py and test_serializers.py
- add get_user api patch in all tests
- fix httppretty request count in some tests
- fix test_patch_read_non_owner_user test

feat: use new coursewaffle flag to run old code
- add `ENABLE_FORUM_V2` course waffle flag to switch between old code i.e.
cs_comment_service and new code i.e. forum v2.
- mock course waffle flag is_enabled method i.e. ENABLE_FORUM_V2.is_enabled(),
so that old unit tests can be run and passed.
- refactor code(that parts of code whose native APIs are implemented till now)
where we call the native APIs

feat: Upgrade Python dependency edx-enterprise

adds logging to debug SAP transmitter

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`

docs: upstream block ADR, take 2 (openedx#35421)

feat: return publishing information on get component endpoint [FC-0062] (openedx#35476)

* feat: return publishing information on get component endpoint

* feat: read data from component.versioning.draft

* test: update tests

* chore: update openedx-learning

---------

Co-authored-by: Jillian <[email protected]>
Co-authored-by: Chris Chávez <[email protected]>

feat: Add collection tags to index [FC-0062] (openedx#35483)

* feat: Add collection tags to index

* feat: Add api functions to update tags in collections

* feat: Update tags on index when tag_object

feat: add idv events to api (openedx#35468)

* feat: add idv events to api

- moved what was in signals.py to a handlers.py (which is what their file should have been called)

* chore: quality

* fix: rename test file + imports

* fix: change handler reverse url in other tests

* fix: refactor signals and handlers pattern

- following OEP-49 pattern for signals directory
- user removed as param for update function
- event now emitted after save

* fix: unpin edx-name-affirmation

* chore: add init to signals dir

* fix: compile requirements

* chore: quality

* chore: fix some imports

* chore: quality

* test: added signal emissions to test_api

* chore: lint

feat: upgrading list_instructor_tasks to DRF ( 10th ) (openedx#35332)

* feat: upgrading simple api to drf compatible.

feat: When editing a v2 library xblock, update search index synchronously (openedx#35495)

feat: added ORA graded notification (openedx#35389)

* feat: added ORA graded by staff notification

* test: updated and added new unit tests

* feat: added waffle flag and updated notification

chore: update ora2 version in requirements (openedx#35505)

feat: use idv approved event (openedx#35470)

* feat: replace LEARNER_NOW_VERIFIED signal with new openedx-event

fix: Adds a check to initialize legacy proctoring dashboard

chore: Fixed a typo on a comment

feat: add course_run_key to learner home upgrade url (openedx#35461)

* fix: fix learner home URL to have course_run_key

feat: upgrading simple api to drf compatible ( 17th ) (openedx#35394)

* feat: upgrading simple api to drf compatible.

feat: Be able to login to bare-metal studio easily. (openedx#35172)

* feat: Be able to login to bare-metal studio easily.

Updating the documentation and the devstack.py files so that if you're
running bare-metal you can easily setup studio login via the LMS.

I also added the Ports that the various MFEs expect to the runserver
scripts so that it's easier to run those locally as well.

Co-authored-by: Kyle McCormick <[email protected]>

feat: [FC-0047] add settings for edx-ace push notifications

feat: [FC-0047] Add push notifications for user enroll

feat: [FC-0047] Add push notifications for user unenroll

feat: [FC-0047] Add push notifications for add course beta testers

feat: [FC-0047] Add push notifications for remove course beta testers

feat: [FC-0047] Add push notification event to discussions

refactor: [FC-0047] rename subject files to title

docs: [FC-0047] add docs for setting up mobile push notifications

chore: [FC-0047] upgrade requirements

style: [FC-0047] add module docstrings

refactor: [FC-0047] fix review issues

refactor: change name of the policy to course push optout

build: remove diff for requirements

fix: rename email to message params

fix: fix linter warning

fix: remove docs from app level

style: [FC-0047] fix code style issues

chore: [FC-0047] change push notifications texts

style: [FC-0047] remove unnecessary pass

feat: add block_id field to filterable attributes of meilisearch (openedx#35493)

feat: added sender in bulk_email event (openedx#35504)

feat: replaced button and heading tags in email digest content (openedx#35518)

chore: Aperture code ownership update

chore: update default notification preference for ora_grade_assigned (openedx#35522)

* chore: update default notification preference for ora_grade_assigned

* test: updated tests

fix: updated edx.ace.message_sent event (openedx#35498)

* fix: updated edx.ace.message_sent event

* fix: fixed pylint checks

fix: fix broken proctoring settings link

fix: increase grades rounding precision

Enabling the rounding in openedx#16837 has been causing noticeable (up to 1 percentage
point) differences between non-rounded subsection grades and a total grade for
a course. This increases the grade precision to reduce the negative
implications of double rounding.

chore: add frontend-app-learner-portal-enteprise localhost to trusted origin (openedx#35529)

feat: add verification attempt django admin

feat: added country disabling feature (openedx#35451)

* feat: added country disabling feature

feat: upgrade get_student_enrollment_status api with drf (22nd) (openedx#35464)

* feat!: upgrading simple api with DRF.

fix: don't wrap HTML data with newlines when serializing for LC (openedx#35532)

When serializing to OLX, the Learning Core runtime wraps HTML content in
CDATA to avoid having to escape every individual `<`, `>`, and `&`. The
runtime also puts newlines around the content within the CDATA,
So, given HTML content `...`, we get `<![CDATA[\n...\n]]>`.

The problem is that every time you serialize an HTML block to OLX, it
adds another pair of newlines. These newlines aren't visible to the end
users, but they do make it so that importing and exporting content never
reached a stable, aka "canonical" form. It also makes unit testing
difficult, because the value of `html_block.data` becomes a moving
target.

We do not believe these newlines are necessary, so we have removed them
from the `CDATA` block, and added a unit test to ensure that HTML blocks
having a canonical serialization.

Closes: openedx#35525

fix(deps): update dependency underscore to v1.13.0 [security]

feat: Upgrade Python dependency edx-enterprise (openedx#35538)

Bump the version to drop references to edx-rest-api-client that don't exist in the latest version.

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`

Co-authored-by: feanil <[email protected]>

fix(deps): update dependency underscore to v1.13.1 [security]

Soft delete collections (openedx#35496)

* refactor: use django signals to trigger LIBRARY_COLLECTION events

* refactor: use collection usage_key as search document id

This change standardises the search document "id" to be a meilisearch ID
generated from the usage key, for all types of indexed objects.

This is important for collections so we can locate the collection
document in the search index solely from the data provided by the
LIBRARY_COLLECTION_DELETED event (library_key + collection_key), even if
the collection has been deleted from the database.

* refactor: avoid fetching more data than we have to.

* get_library_collection_usage_key and
  searchable_doc_tags_for_collection do not need a Collection object;
  the usage key can be created from the library_key and collection_key.

* updated searchable_doc_for_collection to require the parts of the
  collection usage key + an optional collection. This allows us to
  identify the collection's search document from its usage key without
  requiring an existing Collection object (in case it's been deleted).
  Also removes the edge case for indexing Collections not associated
  with a ContentLibrary -- this won't ever really happen.

* feat: remove soft- and hard-deleted collections from search index

* feat: adds library_component_usage_key to content_libraries.api

* refactor: send CONTENT_OBJECT_ASSOCIATON_CHANGED on django model signals

so that added/removed collections are removed/re-added to component documents.

Special case: When a collection is soft-deleted/restored, we detect this
in the search index and update the collection's component documents
directly, without a CONTENT_OBJECT_ASSOCIATON_CHANGED signal.

* chore: bumps openedx-learning to 0.13.0

feat: return modified field on get component endpoint (openedx#35508)

feat: pluggable url for idv location (openedx#35494)

* Adds an extension point when generating the url for id verification

chore: version bump

fix: Delete flaky test `test_get_user_group_id_for_partition` (openedx#35545)

This test failed on 2024-08-06 and 2024-09-24 but passed on re-run.

Deleted according to flaky test process:
https://openedx.atlassian.net/wiki/spaces/AC/pages/4306337795/Flaky+Test+Process

Flaky test ticket: https://2u-internal.atlassian.net/browse/CR-7071

feat: set links for CourseAuthoring discussion alert

fix: whitespace issues in some capa problem index_dictionary content (openedx#35543)

fix: suppress errors+warnings when video is used in a content library (openedx#35544)

build: Manually pull some RTD Context.

See https://about.readthedocs.com/blog/2024/07/addons-by-default/ for details but
essentially RTD is changing how it's building docs and this will let us handle the
change gracefully.

chore: bumps openedx-learning to 0.13.1 (openedx#35547)

chore: Upgrade Python requirements

test: Use the correct exception.

This test doesn't actually care about the type of the exception but use
the Requests exception that you're likely to get instead of the
edx-restapi-client/slumber one from before we dropped them.

test: Update a test based on changes to pytz.

pytz dropped the Asia/Almaty timezone according to IANA

stub42/pytz@640c9bd#diff-16061815f611262054e469307ca063a4ef47e158a97784f1e91d254f074324bfR72

chore: version bump

fix: bump version

chore: version bump

build: Switch to ubuntu-latest for builds

This code does not have any dependencies that are specific to any specific
version of ubuntu.  So instead of testing on a specific version and then needing
to do work to keep the versions up-to-date, we switch to the ubuntu-latest
target which should be sufficient for testing purposes.

This work is being done as a part of openedx/platform-roadmap#377

closes openedx#35314

build: Run mongosh commands within the container.

This is no longer installed by default on ubuntu and so we have to
either manually install it or just run the relevant commands in the
container here it's already available. This lets us do some of the test
setup in a more robust way.

fix: Don't start the mongo service.

We stopped using mongo on the runner directly a while ago so this is
just an errant start that should have been removed.

build!: enable md4 for testing.

Operators Note: In newer versions of ubuntu the MD4 hashing algorithm
is disabled by default.  To enable it the openssl config needs to be
updated in a manner similar to what's being done here.  Alternatively,
you can set the `FEATURES['ENABLE_BLAKE2B_HASHING']` setting to `True`
which will switch to a newer hashing algorithm where MD4 was previously
used.

Because this hashing is being used as a part of the edx-platform caching
mechanism, this will effectively clear the cache for the items that use
this hash. The will impact any items where the cache key might have been
too big to store in memcache so it's hard to predict exactly which items
will be impacted.

BREAKING CHANGE: See the operator note above for more details as this
may break for users transitioning from Ubuntu 20.04 to newer versions.

fix: add should_display_status_to_user method and status_changed field to VerificationAttempt model (openedx#35514)

* fix: add placeholder should_display_status_to_user

* fix: have VerificationAttempt inherit StatusModel

- should_display_status_to_user now returns False

* chore: makemigrations

* feat: status_changed field added

* temp: idea to add should_display_status_to_user

* feat: add should_display_status_to_user

* fix: correct call in helpers+services

* chore: lint+test fix

* fix: default hide_status_user as False

* chore: rename field call to STATUS

* chore: remove extra status field

- comment cleanup

* temp: lint + comment out created status for now

* fix: revamp status_changed for back-compat

* fix: override save for status_changed

* fix: replace created/updated instead of status

- also made migrations

* fix: squash commits

- also remove extra updated_at property

* chore: nits

fix: resolve inconsistant python dependencies
- run `make compile-requirements forum`. Error: It appears that the
Python dependencies in this PR are inconsistent: A re-run of
`make compile-requirements` produced changes.
- fix quality checks

fix: fix new failing tests
- fix new tests related to discussion that failed after fixing previous tests
these are failing due to no.of argument difference
https:/openedx/edx-platform/actions/runs/11069160532/job/30756121710?pr=35490

feat: migrate user active_thread api

feat: migrate fetch subscription

feat: add python native APIs from forum v2
- migrate following native APIs i.e. mark_thread_as_read, create_subscription,
delete_subscription, update_thread_votes, update_comment_votes, delete_thread_vote,
delete_comment_vote, get_user_threads, retire_user, update_username,
get_user_course_stats, update_users_in_course, flag/unflag comment/threads
from forum v2 to edx-platform
- refactor some code

feat: update thread apis

feat: add threads api

fix: quality checks

feat: migrate update_user api

feat: retrieve course_id

fix: course_id issue for course_waffle flag

fix: temp

chore: code refactor, fix quality checks

feat: add tests for native APIs
in this commit:
- migrated the tests for native APIs from test_serializers to test_serializers_native_views file.
- tests from test_views to test_views_native_views is still in progress
- fixed edx-platform tests which were failing after we added get course_id APIs
- fixed course_key error for user's retrieve APIs

feat: add group_id actions tests

feat: new native APIs tests progress

feat: Upgrade Python dependency edx-enterprise

Removed loggings for SAPSF channel

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`

chore: delete unused tags_count from LibraryXBlockMetadata (openedx#35510)

chore: revert learner pathway skeleton implementation (openedx#30355)

fix: added ace_enabled in bulk_email logs (openedx#35558)

feat: deprecate get_verification_details_by_id (openedx#35560)

fix: connect verify_student handlers to app (openedx#35561)

refactor: Lift methods out of StaticContentServer (unchanged and broken)

Remove class declaration and simply de-indent contents by 4 spaces.

This results in broken code due to lingering `self` references, but
should make diffs easier to understand.

refactor: Fix content server methods and tests (finish method lift)

Some lingering cleanup from the transition from a middleware to a view.
See openedx#34702 for context.

- Remove IMPL and self/StaticContentServer references
- Add newlines to satisfy code style linter
- Fix test references
- Update module docstring

style: Fix some lint issues that had been amnestied

Several now-irrelevant lint-disables and one legit one that was easy to
fix.

feat: Upgrade Python dependency edx-enterprise

version bump

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`

fix: handle missing name in readonly enterprise account fields

The `settings.ENTERPRISE_READONLY_ACCOUNT_FIELDS` list can be overridden.
Removing the `name` from it should not raise the `ValueError` exception.

feat!: upgrading simple api with DRF. (openedx#35463)

fix: gate use of proctoring api if provider is LTI based (openedx#35564)

docs: Fix links to extensions docs.

The extensions docs did not get linked from the index previously. Add
them to the docs tree to make them easier to find.

fix: remove deprecated runtime.render_template

fix: allow courses to render with invalid proctoring provider (openedx#35573)

fix: receive XBlock visibility from the Learning MFE (openedx#35491)

feat: update ora settings course app for clarity (openedx#35550)

feat: Upgrade Python dependency edx-django-utils (openedx#35575)

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`

Co-authored-by: dianakhuang <[email protected]>

feat: added notification grouping (openedx#35368)

Co-authored-by: Ahtisham Shahid <[email protected]>

feat!: upgrading mark_student_can_skip_entrance_exam api with DRF ( 21 ) (openedx#35460)

* feat!: upgrading simple api with DRF.

feat: history tracking on SAMLProviderConfig model

chore: geoip2: update maxmind geolite country database

docs: Update the docs to build on the latest python version.

feat: set bulk email transactional to True (openedx#35579)

feat!: removes deprecated v1 certificate behavior (openedx#35562)

* feat!: removes deprecated v1 certificate behavior

this removes the long-deprecated v1  certificate behavior. This removes
the old-style date selection behavior  (ie., not a choice between
*Immediately upon passing*, *End date of course*, *A date after the course
end date*), which is no longer reliably maintained or supported in
Studio or Credentials.

FIXES: openedx#35399

feat: Upgrade Python dependency edx-enterprise

edx-enterprise 4.26.1 | proxy login redirects to LMS register page

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`

feat: override refund

fix: _auto_enroll is private

fix: remove 'toggle_status: unsupported' from COURSES_INVITE_ONLY (openedx#35572)

feat: Upgrade Python dependency edx-proctoring (openedx#35589)

4.18.2 fixes bugs caused by a removed proctoring provider

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`

Co-authored-by: alangsto <[email protected]>

feat!: upgrading api to DRF. (openedx#35536)

feat: rework idv cert trigger (openedx#35580)

* feat: rework idv cert trigger
* feat: separate PhotoVerification events

upgrade get_students_features api with DRF( 7th api ) (openedx#35323)

* feat: upgrading simple api to drf compatible.

chore: Update edx-enterprise to 4.27.0 ENT-9585

docs: Add README for third_party_auth. (openedx#35608)

chore: calc version bump (openedx#35590)

chore: Remove edge reference in notification

fix: VerificationAttempt.expiration_datetime field may be None.

This commit fixes a bug introduced by the new VerificationAttempt model. The expiration_datetime field is nullable. Other IDV models in the verify_student application have a default value for expiration_date, so they are typically not null, despite being nullable.

This commit updates code that queries this field to function correctly when the value of expiration_datetime is None. In this case, a None expiration_datetime indicates a non-expiring attempt.

feat: Upgrade Python dependency edx-name-affirmation (openedx#35616)

3.0.1 contains changes to integrate the name affirmation app with the new platform VerificationAttempt model

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`

Co-authored-by: alangsto <[email protected]>

fix: Improve v2 library block permissions checks for read-only authors (openedx#35598)

chore: removed style from email digest content (openedx#35592)

chore: cleanup constraint file and format it (openedx#35601)

* chore: cleanup constraint file and format it

chore: move native API tests to separate PR

fix: CI checks

chore: change the forum branch to master

fix: older edx-platform tests that uses V1
- Before resolving course_id availability issue for all APIs to be used in
CourseWaffle flag, only 4 files tests were failing, We fixed those.
Now due to latest fixes, some more tests were failing that uses v1, this PR will
fix those issues.

feat: Upgrade Python dependency edx-enterprise

version bump

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`

chore: changing codeowners for several components (openedx#35574)

making sure the codeowners for some maintainership is accurate.

feat: update management command to manually create verified names (openedx#35619)

* feat: update management command to manually create verified names

* fix: update function name

feat: pass course_id to native APIs
course_id is needed to be used in second coursewaffle flag

feat: Use jammy repositories for mongo installation.

Unclear if there was a change in the focal repositories
or if there was an issue with something else. The noble
repositories don't support 7.0, so we're stuck here
until we upgrade to 8.0.

feat: Unpin xmlsec and lxml.

We are now on Python 3.11 and running tests
on the latest version of Ubuntu, which seems to mean
we don't need these pins anymore. In fact, it seems
to break while on these pins.

fix: update functions to named arguments

fix: pylint issues

fix(deps): update dependency @edx/frontend-component-cookie-policy-banner to v2.6.0

fix: tests, add mocks for getting course_id APIs

Revert "feat: Unpin xmlsec and lxml."

This reverts commit 6c045c7.

Revert "feat: Use jammy repositories for mongo installation."

This reverts commit a245dec.

fix: Pin select jobs to ubuntu 22.04.

Using Ubuntu 24.04 breaks Mongo installation
and some thing involving lxml/xmlsec. We are
going to pin this until we're ready to upgrade both.

Revert "fix(deps): update dependency @edx/frontend-component-cookie-policy-banner to v2.6.0"

This reverts commit a39367b.

fix: failing unit tests

refactor: remove unnecessary try catch

feat: Upgrade Python dependency edx-enterprise (openedx#35625)

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`

Co-authored-by: kiram15 <[email protected]>

refactor: get rid of XBlockRuntimeSystem for v2 libraries (openedx#35624)

feat: remove all the commerce coordinator (openedx#35527)

fix: fix track selection (openedx#35648)

feat: waffle flag based switch to ses for goal reminder email (openedx#35615)

* feat: waffle based switch to ses for goal reminder email

* test: added test cases for ace message parameters

* feat: updated logic to specify channel name

* chore: update edx-ace version

refactor: remove PII from log messages in programs tasks (openedx#35623)

* refactor: remove PII from log messages in programs tasks

[APER-3723]

Refactors a number of log statements from the Celery tasks in the Programs Django app. This removes username (considered PII) from the log statements and opts to use LMS User ID instead.

* fix: quality

feat: set collections for a library component [FC-0062]  (openedx#35600)

* feat: add & remove collections to component

Co-authored-by: Rômulo Penido <[email protected]>
Co-authored-by: Chris Chávez <[email protected]>

feat!: Remove outdated Libraries Relaunch cruft (openedx#35644)

The V2 libraries project had a few past iterations which were never
launched. This commit cleans up pieces from those which we don't need
for the real Libraries Relaunch MVP in Sumac:

* Remove ENABLE_LIBRARY_AUTHORING_MICROFRONTEND,
  LIBRARY_AUTHORING_FRONTEND_URL, and
  REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND, all of which are obsolete
  now that library authoring has been merged into
  https:/openedx/frontend-app-authoring.
  More details on the new Content Libraries configuration settings are
  here: openedx/frontend-app-authoring#1334

* Remove dangling support for syncing V2 (learning core-backed) library
  content using the LibraryContentBlock. This code was all based on an
  older understanding of V2 Content Libraries, where the libraries were
  smaller and versioned as a whole rather then versioned by-item.
  Reference to V2 libraries will be done on a per-block basis using
  the upstream/downstream system, described here:
  https:/openedx/edx-platform/blob/master/docs/decisions/0020-upstream-downstream.rst
  It's important that we remove this support now so that OLX course
  authors don't stuble upon it and use it, which would be buggy and
  complicate future migrations.

* Remove the "mode" parameter from LibraryContentBlock. The only
  supported mode was and is "random". We will not be adding any further
  modes. Going forward for V2, we will have an ItemBank block for
  randomizing items (regardless of source), which can be synthesized
  with upstream referenced as described above. Existing
  LibraryContentBlocks will be migrated.

* Finally, some renamings:

  * LibraryContentBlock -> LegacyLibraryContentBlock
  * LibraryToolsService -> LegacyLibraryToolsService
  * LibrarySummary -> LegacyLibrarySummary

  Module names and the old OLX tag (library_content) are unchanged.

Closes: openedx/frontend-app-authoring#1115

feat: adaptive display of links

Do not display the 'Learn more' and 'Share feedback' links for
banner that is enabled by the context_course.discussions_settings
flag if the URLs for these links are not set in the settings.

temp: logs for unsubscribe event (openedx#35652)

feat: removed extra spaces from start and end of content (openedx#35647)

feat: added waffle flag for new notification view (openedx#35569)

feat!: hide courses in /courses based on catalog visibility

Previously, courses were always displayed on the LMS /courses page,
independently of their catalog visibility attribute. This meant that
even with visibility="none" courses were being displayed. This was very
counter-intuitive.

With this change, courses are displayed only when their visibility is
set to "both".

This change is flagged as breaking because it has the potential to
affect course catalogs in existing platforms.

To test this change, go to http(s)://LMS/courses as an anonymous user.
Pick a visible course and go to its "advanced settings" in the studio.
Set "Course visibility in catalog" to "about" or "none". Then, clear the
cache with the following command:

    ./manage.py lms shell -c "from django.core.cache import cache; cache.clear()"

Open the /courses page again: the course should no longer be visible.

Close openedx/wg-build-test-release#330

feat: new view & API calls to serve content library assets (openedx#35639)

This commit adds a new view to serve static assets for content
libraries, along with Content Library API calls to add, delete, and get
metadata about these assets. These assets come from Learning Core and
should ONLY BE ACCESSED FROM STUDIO. Users must have read access to the
library in order to see an asset in that library.

This also re-implements video transcript support for content libraries
and re-enables some previously disabled tests around it.

feat: Allow specifying a version when loading a v2 XBlock (openedx#35626)

chore: delete unused requirements.txt file (openedx#35637)

As far as I know, this file is unused. It was not updated since 2022. If
it is actually being used, then maybe we should use instead a file from
requirements/edx/.

fix: Fix pylint_django_settings plugin (openedx#35497)

chore: Upgrade Python requirements (openedx#35651)

feat: Upgrade Python dependency edx-enterprise

4.28.0 | Introduce default enrollment models

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`

build: Pin MyPY due to internal error

Currently all PRs are failing on an INTERNAL ERROR in MyPY that seems to
have been introduced in 1.12.0. This pins edx-platform to <1.12.0 until
we can find out more information and hopefully get an upstream fix.

build: Propagate MyPY constraint

The MyPY constraint is propagated to development.txt

docs: Add issue comment for unpinning MyPY

Co-authored-by: Muhammad Farhan <[email protected]>

feat: Upstream Sync with Content Library Blocks (openedx#34925)

This introdues the idea of "upstream" and "downstream" content,
where downstreams (like course components) can pull content updates from
upstreams (like learning core-backed content library blocks). This
supports the upcoming Content Libraries Relaunch Beta for Sumac.
New features include:

* A new XBlockMixin: UpstreamSyncMixin.
* A new CMS Python API: cms.lib.xblock.upstream_sync
* A new CMS JSON API: /api/contentstore/v2/downstreams
* A temporary, very basic UI for syncing from Content Library blocks

Implements:
https:/kdmccormick/edx-platform/blob/kdmccormick/upstream-proto/docs/decisions/0020-upstream-block.rst

Co-authored-by: Braden MacDonald <[email protected]>

feat: versioned asset support for Learning Core XBlock runtime

Add support for displaying static assets in the Learing Core XBlock
runtime via "/static/asset-name" style substitutions in the OLX. This is
currently used for new Content Library components.

Static asset display is version-aware, so viewing older versions of the
XBlock content via the embed view will show the appropriate assets for
that version.

test: add data.py to acceptable isolated app imports

Per OEP-49, both api.py and data.py are allowed to be imported into
other apps:

https://open-edx-proposals.readthedocs.io/en/latest/best-practices/oep-0049-django-app-patterns.html#api-py
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.

10 participants