EnrollmentTrackUserPartition to be used for differentiated content by cahrens · Pull Request #14731 · openedx/openedx-platform

cahrens · 2017-03-20T18:48:31Z

TNL-6674

Description

Introduce EnrollmentTrackUserPartition for all courses.

Sandbox

https://cahrens.sandbox.edx.org/courses/course-v1:edX+DemoX+Demo_Course/courseware/d8a6192ade314473a78242dfeedfbf5b/4ed9eb74deee4d418d9047db0a326353/

On that unit, you can preview as "verified" to see content for verified learners, and "audit" to see content for audit learners. Note that this sandbox includes a hacky UX change (edx@6eaef6f) to allow setting of visibility of components by enrollment track. The complete implementation of that will be done in a future story, and the temporary commit is not part of this PR.

Testing

Unit, integration, acceptance tests as appropriate
Analytics, no impact
Performance, hopefully no impact! Not sure how to check this off though.

Reviewers

If you've been tagged for review, please check your corresponding box once you've given the 👍.

Code review: @nasthagiri, whole PR
Code review: @staubina, second code reviewer

FYI: @staubina @jlajoie

Post-review

Rebase and squash commits

cahrens · 2017-03-20T19:29:43Z

            }
        ]
-        self.assertEqual(self._get_partition_info(), expected)
+        self.assertEqual(self._get_partition_info(schemes=["cohort", "random"]), expected)


Note to reviewers: exclude the enrollment_track partition.

cahrens · 2017-03-20T19:42:42Z

-            course2_item = self.store.get_item(course2_item_loc)
-
+                course2_item = self.store.get_item(course2_item_loc)
+                # The EnrollmentTrackuserPartition stores the course_id, which will vary split vs. old mongo.


Note to reviewers: cloning an old mongo course into the new modulestore caused me some grief because the course_id at the beginning of the operation is the old ID, and then it changes at some point to the new (split) ID. I updated the course_module code to handle that, and then unit tests calling this method started to fail because the "old" course was not equal to the "new" course.

cahrens · 2017-03-20T19:43:21Z

    for user_partition in user_partitions:
        for group in user_partition['groups']:
-            if str(group['id']) in xblock_display_name:
+            # Temporary hack until TNL-6731 is addressed.


Note to reviewers: this issue will be fixed before this PR merges. FYI @mushtaqak

#14748 is merged :)

cahrens · 2017-03-20T19:43:51Z

        self.course.user_partitions = [
            UserPartition(
-                id=0,
+                id=100,


Note to reviewers: Changing IDs so they more accurately represent what the code does (and avoid non-unique IDs).

cahrens · 2017-03-20T19:44:43Z

            'skipped': 2
        }
-        with self.assertNumQueries(168):
+        with self.assertNumQueries(184):


Note to reviewers: there are 8 different learners (increase of 2 queries per learner).

Side note: Yikes. No wonder certificate generation is slow. We need to optimize this via bulk queries.

cahrens · 2017-03-20T19:46:36Z

        model = CourseModeExpirationConfig

+
+class EnrollmentModeAdmin(admin.ModelAdmin):


Added the admin table just for easy viewing of the options (useful to make sure the migration ran correctly). Here it is (user staff has access): https://cahrens.sandbox.edx.org/admin/course_modes/enrollmentmode/

cahrens · 2017-03-20T19:47:03Z

+    pass

+if not COURSE_MODE_SLUG_CHOICES:
+    # The entries in this table are populated in a migration file, and migration files do not


This is sad-- I was hoping to delete the hardcoded COURSE_MODE_SLUG_CHOICES, but can't because of unit tests.

cahrens · 2017-03-20T19:48:06Z

                  ])


+class EnrollmentMode(models.Model):


@bderusha what do you think of this model (and admin stuff above)? Do you like the name? Can you think of any way to link mode_slug in this model to mode_slug in CourseMode below?

cahrens · 2017-03-20T19:50:03Z

            with check_mongo_calls(mongo_calls):
                self.verify_response()

+    def get_success_enrolled_staff_mongo_count(self):


