Assignment handling within single-user syncing #8219
Conversation
jamalex
changed the title
| ) # noqa: F401 | ||
| from morango.api.viewsets import session_controller # noqa: F401 | ||
|
|
||
| register_sync_event_handlers(session_controller) |
There was a problem hiding this comment.
I wasn't sure whether this was the best place to put this, but... I couldn't think of a better place. Suggestions welcome, or maybe this is ok.
|
|
||
| kwargs = _extract_kwargs_from_context(context) | ||
|
|
||
| if isinstance(context, LocalSessionContext): |
There was a problem hiding this comment.
This is to avoid having the hooks be called twice, once for local and once for remote context.
| @@ -57,7 +57,7 @@ def manage(self, *args): | |||
|
|
|||
| def create_model(self, model, **kwargs): | |||
| kwarg_text = ",".join( | |||
| '{key}=\\"{value}\\"'.format(key=key, value=value) | |||
| "{key}={value}".format(key=key, value=repr(value)) | |||
| for key, value in kwargs.items() | |||
There was a problem hiding this comment.
Was getting some format issues in the piping -- this helped.
jamalex
force-pushed
the
your_assignment_is_due_on_monday_class
branch
from
455b6c8 to
89d106e
Compare
| @@ -0,0 +1,106 @@ | |||
| from .models import ExamAssignment | |||
There was a problem hiding this comment.
This file is virtually identical to kolibri/core/lessons/single_user_assignment_utils.py -- in terms of the overall structure -- some field names and details are different.
I'd be happy to try to DRY it up, it just might make it a bit more "magical" (e.g. a function containing this logic that gets passed models and field names etc, and then the comments in here etc would need to be generalized as well -- might hurt readability.
There was a problem hiding this comment.
agree with the copy/paste strategy for code maintainability
|
|
||
|
|
||
| @define_hook | ||
| class FacilityDataSyncHook(KolibriHook): |
There was a problem hiding this comment.
This could be separated out into a pre-hook and post-hook, which would then allow each method to be setup with an @abc.abstractmethod decorator, which then gives a clear requirement that the method be instantiated.
|
|
||
| # create new syncable exam objects for all new assignments | ||
| for assignment in to_create: | ||
| IndividualSyncableExam.objects.create( |
There was a problem hiding this comment.
Any reason not to do a bulk create here?
Summary
Fixes #8079.
Reviewer guidance
There's a lot of code duplication between the logic for exams and for lessons, but they're different enough (e.g. in the naming of some attributes) that it seemed cleanest to just copy-paste and adapt.
I'll add some inline notes.
To run the test locally, I use:
Testing checklist
PR process
Reviewer checklist
yarnandpip)