fix: Enable courseware access api for all types of course(expired, cl… #35155

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.

Sign up for GitHub

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

Jump to bottom

Merged

jawad-khan merged 8 commits into master from jawad/LEARNER-10121

Jul 23, 2024

Member

jawad-khan commented Jul 22, 2024 •

edited

Loading

Description

Enable courseware access api for all types of course(expired, closed, not started yet etc)

Useful information to include:

This will effect all mobile app users.

Supporting information

Jira Ticket: https://2u-internal.atlassian.net/browse/LEARNER-10121

Sample response data that this api should return

{
  "id": "course-v1:IB12T21",
  "course_updates": {course-updates-url},
  "course_handouts":{{course-handouts-url}},
  "discussion_url": {discussion-url},
  "course_access_details": {
    "has_unmet_prerequisites": false,
    "is_too_early": false,
    "is_staff": true,
    "audit_access_expires": null,
    "courseware_access": {
      "has_access": true,
      "error_code": null,
      "developer_message": null,
      "user_message": null,
      "additional_context_user_message": null,
      "user_fragment": null
    }
  },
  "certificate": {
    
  },
  "enrollment_details": {
    "mode": "",
    "is_active": false
  },
  "course_info_overview": {
      "name": "Introduction to Data Science",
  "number": "Abc",
  "org": "IBM",
  "start": "2021-07-01T16:00:00Z",
  "start_display": "July 1, 2021",
  "start_type": "timestamp",
  "end": "2023-10-16T16:00:00Z",
  "is_self_paced": true,
  "media": {
    "image": {
      "raw": "/asset-v1+_image.png",
      "small": "/asset-v1.jpg",
      "large": "/a.jpg"
    }
  },
  "course_sharing_utm_parameters": {
    "facebook": "utm_medium=social&utm_campaign=social-sharing-db&utm_source=facebook",
    "twitter": "utm_medium=social&utm_campaign=social-sharing-db&utm_source=twitter"
  },
  "course_about": {course-about-url},
  "course_modes": [
    {
      "slug": "audit",
      "sku": "5479",
      "android_sku": null,
      "ios_sku": null,
      "min_price": 0
    }
  ]
  }
}

Testing instructions

Call api /api/mobile/{api_version}/course_info/{course_id}/enrollment_details
and compare data with expected data given above.

Deadline

"ASAP".

justinhynes reviewed

View reviewed changes

lms/djangoapps/mobile_api/course_info/views.py Outdated Show resolved Hide resolved

justinhynes reviewed

View reviewed changes

lms/djangoapps/mobile_api/course_info/views.py Outdated Show resolved Hide resolved

jawad-khan force-pushed the jawad/LEARNER-10121 branch from af6cf8c to 02416f8 Compare

July 23, 2024 14:19

justinhynes reviewed

View reviewed changes

lms/djangoapps/mobile_api/tests/test_course_info_views.py Outdated

+                  @patch('lms.djangoapps.mobile_api.course_info.utils.certificate_downloadable_status')
+                  def test_course_not_started(self, mock_certificate_downloadable_status):
+                      """ Test course data whic is not started yet """

Contributor

justinhynes Jul 23, 2024

Nit, typo: "Test course data which has not started yet"

justinhynes reviewed

View reviewed changes

lms/djangoapps/mobile_api/tests/test_course_info_views.py Outdated

+                  @patch('lms.djangoapps.mobile_api.course_info.utils.certificate_downloadable_status')
+                  def test_course_closed(self, mock_certificate_downloadable_status):
+                      """ Test course data whic is not started yet """

Contributor

justinhynes Jul 23, 2024

Nit: Is this bad copy-paste? Should the description for this test be different than the above unit test?

justinhynes reviewed

View reviewed changes

lms/djangoapps/mobile_api/tests/test_course_info_views.py Outdated

+                  @patch('lms.djangoapps.mobile_api.course_info.utils.certificate_downloadable_status')
+                  def test_invalid_course_id(self, mock_certificate_downloadable_status):
+                      """ Test course data whic is not started yet """

Contributor

justinhynes Jul 23, 2024

Nit: Bad copy-paste again for test description.

justinhynes reviewed

View reviewed changes

lms/djangoapps/mobile_api/tests/test_course_info_views.py Outdated

+                      response = self.client.get(path=url)
+                      assert response.status_code == 400
+                      assert response.data['error']

Contributor

justinhynes Jul 23, 2024

Nit: Should we validate the contents of the error message are what we expect?

justinhynes reviewed

View reviewed changes

lms/djangoapps/mobile_api/tests/test_course_info_views.py

Contributor

justinhynes Jul 23, 2024

I think we're missing one more unit test, for the case where the course_id is missing from the request, to cover lines 431-432 in ../course_info/views.py

Member Author

jawad-khan Jul 23, 2024

I tried to build this url but because of re pattern we will get page not found. Therefore I have removed that check from view.

justinhynes suggested changes

View reviewed changes

Contributor

justinhynes left a comment

I think we're missing one unit test for the new functionality, please see my comment in test_course_info_views.py.

jawad-khan added 7 commits

July 24, 2024 00:42


          fix: Enable courseware access api for all types of course(expired, cl…

da806a7

…osed etc)


          fix: removed comment

879ba69


          fix: temp change for sandbox job

64f59be


          fix: Revert to previous original scheme

13a9bb7


          fix: Updated errro status code

a5bf9c1


          fix: Added back file

efe78d0


          fix: refactored code

73ef648

jawad-khan force-pushed the jawad/LEARNER-10121 branch from 02416f8 to 73ef648 Compare

July 23, 2024 19:57

Member Author

jawad-khan commented Jul 23, 2024

I think we're missing one unit test for the new functionality, please see my comment in test_course_info_views.py.

After this change that new case isn't required anymore.

justinhynes approved these changes

View reviewed changes

Contributor

justinhynes left a comment

Thanks for addressing my comments!


          Merge branch 'master' into jawad/LEARNER-10121

f4e71f8

jawad-khan merged commit b77b90a into master

jawad-khan deleted the jawad/LEARNER-10121 branch

July 23, 2024 20:40

Contributor

edx-pipeline-bot commented Jul 23, 2024

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

Contributor

edx-pipeline-bot commented Jul 23, 2024

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment

Contributor

edx-pipeline-bot commented Jul 23, 2024

2U Release Notice: This PR has been deployed to the edX production environment.

OmarIthawi mentioned this pull request

Mobile API v4 and other related fixes nelc/edx-platform#33

Closed

25 tasks

mudassir-hafeez pushed a commit to mudassir-hafeez/edx-platform that referenced this pull request


          fix: Enable courseware access api for all types of course(expired, cl… (

a598090

openedx#35155)

* fix: Enable courseware access api for all types of course(expired, closed etc)

mudassir-hafeez pushed a commit to mudassir-hafeez/edx-platform that referenced this pull request


          fix: Enable courseware access api for all types of course(expired, cl… (

cea7bed

openedx#35155)

* fix: Enable courseware access api for all types of course(expired, closed etc)

mudassir-hafeez pushed a commit to mudassir-hafeez/edx-platform that referenced this pull request


          fix: Enable courseware access api for all types of course(expired, cl… (

e8e3032

openedx#35155)

* fix: Enable courseware access api for all types of course(expired, closed etc)

mudassir-hafeez pushed a commit to mudassir-hafeez/edx-platform that referenced this pull request


          fix: Enable courseware access api for all types of course(expired, cl… (

51f7dea

openedx#35155)

* fix: Enable courseware access api for all types of course(expired, closed etc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet