mitodl / ccxcon

CCXCon API
GNU Affero General Public License v3.0
7 stars 0 forks source link

Create Endpoint fixes for edX to work properly. #74

Closed justinabrahms closed 8 years ago

justinabrahms commented 8 years ago

What's this PR do?

Updates the API to take into account missing information from the edx side of things.

Where should the reviewer start?

courses/views.py

How should this be manually tested?

It should be run against https://github.com/mitocw/edx-platform/pull/153

  1. Download Giovanni's branch.
  2. Edit your vm's /etc/hosts file to point to the ip of your docker file with the hostname localhost.daplie.com.
  3. Add edx user on ccxcon. They should have an edx_instance (at the bottom of the form) that's valid.
  4. Add an OAuth Application object in the CCXCon admin. You'll need the generated id and secret later. It should belong to the edx user you just made.
  5. Create a CCXCon object in edX (using shell) with the URL of https://localhost.daplie.com:8077/ and id and secret tokens generated in the previous step.
  6. Enable CCX for a course.
  7. Put in the above daplie url (with port) in your CCXcon info.
  8. Save settings.
  9. You should see the resulting course in https://localhost.daplie.com:8077/api/v1/coursexs/

To get the IP of your docker image, run this:

docker ps | grep "ccxcon_web_1" | awk '{ print $1 }' | xargs docker inspect | grep 'IPAddress"' | head -n 1 | awk '{print $2}' | tr '"' ' ' | tr "," " "

Any background context you want to provide?

It's difficult to get the base url from edx, so we can't get the proper image path either. edX also doesn't know if its a create or update, so we just handle it all seamlessly.

What are the relevant tickets?

Fixes #70

What gif best describes this PR or how it makes you feel?

noisecapella commented 8 years ago

Took me a while but I was able to step through the manual steps without an issue. I'm not really sure where we're supposed to add our openedx.core.djangoapps.ccxcon app to the edX INSTALLED_APPS list. I just added it to lms/envs/common.py and hoped for the best

justinabrahms commented 8 years ago

Thanks! You don't add it to the installed apps list. It's automatically added if you've setup giovanni's PR and have ccx enabled. I'll address your other comments later today.

https://github.com/mitocw/edx-platform/pull/153/files#diff-8a23909bc8d201fba8525b1dd6396247R677

noisecapella commented 8 years ago

Can you rename migration 0006_... to its old name to avoid Django migration confusion on deploy? I also saw an unimplemented function which seemed out of place

noisecapella commented 8 years ago

I'm getting this error when running migrations:

web_1    |   Apply all migrations: oauth2_provider, sessions, admin, auth, courses, contenttypes, webhooks
web_1    | Running migrations:
web_1    |   Rendering model states... DONE
web_1    |   Applying courses.0007_course_id... OK
web_1    |   Applying courses.0008_course_ids_backfilled... OK
db_1     | ERROR:  could not create unique index "courses_course_course_id_dd69d4ee_uniq"
db_1     | DETAIL:  Key (course_id)=(2015-12-16 14:52:05.278075+00:00) is duplicated.
db_1     | STATEMENT:  ALTER TABLE "courses_course" ADD CONSTRAINT "courses_course_course_id_dd69d4ee_uniq" UNIQUE ("course_id")
web_1    |   Applying courses.0009_nullable_description_overview_author_name__unique_course_id...Traceback (most recent call last):
web_1    |   File "manage.py", line 14, in <module>
web_1    |     execute_from_command_line(sys.argv)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/core/management/__init__.py", line 350, in execute_from_command_line
web_1    |     utility.execute()
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/core/management/__init__.py", line 342, in execute
web_1    |     self.fetch_command(subcommand).run_from_argv(self.argv)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/core/management/base.py", line 348, in run_from_argv
web_1    |     self.execute(*args, **cmd_options)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/core/management/base.py", line 399, in execute
web_1    |     output = self.handle(*args, **options)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/core/management/commands/migrate.py", line 200, in handle
web_1    |     executor.migrate(targets, plan, fake=fake, fake_initial=fake_initial)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/db/migrations/executor.py", line 92, in migrate
web_1    |     self._migrate_all_forwards(plan, full_plan, fake=fake, fake_initial=fake_initial)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/db/migrations/executor.py", line 121, in _migrate_all_forwards
web_1    |     state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/db/migrations/executor.py", line 198, in apply_migration
web_1    |     state = migration.apply(state, schema_editor)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/db/migrations/migration.py", line 123, in apply
web_1    |     operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/db/migrations/operations/fields.py", line 201, in database_forwards
web_1    |     schema_editor.alter_field(from_model, from_field, to_field)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/db/backends/base/schema.py", line 482, in alter_field
web_1    |     old_db_params, new_db_params, strict)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/db/backends/base/schema.py", line 663, in _alter_field
web_1    |     self.execute(self._create_unique_sql(model, [new_field.column]))
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/db/backends/base/schema.py", line 110, in execute
web_1    |     cursor.execute(sql, params)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/db/backends/utils.py", line 79, in execute
web_1    |     return super(CursorDebugWrapper, self).execute(sql, params)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/db/backends/utils.py", line 64, in execute
web_1    |     return self.cursor.execute(sql, params)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/db/utils.py", line 95, in __exit__
web_1    |     six.reraise(dj_exc_type, dj_exc_value, traceback)
web_1    |   File "/usr/local/lib/python2.7/dist-packages/django/db/backends/utils.py", line 64, in execute
web_1    |     return self.cursor.execute(sql, params)
web_1    | django.db.utils.IntegrityError: could not create unique index "courses_course_course_id_dd69d4ee_uniq"
web_1    | DETAIL:  Key (course_id)=(2015-12-16 14:52:05.278075+00:00) is duplicated.
justinabrahms commented 8 years ago

Pushed the updated. You'll probably need to fake to 7, roll back down to 6 (not faked) to get back to a pre-PR state. Then this should work correctly. See this session to prove it works.

screenshot from 2015-12-17 14-50-55

noisecapella commented 8 years ago

Migrations work fine now :+1: