Skip to content

Commit

Permalink
Add some comments and type hints for configuring
Browse files Browse the repository at this point in the history
Some type hints were missing and added comments to clarify some parts
  • Loading branch information
lainets authored and markkuriekkinen committed May 30, 2023
1 parent 0a5d7f5 commit 2d74071
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 5 deletions.
12 changes: 9 additions & 3 deletions course/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import json
import logging
import string
from typing import Any, Dict, List, Tuple
from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING
import urllib.request
import urllib.parse
from random import choice
Expand Down Expand Up @@ -44,6 +44,9 @@
from userprofile.models import User, UserProfile, GraderUser
from .sis import get_sis_configuration, StudentInfoSystem

if TYPE_CHECKING:
from edit_course.operations.configure import ConfigParts


logger = logging.getLogger('aplus.course')

Expand Down Expand Up @@ -1113,17 +1116,20 @@ def get_url_kwargs(self):
# python dicts in single line: http://stackoverflow.com/a/26853961
return dict(instance_slug=self.url, **self.course.get_url_kwargs()) # pylint: disable=use-dict-literal

# A ConfigPart object is cached for each instance during an update. Cache is cleared
# when the instance is deleted. The edit_course_operations.configure logic is responsible
# for setting the cache on update.
@property
def config_cache_key(self) -> str:
return f"instance.config.{self.id}"

def delete_cached_config(self) -> None:
cache.delete(self.config_cache_key)

def set_cached_config(self, obj: Any) -> None:
def set_cached_config(self, obj: "ConfigParts") -> None:
cache.set(self.config_cache_key, obj)

def get_cached_config(self, default=None) -> Any:
def get_cached_config(self, default=None) -> Optional["ConfigParts"]:
return cache.get(self.config_cache_key, default)


Expand Down
13 changes: 11 additions & 2 deletions edit_course/operations/configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,10 @@ class ConfigParts:
the lobject class type.
- *_keys fields contain whatever the <new> config contained. I.e. they
contain the set of all such keys. E.g. module_keys contains the keys of
all modules in the <new> config.
all modules in the <new> config. They can be used to determined whether
a module, exercise, etc. still exists in the course config even if nothing
has changed (e.g. module config disappears from the modules field in the
.diff() result if it had no changes).
"""
config: Dict[Any, Any]

Expand Down Expand Up @@ -490,7 +493,8 @@ def get_children(config, n = 0):
)

@staticmethod
def diff(old: Optional["ConfigParts"], new: "ConfigParts"):
def diff(old: Optional["ConfigParts"], new: "ConfigParts") -> "ConfigParts":
"""Return a new ConfigParts object containing the differences between the old and new configs"""
if old is None:
return new

Expand All @@ -502,6 +506,7 @@ def diff(old: Optional["ConfigParts"], new: "ConfigParts"):
keep_unchanged=True
)

# Get the changes between the configs
categories_config = get_config_changes(old.categories, new.categories)
if categories_config is None:
categories_config = {}
Expand Down Expand Up @@ -550,8 +555,11 @@ def configure(instance: CourseInstance, new_config: dict) -> Tuple[bool, List[st
errors.append(_('COURSE_CONFIG_ERROR_MODULES_REQUIRED_ARRAY'))
return False, errors

# Get the previous config (the one used in the last successful update)
old_cparts = instance.get_cached_config()
new_cparts = ConfigParts.from_config(new_config, errors)
# Get the changes between the new config and the previous config with some auxiliary information.
# See ConfigParts and ConfigParts.diff for more info
cparts = ConfigParts.diff(old_cparts, new_cparts)

config = cparts.config
Expand Down Expand Up @@ -761,6 +769,7 @@ def configure(instance: CourseInstance, new_config: dict) -> Tuple[bool, List[st
for child_key in lobject_config["children"]
)

# Delete or hide learning objects that are not included in the module anymore
for lobject in outdated_lobjects:
if (
not isinstance(lobject, BaseExercise)
Expand Down

0 comments on commit 2d74071

Please sign in to comment.