Skip to content

Fix mutlipe bugs when integrating with Open edX - #42

Merged
OmarIthawi merged 7 commits into
masterfrom
omar/bug-fixes
Mar 18, 2020
Merged

Fix mutlipe bugs when integrating with Open edX#42
OmarIthawi merged 7 commits into
masterfrom
omar/bug-fixes

Conversation

@OmarIthawi

@OmarIthawi OmarIthawi commented Mar 16, 2020

Copy link
Copy Markdown
Contributor

Overview

I installed the course access groups app into devstack with the steps below and found a couple of bugs. I fixed those bugs and tested the CAG app and saw it working properly after the fix.

Steps to Install CAG

Set FEATURES["ORGANIZATIONS_APP"] = true in both lms.env.json and cms.env.json.
Set "ENABLE_COURSE_ACCESS_GROUPS": true in Site Configuration.

$ cd ~/work/tahoe-hawthorn/src/
$ git clone git@github.com:appsembler/course-access-groups.git cag
$ cd ../devstack
$ make COMMAND='pip install -e /edx/src/cag -e /edx/src/search' tahoe.exec.edxapp
$ make lms-restart studio-restart
$ python manage.py lms --settings=devstack_docker migrate

You should be able to control the CAG model from within: http://localhost:18000/admin/course_access_groups/

Good luck fiddling with it.

List of Bug Fixes

  • bug fix: MembershipRule queries was not working:

    After account activation, the automatic Membership objects were not created.

  • bug fix: cag admin cannot add GroupCourse:

    Because Django admin by default cannot deal with CourseKey.

  • fix bug: edx-organizaitons app is a requirement for CAG:

    Disabling ORGANIZATIONS_APP in either lms.env.json or cms.env.json makes CAG goes awry.

  • Refactor acl_backends.user_has_access to use AccessResponse:

    My bad. I assumed that has_access returns native True/False but it turns out it returns an AccessResponse() object.

  • fix query bug: don't check Membership for AnonymousUser:

    Otherwise 500 errors would happen when using the CAG feature with AnonymousUser.

  • Simplify check for public courses for AnonymousUser:

    Otherwise 500 errors would happen when using the CAG feature with AnonymousUser.

  • Fix exception when platform passes CourseDescriptorWithMixins to user_has_access:

    So it works with both course like objects: CourseOverview (mysql) and CourseDescriptorWithMixins (mongo).

Related PRs

Also see the related platform PR: appsembler/edx-platform#543

@OmarIthawi OmarIthawi changed the title Fix misc bugs when integrating with Open edX Fix mutlipe bugs when integrating with Open edX Mar 16, 2020
@OmarIthawi OmarIthawi changed the title Fix mutlipe bugs when integrating with Open edX (Fix mutlipe bugs when integrating with Open edX Mar 16, 2020
@OmarIthawi OmarIthawi changed the title (Fix mutlipe bugs when integrating with Open edX (WIP) Fix mutlipe bugs when integrating with Open edX Mar 16, 2020
@OmarIthawi OmarIthawi changed the title (WIP) Fix mutlipe bugs when integrating with Open edX Fix mutlipe bugs when integrating with Open edX Mar 17, 2020
@OmarIthawi
OmarIthawi marked this pull request as ready for review March 17, 2020 11:59
…_has_access

So it works with both course like objects: CourseOverview (mysql) and CourseDescriptorWithMixins (mongo).
Otherwise 500 errors would happen in the `/courses` page when using the CAG feature with AnonymousUser.
Otherwise 500 errors would happen when using the CAG feature with AnonymousUser.
My bad. I assumed that `has_access` returns native `True`/`False`
but it turns out it returns an `AccessResponse()` object.
Because disabling `ORGANIZATIONS_APP` in either `lms.env.json`
or `cms.env.json` would makes CAG non-functional.
Because Django admin by default cannot deal with CourseKey.
After account activation, the automatic Membership objects were not created.
@OmarIthawi
OmarIthawi merged commit d887222 into master Mar 18, 2020
@OmarIthawi
OmarIthawi deleted the omar/bug-fixes branch March 18, 2020 08:33
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.

2 participants