Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some comments and type hints for configuring #1178

Merged
merged 1 commit into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
lainets marked this conversation as resolved.
Show resolved Hide resolved
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
Loading