See lms/djangoapps/lti_provider/tests/test_views.py below. I didn't get all the way to the root cause here, but given that it is old mongo only (and lti only), I didn't think it was worth spending time on it.

cahrens · 2017-03-20T19:51:20Z

        # Check that a new user partition was created for the ICRV block
-        self.assertEqual(len(self.course.user_partitions), 1)
-        partition = self.course.user_partitions[0]
+        partitions = _get_non_enrollment_track_partitions(self.course)


Note to reviewers: the VerificationUserPartition will be going away soon, but in the meantime I have to keep tests passing!

cahrens · 2017-03-20T19:52:28Z


-from verified_track_content.forms import VerifiedTrackCourseForm
-from verified_track_content.models import VerifiedTrackCohortedCourse
+from openedx.core.djangoapps.verified_track_content.forms import VerifiedTrackCourseForm


Move of verified_track_content from LMS to openedx because of CMS unit test dependencies.

nasthagiri

This looks great overall.
My main concern is around persisting the course_id in the partition's parameters. If you can avoid that, it will also simplify the logic around changing course-ids.

nasthagiri · 2017-03-21T02:25:41Z

+                # The EnrollmentTrackuserPartition stores the course_id, which will vary split vs. old mongo.
+                # Also at least one test calls this comparison method with a purposefully different course.
+                # Clear stored course_id so that own_metadata call below does not fail.
+                course1_item.user_partitions[0].parameters['course_id'] = None


Why is the course_id persisted in the course content? Generally speaking, no block within a course should store the containing course's id. One of the key benefits of SplitModulestore is that block-content is decoupled from the structure - so you can have multiple pointers to the same block-content. That also allows us to easily implement course-cloning, for example.

Rather, can you have the course_id be passed to the partition whenever it's needed - at runtime? And don't have it persisted within the partition's data.

The problem is that we need the course_id to return the groups from the user partition (and groups is a property). I don't see how we can pass the course_id unless I change groups completely in a method.

UserPartitions (after initial construction) are constructed via the from_json method. We don't have the course_id available then either.

Yes. Exactly. UserPartitions don't need to always know the course_id context. They can be provided the context by the caller when needed.

course_id is no longer persisted. We don't call to_json on the partition anymore, but just to be clear, I'm also explicitly not persisting the course_id in the to_json method.

nasthagiri · 2017-03-21T02:27:55Z


+if not COURSE_MODE_SLUG_CHOICES:
+    # The entries in this table are populated in a migration file, and migration files do not
+    # run as part of unit tests. Therefore, for tests hard-code the choices.


Since this code is specifically for tests, I'd prefer it be in test code only. Can you move this hard-coded list out of production code and into lms/envs/test.py?

I was chatting with @ormsbee, and there are general problems with me trying to access a database at the time that the file is loaded (with future versions of django). Plus edx-platform would have to be restarted for any new EnrollmentMode instances to be recognized (both here and in verified_track_content/partition_scheme.py).

I could try to make both of these usages lazy, but then I still have to figure out how to populate them in all unit tests which need them (a much bigger deal for verified_track_content/partition_scheme.py than for the admin table!).

@bderusha sorry, but I think I am going to abandon the database table approach and go with a setting that has the slug/ID mapping. :(

nasthagiri · 2017-03-21T02:28:58Z


+
+class CourseModeForm(forms.ModelForm):
+    """ CourseModeForm """


Nit: I understand this is a pre-existing class, but a more useful docstring would be helpful for future readers. :)

nasthagiri · 2017-03-21T02:30:54Z

+    class Meta(object):
+        model = EnrollmentMode
+
+    def has_add_permission(self, request):


What about has_change_permission?

I deleted the EnrollmentMode model and associated admin view.

nasthagiri · 2017-03-21T02:32:49Z

+        ),
+        migrations.RunPython(
+            code=create_modes,
+            reverse_code=delete_modes


Nit: Can you update the names of the helper functions so they specifically say enrollment_modes? To distinguish from course_modes?

nasthagiri · 2017-03-21T03:05:57Z

+except:  # pylint: disable=bare-except
+    pass
+
+if not ENROLLMENT_GROUP_IDS:


Once again, can we move this to lms/env/test.py instead, so it's not part of production code?

nasthagiri · 2017-03-21T03:13:54Z

+            # The user is masquerading as a generic student. We can't show any particular group.
+            return None
+
+        if is_course_using_cohort_instead(course_key):


Shouldn't this check be done first (before the masquerading logic)?

nasthagiri · 2017-03-21T03:19:59Z

+            return None
+        mode_slug, is_active = CourseEnrollment.enrollment_mode_for_user(user, course_key)
+        if mode_slug and is_active:
+            course_mode = CourseMode.mode_for_course(course_key, mode_slug)


This may not work for old courses (as tagged on the bulk-email PR). The implementation of mode_for_course calls modes_for_course with the limiting default values for include_expired and only_selectable.

Good catch! I'll have to figure out what to do here.

OK, passing in all modes.

nasthagiri · 2017-03-21T03:23:34Z

+            group = Group(ENROLLMENT_GROUP_IDS[mode.slug], unicode(mode.name))
+            all_groups.append(group)
+
+        return all_groups


Nitty: Consider using a list comprehension syntax for readability, consolidating the above 6 lines of code.

return [ Group(ENROLLMENT_GROUP_IDS[mode.slug], unicode(mode.name)) for mode in CourseMode.modes_for_course(course_key, include_expired=True, only_selectable=False) ]

Yikes, that seems a lot less readable to me!

nasthagiri · 2017-03-21T03:25:00Z

+        'Serialize' to a json-serializable representation.
+
+        Returns:
+            a dictionary with keys for the properties of the partition, with an empty array for


Nitty: capitalize sentence.

cahrens · 2017-03-21T16:16:12Z

@cpennington @ormsbee What are your thoughts about persisting a change to course_module via the init method? @nasthagiri and I have been talking about it, and we are not sure if there is a better solution.

The background here is that we are introducing a new UserPartition type that is based on enrollment track (so we can ultimately get rid of the cohorting/enrollment track beta feature and instead use a UserPartition that dynamically gets the learner's enrollment mode). We want this partition to be available for all courses.

With the code as-is, if the course_module is loaded from LMS and no enrollment track partition exists, we will create one. This means that the "published" version of the course will end up persisting user_partitions through the save method in caching_descriptor_system's "xblock_from_json" method.

There are other fields already being set in this way (discussion_topics, grading policy, etc.), so I don't think this is a new issue. But is there a better way to do this?

cahrens · 2017-03-23T01:39:22Z

            "i18n": ModuleI18nService,
            "settings": SettingsService(),
            "user": DjangoXBlockUserService(request.user),
+            "partitions": PreviewPartitionService(user=request.user, course_id=course_id)


@nasthagiri I needed to add a Studio version of the service or else viewing the components in Studio failed (due to mixin further down, which is used when running in Studio despite the fact that it actually lives in lms).

cahrens · 2017-03-23T01:42:33Z

        user_partitions = [
            user_partition
-            for user_partition in getattr(root_block, 'user_partitions', [])
+            for user_partition in get_course_user_partitions(root_block)


@nasthagiri transformer code change. I am assuming "root_block" here is the course.

cahrens · 2017-03-23T01:44:21Z

            return {unicode(k): access_dict[k] for k in access_dict}


+@XBlock.wants('partitions')


@nasthagiri This is the mixin, which needs to reference the runtime service (no direct reference to course here).

cahrens · 2017-03-23T01:46:58Z

+ENROLLMENT_TRACK_PARTITION_ID = 50
+
+
+def get_course_user_partitions(course):


@nasthagiri here is the helper method, get_course_user_partitions.

cahrens · 2017-03-23T01:50:51Z

+        return
+
+    used_ids = set(p.id for p in course.user_partitions)
+    if ENROLLMENT_TRACK_PARTITION_ID in used_ids:


@nasthagiri the ID situation here is tricky. I can't just pick any unused ID anymore because this partition/ID is never persisted on the course, and yet xblock's group_access WILL store the partition ID, so it is crucial that the ID always be the same whenever this dynamic partition is returned.

Luckily we should have 0-99 free as indices, so I think that dynamic partitions just have to declare their IDs as constants within that range (and if an XML-authored course happens to hit one of these IDs, then we have to skip adding those dynamic partitions). Of course, this will require much commenting, but for now I'm using "50".

cahrens · 2017-03-23T01:54:09Z

        `NoSuchUserPartitionError` if the lookup fails.
        """
-        for user_partition in self.user_partitions:
+        for user_partition in self.runtime.service(self, 'partitions').course_partitions:


@nasthagiri this method is what the access control code calls. Note that "self" is any xblock, not just the course. I am now looking at the course user_partitions instead of self.user_partitions, but I think that is fine since the course is the only place the partitions should be added/modified.

For reference, here is the access usage: https://github.com/edx/edx-platform/blob/b287f48f8b9ea31e2ad42be8f609dbb0e0059ef9/lms/djangoapps/courseware/access.py#L496

cahrens · 2017-03-23T02:27:42Z

+    def get_course(self):
+        return modulestore().get_course(self._course_id)
+
+    def get_user_group_id_for_partition(self, user_partition_id):


@nasthagiri We have to override this method because now when running in Studio a partitions runtime service actually exists. Previously, code (see link below) would just return None when trying to get the service (and the code was robust to that):

https://github.com/edx/edx-platform/blob/eea25184521c4fe224adc676006f6d86edb8e215/common/lib/xmodule/xmodule/split_test_module.py#L195

Rename as StudioPartitionService.

cahrens · 2017-03-23T15:22:19Z

@nasthagiri and @cpennington I'm struggling with how best to set the partition service for tests that depend on checking group access. There seem to be a variety of ways that tests set services, and I don't understand the differences.

In this particular case, you can see I just stuck it direction on item.runtime._services. That seems pretty ugly.

I'll add some comments to other ways this is done, as I'd like to make sure I'm not introducing technical debt into the tests. I'm not clear about when services that require a user instance are normally mixed in.

@cpennington @nasthagiri just hit another partition service unit test problem that I don't know how to solve. It's related to discussion component visibility, and I can't bind a partition service to an actual xblock instance because the unit tests are testing APIs that read all the discussion xblocks from the course module and then iterate through them.

cahrens · 2017-03-23T15:25:32Z

@cpennington @nasthagiri Same sort of pattern as the last test case, though here I'm using LmsPartitionService becuase the test lives in LMS.

cahrens · 2017-03-23T18:51:38Z

Defaulting to self.user_partitions if the service is unavailable should fix all my failing unit tests, but I don't feel great about it. Though if I don't provide this fallback, I should change the decorator to XBlock.needs.

@cpennington @nasthagiri thoughts?

cahrens · 2017-03-27T20:25:41Z

jenkins run bokchoy

cahrens · 2017-03-27T20:27:10Z

jenkins run lettuce

cahrens · 2017-03-27T20:38:11Z

@staubina and @nasthagiri this PR is ready for re-review, and the sandbox has been updated (the link in the Description section is up-to-date).

cahrens · 2017-03-27T21:00:04Z

+
+
+# settings will not be available when running nosetests.
+FEATURES = getattr(settings, 'FEATURES', {})


This is the same thing that was recently introduced into capa, so hopefully it is an acceptable pattern.

https://github.com/edx/edx-platform/blob/master/common/lib/xmodule/xmodule/capa_base.py#L45

cahrens · 2017-03-27T21:03:50Z

 persist the assignments.
 """
-from abc import ABCMeta, abstractproperty
+from django.conf import settings


@nasthagiri are these django imports OK in this file? Other files in common/lib/xmodule use this workaround instead of importing django.utils.translation:

# Make '_' a no-op so we can scrape strings. Using lambda instead of # `django.utils.translation.ugettext_noop` because Django cannot be imported in this file _ = lambda text: text

django.conf, on the other hand, is imported in a number of files in common/lib/xmodule.

Yes, it's Ok. Just as modulestore/django.py is django-dependent, we can say that the partitions folder is also django-dependent.

staubina · 2017-03-28T16:53:38Z

 from xmodule.course_metadata_utils import DEFAULT_START_DATE
 from xmodule.graders import grader_from_conf
 from xmodule.mixin import LicenseMixin
+from xmodule.partitions.partitions import UserPartition


Is this used anywhere in the file? I do not see it when I search.

Not really-- it's referenced in doc, but I think the import can be removed.

Reminder to remove this import.

staubina · 2017-03-28T17:19:23Z

+    def setUp(self):
+        super(TestGetCourseUserPartitions, self).setUp()
+        # django.conf.settings is not available when nosetests are run
+        FEATURES['ENABLE_ENROLLMENT_TRACK_USER_PARTITION'] = True


nit: Define 'ENABLE_ENROLLMENT_TRACK_USER_PARTITION' as a constant because it is used more than once.

cahrens · 2017-03-28T17:50:05Z

 from xblock.fields import Scope, Boolean, String, Float, XBlockMixin, Dict, Integer, List
 from xblock.runtime import KeyValueStore, KvsFieldData
 from xmodule.fields import Date, Timedelta
+from xmodule.partitions.partitions import UserPartition


I can move this import back to its original location so this file isn't part of the change set.

No worries. Small change.

staubina · 2017-03-28T18:03:14Z

Just some minor comments, nothing major from what I can see.

Tested on the Sandbox also and it looks good.

nasthagiri · 2017-03-29T13:13:42Z

        Assign ids for the group_configuration's groups.
        """
-        used_ids = [g.id for p in self.course.user_partitions for g in p.groups]
+        used_ids = [g.id for p in get_course_user_partitions(self.course) for g in p.groups]


Nit: I wonder if there's a better name for get_course_user_partitions? It was unclear from the name whether this returned only the partitions stored in the course content, or also included the dynamic partitions on the course?
Maybe rename to get_all_partitions_for_course?

Yes, I agree. I'll change it to get_all_partitions_for_course.

nasthagiri · 2017-03-29T13:34:56Z

        self.course.user_partitions = [
            UserPartition(
-                id=0,
+                id=100,


Can you add a comment to explain why this number?

It actually could stay at 0 now, since the enrollment track partition is at 50. I was just trying to make the numbers more realistic, as they would have a minimum value of 100.

I don't want to put comments throughout the whole file, but I will put a place to put a general comment about IDs and the different ranges of numbers.

nasthagiri · 2017-03-29T13:41:20Z

    'ALLOW_PUBLIC_ACCOUNT_CREATION': True,
+
+    # Whether or not the dynamic EnrollmentTrackUserPartition should be registered.
+    'ENABLE_ENROLLMENT_TRACK_USER_PARTITION': False


Nit: Add ending commas here and above.

nasthagiri · 2017-03-29T13:43:25Z

 from xmodule.course_metadata_utils import DEFAULT_START_DATE
 from xmodule.graders import grader_from_conf
 from xmodule.mixin import LicenseMixin
+from xmodule.partitions.partitions import UserPartition


Reminder to remove this import.

nasthagiri · 2017-03-29T13:44:22Z

 from xblock.fields import Scope, Boolean, String, Float, XBlockMixin, Dict, Integer, List
 from xblock.runtime import KeyValueStore, KvsFieldData
 from xmodule.fields import Date, Timedelta
+from xmodule.partitions.partitions import UserPartition


No worries. Small change.

nasthagiri · 2017-03-29T14:00:08Z

 persist the assignments.
 """
-from abc import ABCMeta, abstractproperty
+from django.conf import settings


Yes, it's Ok. Just as modulestore/django.py is django-dependent, we can say that the partitions folder is also django-dependent.

cahrens · 2017-03-29T20:14:10Z

@nasthagiri and @staubina I have addressed the review feedback and rebased on master.

staubina · 2017-03-30T11:16:47Z

👍

nasthagiri

Looks great overall. My final comments are mostly Nits/Considerations. Except for the comment regarding raising an exception rather than defensively handling with a silent failure. I trust you'll figure out a solution for that.

nasthagiri · 2017-03-30T12:29:28Z

+        log.warning("No 'enrollment_track' scheme registered, EnrollmentTrackUserPartition will not be created.")
+        return None
+
+    used_ids = set(p.id for p in course.user_partitions)


Do we also prevent new partitions from being created with the same ID as ENROLLMENT_TRACK_PARTITION_ID? I'm concerned of long downstream issues where we have this check in place every time get_all_partitions_for_course is called, and we silently fail, allowing a course-team/developer to override an already existing partition. This can result in potentially hard-to-debug and unexpected conditions - where a new partition overtakes the enrollment-track partition and starts using previously configured user-access mappings.
Why not raise an exception here instead?
If you're concerned about initial rollout and worried there may be courses already using this partition, then, you can:

A priori: Run a mongo query to find those courses beforehand.

Post priori: Release this initially with a silent failure, but monitor splunk. And then change this to an exception after rollout (with a TNL ticket reminding to change this). However, you have no guarantee that all courses have been accessed by that time.

Put the raise exception behind a waffle flag so you can disable it quickly if needed.

@nasthagiri I have thought more about this and am coming around to an exception. We are guaranteed that partitions created in Studio won't conflict (they are > 100, and they also avoid all current IDs, including the dynamic ones), but partitions created in some other way could potentially conflict.

I like option 2. I will create a ticket for it in our epic.

Note that when we get to the discussions work we will likely want to throw an exception if 2 user partitions exist with a group that has the same ID. That's another place where we should be fine for partitions created by Studio, but could in the future run into difficulty.

nasthagiri · 2017-03-30T12:32:02Z

By the way, in the future, consider using a waffle switch in these cases. This way, you wouldn't need to be blocked on a release in order to enable/disable the feature. I've been using it for several features now, and created helper waffle utility classes: https://github.com/edx/edx-platform/blob/master/openedx/core/djangolib/waffle_utils.py

nasthagiri · 2017-03-30T12:35:51Z

+        if partition.id == user_partition_id:
+            return partition
+
+    return None


By the way, python always returns None by default. So this line unnecessary.

I know, it's just clearer to me.

nasthagiri · 2017-03-30T12:39:02Z

+FEATURES = getattr(settings, 'FEATURES', {})
+
+
+def get_all_partitions_for_course(course):


Do you expect all callers to remember to check for the .is_active field on each partition? Or, do you want to add 'check_for_active` as an optional parameter here?

That's a good idea-- it will simplify some of our other code and remind people that "active" exists (something which I myself forgot). Will do.

nasthagiri · 2017-03-30T12:40:57Z

Hopefully this doesn't cause a noticeable performance issue. We really need to transition over to using (the optimized) transformers soon for all course access control.

nasthagiri · 2017-03-30T12:43:55Z

+    "professional": 3,
+    "no-id-professional": 4,
+    "credit": 5,
+    "honor": 6


Nitty: ending comma

nasthagiri · 2017-03-30T12:44:25Z


-    # Verified Track Content Cohorting
-    'verified_track_content',
+    # Verified Track Content Cohorting (Beta feature that will hopefully be removed)


Is there a TNL ticket for this cleanup task? Consider adding the ticket number to this comment.

No, there isn't one yet. I think creating it is a bit premature, as we have to see how the migration of courses (at rerun time) ends up going.

nasthagiri · 2017-03-30T12:47:03Z

+            course_mode = CourseMode.mode_for_course(
+                course_key,
+                mode_slug,
+                modes=CourseMode.modes_for_course(course_key, include_expired=True, only_selectable=False)


Nitty: ending comma

nasthagiri · 2017-03-30T12:48:06Z

+                modes=CourseMode.modes_for_course(course_key, include_expired=True, only_selectable=False)
+            )
+            if not course_mode:
+                course_mode = CourseMode.DEFAULT_MODE


Shouldn't the logic of returning the default mode be included within CourseMode.mode_for_course?

It's not-- that method returns None if the mode requested does not exist. What our system does in general in that extreme edge case, I can't venture to guess.

nasthagiri · 2017-03-30T12:51:12Z

-        partition_id = 0
-        group_0_id = 0
-        group_1_id = 1
+        partition_id = 100


Here's another case of using the magic 100 value. Consider adding a constant for this somewhere so we can use it in all affected tests - and the comment to explain why would be in that single place where the constant is defined.

Yes, I think I will create a constant and have the tests and Studio code reference it.

cahrens · 2017-03-30T15:36:41Z

@nasthagiri @staubina I have addressed the most recent feedback and rebased on master. If you are interested in looking at the changes, see the last 3 commits.

I plan to squash commits once I get a clean build on this, so probably around 12:30 PM.

Update: these are the commits, if anyone wants to see them.

TNL-6674

cahrens · 2017-03-30T16:49:29Z

jenkins run js

edx-pipeline-bot · 2017-03-30T19:55:19Z

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Friday, March 31, 2017.

cpennington · 2017-03-31T19:28:37Z

EdX Release Notice: This PR has been deployed to production. (We had an infrastructure issue, so this message was posted manually.)

cahrens commented Mar 20, 2017

View reviewed changes

cahrens changed the title ~~Christina/enrollment track partition review~~ EnrollmentTrackUserPartition to be used for differentiated content Mar 20, 2017

cahrens mentioned this pull request Mar 20, 2017

Introduce EnrollmentTrackUserPartition. #14682

Closed

nasthagiri reviewed Mar 21, 2017

View reviewed changes

cahrens commented Mar 21, 2017

View reviewed changes

cahrens force-pushed the christina/enrollment_track_partition_review branch 5 times, most recently from 025e07e to 0ecbbf2 Compare March 23, 2017 00:55

cahrens commented Mar 23, 2017

View reviewed changes

cahrens force-pushed the christina/enrollment_track_partition_review branch 3 times, most recently from d56223e to cc6a17c Compare March 27, 2017 20:00

cahrens closed this Mar 27, 2017

cahrens reopened this Mar 27, 2017

cahrens commented Mar 27, 2017

View reviewed changes

staubina reviewed Mar 28, 2017

View reviewed changes

cahrens commented Mar 28, 2017

View reviewed changes

nasthagiri reviewed Mar 29, 2017

View reviewed changes

cahrens force-pushed the christina/enrollment_track_partition_review branch 2 times, most recently from c351f79 to e38623d Compare March 29, 2017 18:56

cahrens mentioned this pull request Mar 29, 2017

Create task to process a batch of grade computations. #14752

Merged

1 task

nasthagiri approved these changes Mar 30, 2017

View reviewed changes

cahrens force-pushed the christina/enrollment_track_partition_review branch from f7dccf5 to ebb9e96 Compare March 30, 2017 15:35

cahrens force-pushed the christina/enrollment_track_partition_review branch from ebb9e96 to e503574 Compare March 30, 2017 16:24

Introduce EnrollmentTrackUserPartition.

e503574

TNL-6674

cahrens merged commit 7c774ae into master Mar 30, 2017

jdmulloy deleted the christina/enrollment_track_partition_review branch April 5, 2018 19:23

Agrendalath mentioned this pull request Jul 18, 2023

refactor: reuse services and wrappers between XBlocks (fixed) [FC-0026] #32779

Merged

		model = CourseModeExpirationConfig


		class EnrollmentModeAdmin(admin.ModelAdmin):



		class CourseModeForm(forms.ModelForm):
		""" CourseModeForm """

		return {unicode(k): access_dict[k] for k in access_dict}


		@XBlock.wants('partitions')

		ENROLLMENT_TRACK_PARTITION_ID = 50


		def get_course_user_partitions(course):



		# settings will not be available when running nosetests.
		FEATURES = getattr(settings, 'FEATURES', {})

		FEATURES = getattr(settings, 'FEATURES', {})


		def get_all_partitions_for_course(course):

Uh oh!

Conversation

cahrens commented Mar 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Sandbox

Testing

Reviewers

Post-review

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nasthagiri left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cahrens Mar 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cahrens commented Mar 20, 2017 •

edited

Loading

nasthagiri left a comment •

edited

Loading

cahrens Mar 23, 2017 •

edited

Loading

cahrens Mar 23, 2017 •

edited

Loading

cahrens Mar 23, 2017 •

edited

Loading

cahrens Mar 23, 2017 •

edited

Loading

cahrens Mar 23, 2017 •

edited

Loading

cahrens Mar 23, 2017 •

edited

Loading