From 66cf7a24d495d9ddcf2f4739dcdc91dbc33d7725 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Fri, 1 Mar 2024 16:11:25 -0500 Subject: [PATCH] chore: use black and isort to format files automatically --- Makefile | 4 + docs/conf.py | 113 +++++++------- manage.py | 4 +- platform_plugin_aspects/apps.py | 4 +- platform_plugin_aspects/extensions/filters.py | 4 +- platform_plugin_aspects/serializers.py | 1 + platform_plugin_aspects/settings/common.py | 3 +- .../settings/production.py | 4 +- platform_plugin_aspects/signals.py | 17 ++- platform_plugin_aspects/sinks/base_sink.py | 13 +- .../sinks/course_published.py | 7 +- .../sinks/external_id_sink.py | 1 + .../sinks/user_profile_sink.py | 1 + platform_plugin_aspects/sinks/user_retire.py | 1 + platform_plugin_aspects/tasks.py | 4 +- .../commands/test_dump_data_to_clickhouse.py | 2 +- .../tests/test_base_sink.py | 18 ++- .../tests/test_course_published.py | 130 +++++++++------- .../tests/test_external_id_sink.py | 4 +- .../tests/test_settings.py | 6 +- platform_plugin_aspects/tests/test_signals.py | 7 +- platform_plugin_aspects/tests/test_tasks.py | 5 +- .../tests/test_user_retire.py | 1 + platform_plugin_aspects/tests/test_utils.py | 22 ++- platform_plugin_aspects/utils.py | 4 +- platform_plugin_aspects/waffle.py | 1 + setup.cfg | 6 +- setup.py | 97 +++++++----- test_settings.py | 31 ++-- test_utils/helpers.py | 143 +++++++++++------- tox.ini | 11 +- 31 files changed, 395 insertions(+), 274 deletions(-) diff --git a/Makefile b/Makefile index 08ada5f..5273bcc 100644 --- a/Makefile +++ b/Makefile @@ -72,6 +72,10 @@ test: clean ## run tests in the current virtualenv diff_cover: test ## find diff lines that need test coverage diff-cover coverage.xml +format: + isort platform_plugin_aspects + black . + test-all: quality pii_check ## run tests on every supported Python/Django combination tox tox -e docs diff --git a/docs/conf.py b/docs/conf.py index caf4d87..e2fb66a 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -33,15 +33,15 @@ def get_version(*file_paths): version_match = re.search(r"^__version__ = ['\"]([^'\"]*)['\"]", version_file, re.M) if version_match: return version_match.group(1) - raise RuntimeError('Unable to find version string.') + raise RuntimeError("Unable to find version string.") REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) sys.path.append(REPO_ROOT) -VERSION = get_version('../platform_plugin_aspects', '__init__.py') +VERSION = get_version("../platform_plugin_aspects", "__init__.py") # Configure Django for autodoc usage -os.environ['DJANGO_SETTINGS_MODULE'] = 'test_settings' +os.environ["DJANGO_SETTINGS_MODULE"] = "test_settings" django_setup() # If extensions (or modules to document with autodoc) are in another directory, @@ -62,46 +62,46 @@ def get_version(*file_paths): # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom # ones. extensions = [ - 'sphinx.ext.autodoc', - 'sphinx.ext.doctest', - 'sphinx.ext.intersphinx', - 'sphinx.ext.ifconfig', - 'sphinx.ext.napoleon' + "sphinx.ext.autodoc", + "sphinx.ext.doctest", + "sphinx.ext.intersphinx", + "sphinx.ext.ifconfig", + "sphinx.ext.napoleon", ] # A list of warning types to suppress arbitrary warning messages. suppress_warnings = [ - 'image.nonlocal_uri', + "image.nonlocal_uri", ] # Add any paths that contain templates here, relative to this directory. -templates_path = ['_templates'] +templates_path = ["_templates"] # The suffix(es) of source filenames. # You can specify multiple suffix as a list of string: # # source_suffix = ['.rst', '.md'] -source_suffix = '.rst' +source_suffix = ".rst" # The encoding of source files. # # source_encoding = 'utf-8-sig' # The top level toctree document. -top_level_doc = 'index' +top_level_doc = "index" # General information about the project. -project = 'platform-plugin-aspects' -copyright = f'{datetime.now().year}, Axim Collaborative, Inc.' # pylint: disable=redefined-builtin -author = 'Axim Collaborative, Inc.' -project_title = 'platform-plugin-aspects' +project = "platform-plugin-aspects" +copyright = f"{datetime.now().year}, Axim Collaborative, Inc." # pylint: disable=redefined-builtin +author = "Axim Collaborative, Inc." +project_title = "platform-plugin-aspects" documentation_title = f"{project_title}" # Set display_github to False if you don't want "edit on Github" button html_context = { "display_github": True, # Integrate GitHub "github_user": "edx", # Username - "github_repo": 'platform-plugin-aspects', # Repo name + "github_repo": "platform-plugin-aspects", # Repo name "github_version": "main", # Version "conf_py_path": "/docs/", # Path in the checkout to the docs root } @@ -120,7 +120,7 @@ def get_version(*file_paths): # # This is also used if you do content translation via gettext catalogs. # Usually you set "language" from the command line for these cases. -language = 'en' +language = "en" # There are two options for replacing |today|: either, you set today to some # non-false value, then it is used: @@ -135,12 +135,12 @@ def get_version(*file_paths): # directories to ignore when looking for source files. # This patterns also effect to html_static_path and html_extra_path exclude_patterns = [ - '_build', - 'Thumbs.db', - '.DS_Store', + "_build", + "Thumbs.db", + ".DS_Store", # This file is intended as a guide for developers browsing the source tree, # not to be rendered into the output docs. - 'decisions/README.rst', + "decisions/README.rst", ] # The reST default role (used for this markup: `text`) to use for all @@ -163,7 +163,7 @@ def get_version(*file_paths): # show_authors = False # The name of the Pygments (syntax highlighting) style to use. -pygments_style = 'sphinx' +pygments_style = "sphinx" # A list of ignored prefixes for module index sorting. # modindex_common_prefix = [] @@ -179,7 +179,7 @@ def get_version(*file_paths): # The theme to use for HTML and HTML Help pages. See the documentation for # a list of builtin themes. -html_theme = 'sphinx_book_theme' +html_theme = "sphinx_book_theme" # Theme options are theme-specific and customize the look and feel of a theme # further. For a list of options available for each theme, see the @@ -187,7 +187,7 @@ def get_version(*file_paths): # html_theme_options = { "repository_url": "https://github.com/openedx/platform-plugin-aspects", - "repository_branch": 'main', + "repository_branch": "main", "path_to_docs": "docs/", "home_page_in_toc": True, "use_repository_button": True, @@ -214,7 +214,7 @@ def get_version(*file_paths): rel="license" href="https://creativecommons.org/licenses/by-sa/4.0/" >Creative Commons Attribution-ShareAlike 4.0 International License. - """ + """, } # Add any paths that contain custom themes here, relative to this directory. @@ -244,7 +244,7 @@ def get_version(*file_paths): # Add any paths that contain custom static files (such as style sheets) here, # relative to this directory. They are copied after the builtin static files, # so a file named "default.css" will overwrite the builtin "default.css". -html_static_path = ['_static'] +html_static_path = ["_static"] # Add any extra paths that contain custom files (such as robots.txt or # .htaccess) here, relative to this directory. These files are copied @@ -324,7 +324,7 @@ def get_version(*file_paths): # html_search_scorer = 'scorer.js' # Output file base name for HTML help builder. -htmlhelp_basename = f'{project}doc' +htmlhelp_basename = f"{project}doc" # -- Options for LaTeX output --------------------------------------------- @@ -332,15 +332,12 @@ def get_version(*file_paths): # The paper size ('letterpaper' or 'a4paper'). # # 'papersize': 'letterpaper', - # The font size ('10pt', '11pt' or '12pt'). # # 'pointsize': '10pt', - # Additional stuff for the LaTeX preamble. # # 'preamble': '', - # Latex figure (float) alignment # # 'figure_align': 'htbp', @@ -349,10 +346,9 @@ def get_version(*file_paths): # Grouping the document tree into LaTeX files. List of tuples # (source start file, target name, title, # author, documentclass [howto, manual, or own class]). -latex_target = f'{project}.tex' +latex_target = f"{project}.tex" latex_documents = [ - (top_level_doc, latex_target, documentation_title, - author, 'manual'), + (top_level_doc, latex_target, documentation_title, author, "manual"), ] # The name of an image file (relative to this directory) to place at the top of @@ -392,10 +388,7 @@ def get_version(*file_paths): # One entry per manual page. List of tuples # (source start file, name, description, authors, manual section). -man_pages = [ - (top_level_doc, project_title, documentation_title, - [author], 1) -] +man_pages = [(top_level_doc, project_title, documentation_title, [author], 1)] # If true, show URL addresses after external links. # @@ -408,9 +401,15 @@ def get_version(*file_paths): # (source start file, target name, title, author, # dir menu entry, description, category) texinfo_documents = [ - (top_level_doc, project_title, documentation_title, - author, project_title, 'Aspects plugins for edx-platform', - 'Miscellaneous'), + ( + top_level_doc, + project_title, + documentation_title, + author, + project_title, + "Aspects plugins for edx-platform", + "Miscellaneous", + ), ] # Documents to append as an appendix to all manuals. @@ -484,7 +483,7 @@ def get_version(*file_paths): # epub_post_files = [] # A list of files that should not be packed into the epub file. -epub_exclude_files = ['search.html'] +epub_exclude_files = ["search.html"] # The depth of the table of contents in toc.ncx. # @@ -517,9 +516,12 @@ def get_version(*file_paths): # Example configuration for intersphinx: refer to the Python standard library. intersphinx_mapping = { - 'python': ('https://docs.python.org/3.8', None), - 'django': ('https://docs.djangoproject.com/en/3.2/', 'https://docs.djangoproject.com/en/3.2/_objects/'), - 'model_utils': ('https://django-model-utils.readthedocs.io/en/latest/', None), + "python": ("https://docs.python.org/3.8", None), + "django": ( + "https://docs.djangoproject.com/en/3.2/", + "https://docs.djangoproject.com/en/3.2/_objects/", + ), + "model_utils": ("https://django-model-utils.readthedocs.io/en/latest/", None), } @@ -531,17 +533,24 @@ def on_init(app): # pylint: disable=unused-argument avoid checking in the generated reStructuredText files. """ docs_path = os.path.abspath(os.path.dirname(__file__)) - root_path = os.path.abspath(os.path.join(docs_path, '..')) - apidoc_path = 'sphinx-apidoc' - if hasattr(sys, 'real_prefix'): # Check to see if we are in a virtualenv + root_path = os.path.abspath(os.path.join(docs_path, "..")) + apidoc_path = "sphinx-apidoc" + if hasattr(sys, "real_prefix"): # Check to see if we are in a virtualenv # If we are, assemble the path manually - bin_path = os.path.abspath(os.path.join(sys.prefix, 'bin')) + bin_path = os.path.abspath(os.path.join(sys.prefix, "bin")) apidoc_path = os.path.join(bin_path, apidoc_path) - check_call([apidoc_path, '-o', docs_path, os.path.join(root_path, 'platform_plugin_aspects'), - os.path.join(root_path, 'platform_plugin_aspects/migrations')]) + check_call( + [ + apidoc_path, + "-o", + docs_path, + os.path.join(root_path, "platform_plugin_aspects"), + os.path.join(root_path, "platform_plugin_aspects/migrations"), + ] + ) def setup(app): """Sphinx extension: run sphinx-apidoc.""" - event = 'builder-inited' + event = "builder-inited" app.connect(event, on_init) diff --git a/manage.py b/manage.py index c4c1f40..f45575c 100644 --- a/manage.py +++ b/manage.py @@ -8,8 +8,8 @@ PWD = os.path.abspath(os.path.dirname(__file__)) -if __name__ == '__main__': - os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'test_settings') +if __name__ == "__main__": + os.environ.setdefault("DJANGO_SETTINGS_MODULE", "test_settings") sys.path.append(PWD) try: from django.core.management import execute_from_command_line diff --git a/platform_plugin_aspects/apps.py b/platform_plugin_aspects/apps.py index 97db605..fa2b8a4 100644 --- a/platform_plugin_aspects/apps.py +++ b/platform_plugin_aspects/apps.py @@ -49,4 +49,6 @@ def ready(self): sinks, tasks, ) - from platform_plugin_aspects.extensions import filters # pylint: disable=unused-import, import-outside-toplevel + from platform_plugin_aspects.extensions import ( # pylint: disable=unused-import, import-outside-toplevel + filters, + ) diff --git a/platform_plugin_aspects/extensions/filters.py b/platform_plugin_aspects/extensions/filters.py index 1ba6d23..60b7959 100644 --- a/platform_plugin_aspects/extensions/filters.py +++ b/platform_plugin_aspects/extensions/filters.py @@ -37,9 +37,7 @@ def run_filter( filters = ASPECTS_SECURITY_FILTERS_FORMAT + extra_filters_format - context = generate_superset_context( - context, dashboard_uuid, filters - ) + context = generate_superset_context(context, dashboard_uuid, filters) template = Template(self.resource_string("static/html/superset.html")) html = template.render(Context(context)) diff --git a/platform_plugin_aspects/serializers.py b/platform_plugin_aspects/serializers.py index 5be3f64..1057d80 100644 --- a/platform_plugin_aspects/serializers.py +++ b/platform_plugin_aspects/serializers.py @@ -1,4 +1,5 @@ """Django serializers for the event_sink_clickhouse app.""" + import json import uuid diff --git a/platform_plugin_aspects/settings/common.py b/platform_plugin_aspects/settings/common.py index 8b64056..b8b84e2 100644 --- a/platform_plugin_aspects/settings/common.py +++ b/platform_plugin_aspects/settings/common.py @@ -5,6 +5,7 @@ For the full list of settings and their values, see https://docs.djangoproject.com/en/2.22/ref/settings/ """ + from platform_plugin_aspects import ROOT_DIRECTORY @@ -57,5 +58,5 @@ def plugin_settings(settings): "custom_course_edx": { "module": "lms.djangoapps.ccx.models", "model": "CustomCourseForEdX", - } + }, } diff --git a/platform_plugin_aspects/settings/production.py b/platform_plugin_aspects/settings/production.py index e936592..7e344ef 100644 --- a/platform_plugin_aspects/settings/production.py +++ b/platform_plugin_aspects/settings/production.py @@ -11,7 +11,9 @@ def plugin_settings(settings): settings.SUPERSET_CONFIG = getattr(settings, "ENV_TOKENS", {}).get( "SUPERSET_CONFIG", settings.SUPERSET_CONFIG ) - settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID = getattr(settings, "ENV_TOKENS", {}).get( + settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID = getattr( + settings, "ENV_TOKENS", {} + ).get( "ASPECTS_INSTRUCTOR_DASHBOARD_UUID", settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID ) settings.SUPERSET_EXTRA_FILTERS_FORMAT = getattr(settings, "ENV_TOKENS", {}).get( diff --git a/platform_plugin_aspects/signals.py b/platform_plugin_aspects/signals.py index 5fe78be..513c4a6 100644 --- a/platform_plugin_aspects/signals.py +++ b/platform_plugin_aspects/signals.py @@ -1,6 +1,7 @@ """ Signal handler functions, mapped to specific signals in apps.py. """ + from django.db.models.signals import post_save from django.dispatch import Signal, receiver @@ -23,7 +24,9 @@ def receive_course_publish( # pylint: disable=unused-argument # pragma: no cov Receives COURSE_PUBLISHED signal and queues the dump job. """ # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded - from platform_plugin_aspects.tasks import dump_course_to_clickhouse # pylint: disable=import-outside-toplevel + from platform_plugin_aspects.tasks import ( # pylint: disable=import-outside-toplevel + dump_course_to_clickhouse, + ) dump_course_to_clickhouse.delay(str(course_key)) @@ -36,7 +39,9 @@ def on_user_profile_updated( # pylint: disable=unused-argument # pragma: no co Receives post save signal and queues the dump job. """ # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded - from platform_plugin_aspects.tasks import dump_data_to_clickhouse # pylint: disable=import-outside-toplevel + from platform_plugin_aspects.tasks import ( # pylint: disable=import-outside-toplevel + dump_data_to_clickhouse, + ) sink = UserProfileSink(None, None) dump_data_to_clickhouse.delay( @@ -54,7 +59,9 @@ def on_externalid_saved( # pylint: disable=unused-argument # pragma: no cover Receives post save signal and queues the dump job. """ # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded - from platform_plugin_aspects.tasks import dump_data_to_clickhouse # pylint: disable=import-outside-toplevel + from platform_plugin_aspects.tasks import ( # pylint: disable=import-outside-toplevel + dump_data_to_clickhouse, + ) sink = ExternalIdSink(None, None) dump_data_to_clickhouse.delay( @@ -72,7 +79,9 @@ def on_user_retirement( # pylint: disable=unused-argument # pragma: no cover Receives a user retirement signal and queues the retire_user job. """ # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded - from platform_plugin_aspects.tasks import dump_data_to_clickhouse # pylint: disable=import-outside-toplevel + from platform_plugin_aspects.tasks import ( # pylint: disable=import-outside-toplevel + dump_data_to_clickhouse, + ) sink = UserRetirementSink(None, None) dump_data_to_clickhouse.delay( diff --git a/platform_plugin_aspects/sinks/base_sink.py b/platform_plugin_aspects/sinks/base_sink.py index fee2245..7e80ea2 100644 --- a/platform_plugin_aspects/sinks/base_sink.py +++ b/platform_plugin_aspects/sinks/base_sink.py @@ -1,6 +1,7 @@ """ Base classes for event sinks """ + import csv import datetime import io @@ -258,9 +259,9 @@ def send_item(self, serialized_item, many=False): params = self.CLICKHOUSE_BULK_INSERT_PARAMS.copy() # "query" is a special param for the query, it's the best way to get the FORMAT CSV in there. - params[ - "query" - ] = f"INSERT INTO {self.ch_database}.{self.clickhouse_table_name} FORMAT CSV" + params["query"] = ( + f"INSERT INTO {self.ch_database}.{self.clickhouse_table_name} FORMAT CSV" + ) output = io.StringIO() writer = csv.writer(output, quoting=csv.QUOTE_NONNUMERIC) @@ -281,7 +282,9 @@ def send_item(self, serialized_item, many=False): self._send_clickhouse_request(request) - def fetch_target_items(self, start_pk=None, ids=None, skip_ids=None, force_dump=False, batch_size=None): + def fetch_target_items( + self, start_pk=None, ids=None, skip_ids=None, force_dump=False, batch_size=None + ): """ Fetch the items that should be dumped to ClickHouse """ @@ -295,7 +298,7 @@ def fetch_target_items(self, start_pk=None, ids=None, skip_ids=None, force_dump= queryset = queryset.exclude(pk__in=skip_ids) paginator = Paginator(queryset, batch_size) - for i in range(1, paginator.num_pages+1): + for i in range(1, paginator.num_pages + 1): page = paginator.page(i) items = page.object_list for item in items: diff --git a/platform_plugin_aspects/sinks/course_published.py b/platform_plugin_aspects/sinks/course_published.py index 5618959..42f164c 100644 --- a/platform_plugin_aspects/sinks/course_published.py +++ b/platform_plugin_aspects/sinks/course_published.py @@ -9,6 +9,7 @@ Note that the serialization format does not include all fields as there may be things like LTI passwords and other secrets. We just take the fields necessary for reporting at this time. """ + import datetime import json @@ -87,9 +88,9 @@ def serialize_item(self, item, many=False, initial=None): fields["xblock_data_json"]["unit"] = unit_idx fields["xblock_data_json"] = json.dumps(fields["xblock_data_json"]) - location_to_node[ - XBlockSink.strip_branch_and_version(block.location) - ] = fields + location_to_node[XBlockSink.strip_branch_and_version(block.location)] = ( + fields + ) return list(location_to_node.values()) diff --git a/platform_plugin_aspects/sinks/external_id_sink.py b/platform_plugin_aspects/sinks/external_id_sink.py index 7121807..3287eb6 100644 --- a/platform_plugin_aspects/sinks/external_id_sink.py +++ b/platform_plugin_aspects/sinks/external_id_sink.py @@ -1,4 +1,5 @@ """User profile sink""" + from platform_plugin_aspects.serializers import UserExternalIDSerializer from platform_plugin_aspects.sinks.base_sink import ModelBaseSink diff --git a/platform_plugin_aspects/sinks/user_profile_sink.py b/platform_plugin_aspects/sinks/user_profile_sink.py index 565f7a0..31e038f 100644 --- a/platform_plugin_aspects/sinks/user_profile_sink.py +++ b/platform_plugin_aspects/sinks/user_profile_sink.py @@ -1,4 +1,5 @@ """User profile sink""" + from platform_plugin_aspects.serializers import UserProfileSerializer from platform_plugin_aspects.sinks.base_sink import ModelBaseSink diff --git a/platform_plugin_aspects/sinks/user_retire.py b/platform_plugin_aspects/sinks/user_retire.py index e170231..2281c21 100644 --- a/platform_plugin_aspects/sinks/user_retire.py +++ b/platform_plugin_aspects/sinks/user_retire.py @@ -1,4 +1,5 @@ """User retirement sink""" + import requests from django.conf import settings diff --git a/platform_plugin_aspects/tasks.py b/platform_plugin_aspects/tasks.py index 31b8d1a..1e7c967 100644 --- a/platform_plugin_aspects/tasks.py +++ b/platform_plugin_aspects/tasks.py @@ -29,7 +29,9 @@ def dump_course_to_clickhouse(course_key_string, connection_overrides=None): """ if CourseOverviewSink.is_enabled(): # pragma: no cover course_key = CourseKey.from_string(course_key_string) - sink = CourseOverviewSink(connection_overrides=connection_overrides, log=celery_log) + sink = CourseOverviewSink( + connection_overrides=connection_overrides, log=celery_log + ) sink.dump(course_key) ccx_courses = get_ccx_courses(course_key) diff --git a/platform_plugin_aspects/tests/commands/test_dump_data_to_clickhouse.py b/platform_plugin_aspects/tests/commands/test_dump_data_to_clickhouse.py index ec63108..98e7959 100644 --- a/platform_plugin_aspects/tests/commands/test_dump_data_to_clickhouse.py +++ b/platform_plugin_aspects/tests/commands/test_dump_data_to_clickhouse.py @@ -132,7 +132,7 @@ def dump_command_basic_options(): expected_logs=[ "Now dumping 1 Dummy to ClickHouse", "Dumped 2 objects to ClickHouse", - "Last ID: 3" + "Last ID: 3", ], ), CommandOptions( diff --git a/platform_plugin_aspects/tests/test_base_sink.py b/platform_plugin_aspects/tests/test_base_sink.py index 3551675..054b1b8 100644 --- a/platform_plugin_aspects/tests/test_base_sink.py +++ b/platform_plugin_aspects/tests/test_base_sink.py @@ -1,6 +1,7 @@ """ Tests for the base sinks. """ + import logging from unittest.mock import MagicMock, Mock, patch @@ -44,13 +45,16 @@ def test_connection_overrides(self): """ Test that connection_overrides() returns the correct data. """ - child_sink = ChildSink(connection_overrides={ - "url": "http://dummy:8123", - "username": "dummy_username", - "password": "dummy_password", - "database": "dummy_database", - "timeout_secs": 0, - }, log=logging.getLogger()) + child_sink = ChildSink( + connection_overrides={ + "url": "http://dummy:8123", + "username": "dummy_username", + "password": "dummy_password", + "database": "dummy_database", + "timeout_secs": 0, + }, + log=logging.getLogger(), + ) self.assertEqual(child_sink.ch_url, "http://dummy:8123") self.assertEqual(child_sink.ch_auth, ("dummy_username", "dummy_password")) diff --git a/platform_plugin_aspects/tests/test_course_published.py b/platform_plugin_aspects/tests/test_course_published.py index d9fb026..49725fd 100644 --- a/platform_plugin_aspects/tests/test_course_published.py +++ b/platform_plugin_aspects/tests/test_course_published.py @@ -1,6 +1,7 @@ """ Tests for the course_published sinks. """ + import json import logging from datetime import datetime @@ -13,7 +14,10 @@ from responses import matchers from responses.registries import OrderedRegistry -from platform_plugin_aspects.sinks.course_published import CourseOverviewSink, XBlockSink +from platform_plugin_aspects.sinks.course_published import ( + CourseOverviewSink, + XBlockSink, +) from platform_plugin_aspects.tasks import dump_course_to_clickhouse from test_utils.helpers import ( check_block_csv_matcher, @@ -27,9 +31,13 @@ ) -@responses.activate(registry=OrderedRegistry) # pylint: disable=unexpected-keyword-arg,no-value-for-parameter +@responses.activate( # pylint: disable=unexpected-keyword-arg,no-value-for-parameter + registry=OrderedRegistry +) @override_settings(EVENT_SINK_CLICKHOUSE_COURSE_OVERVIEW_ENABLED=True) -@patch("platform_plugin_aspects.sinks.course_published.CourseOverviewSink.serialize_item") +@patch( + "platform_plugin_aspects.sinks.course_published.CourseOverviewSink.serialize_item" +) @patch("platform_plugin_aspects.sinks.course_published.CourseOverviewSink.get_model") @patch("platform_plugin_aspects.sinks.course_published.get_detached_xblock_types") @patch("platform_plugin_aspects.sinks.course_published.get_modulestore") @@ -39,7 +47,7 @@ def test_course_publish_success( mock_modulestore, mock_detached, mock_overview, - mock_serialize_item + mock_serialize_item, ): """ Test of a successful end-to-end run. @@ -49,7 +57,9 @@ def test_course_publish_success( course_overview = fake_course_overview_factory(modified=datetime.now()) mock_modulestore.return_value.get_items.return_value = course - mock_serialize_item.return_value = fake_serialize_fake_course_overview(course_overview) + mock_serialize_item.return_value = fake_serialize_fake_course_overview( + course_overview + ) # Fake the "detached types" list since we can't import it here mock_detached.return_value = mock_detached_xblock_types() @@ -66,14 +76,14 @@ def test_course_publish_success( "https://foo.bar/", match=[ matchers.query_param_matcher(course_overview_params), - check_overview_csv_matcher(course_overview) + check_overview_csv_matcher(course_overview), ], ) responses.post( "https://foo.bar/", match=[ matchers.query_param_matcher(blocks_params), - check_block_csv_matcher(course) + check_block_csv_matcher(course), ], ) @@ -86,13 +96,19 @@ def test_course_publish_success( mock_get_ccx_courses.assert_called_once_with(course_overview.id) -@responses.activate(registry=OrderedRegistry) # pylint: disable=unexpected-keyword-arg,no-value-for-parameter -@patch("platform_plugin_aspects.sinks.course_published.CourseOverviewSink.serialize_item") +@responses.activate( # pylint: disable=unexpected-keyword-arg,no-value-for-parameter + registry=OrderedRegistry +) +@patch( + "platform_plugin_aspects.sinks.course_published.CourseOverviewSink.serialize_item" +) @patch("platform_plugin_aspects.sinks.course_published.CourseOverviewSink.get_model") @patch("platform_plugin_aspects.sinks.course_published.get_detached_xblock_types") @patch("platform_plugin_aspects.sinks.course_published.get_modulestore") # pytest:disable=unused-argument -def test_course_publish_clickhouse_error(mock_modulestore, mock_detached, mock_overview, mock_serialize_item, caplog): +def test_course_publish_clickhouse_error( + mock_modulestore, mock_detached, mock_overview, mock_serialize_item, caplog +): """ Test the case where a ClickHouse POST fails. """ @@ -103,14 +119,12 @@ def test_course_publish_clickhouse_error(mock_modulestore, mock_detached, mock_o course_overview = fake_course_overview_factory(modified=datetime.now()) mock_overview.return_value.get_from_id.return_value = course_overview - mock_serialize_item.return_value = fake_serialize_fake_course_overview(course_overview) + mock_serialize_item.return_value = fake_serialize_fake_course_overview( + course_overview + ) # This will raise an exception when we try to post to ClickHouse - responses.post( - "https://foo.bar/", - body="Test Bad Request error", - status=400 - ) + responses.post("https://foo.bar/", body="Test Bad Request error", status=400) course = course_str_factory() @@ -119,7 +133,9 @@ def test_course_publish_clickhouse_error(mock_modulestore, mock_detached, mock_o # Make sure our log messages went through. assert "Test Bad Request error" in caplog.text - assert f"Error trying to dump Course Overview {course} to ClickHouse!" in caplog.text + assert ( + f"Error trying to dump Course Overview {course} to ClickHouse!" in caplog.text + ) def test_get_course_last_published(): @@ -130,12 +146,16 @@ def test_get_course_last_published(): course_overview = fake_course_overview_factory(modified=datetime.now()) # Confirm that the string date we get back is a valid date - last_published_date = CourseOverviewSink(None, None).get_course_last_published(course_overview) + last_published_date = CourseOverviewSink(None, None).get_course_last_published( + course_overview + ) dt = datetime.strptime(last_published_date, "%Y-%m-%d %H:%M:%S.%f") assert dt -@responses.activate(registry=OrderedRegistry) # pylint: disable=unexpected-keyword-arg,no-value-for-parameter +@responses.activate( # pylint: disable=unexpected-keyword-arg,no-value-for-parameter + registry=OrderedRegistry +) def test_no_last_published_date(): """ Test that we get a None value back for courses that don't have a modified date. @@ -148,10 +168,7 @@ def test_no_last_published_date(): # should_dump_course will reach out to ClickHouse for the last dump date # we'll fake the response here to have any date, such that we'll exercise # all the "no modified date" code. - responses.get( - "https://foo.bar/", - body="2023-05-03 15:47:39.331024+00:00" - ) + responses.get("https://foo.bar/", body="2023-05-03 15:47:39.331024+00:00") # Confirm that the string date we get back is a valid date sink = CourseOverviewSink(connection_overrides={}, log=logging.getLogger()) @@ -161,20 +178,21 @@ def test_no_last_published_date(): assert reason == "No last modified date in CourseOverview" -@responses.activate(registry=OrderedRegistry) # pylint: disable=unexpected-keyword-arg,no-value-for-parameter +@responses.activate( # pylint: disable=unexpected-keyword-arg,no-value-for-parameter + registry=OrderedRegistry +) def test_should_dump_item(): """ Test that we get the expected results from should_dump_item. """ - course_overview = fake_course_overview_factory(modified=datetime.now().strftime("%Y-%m-%d %H:%M:%S.%f+00:00")) + course_overview = fake_course_overview_factory( + modified=datetime.now().strftime("%Y-%m-%d %H:%M:%S.%f+00:00") + ) # should_dump_course will reach out to ClickHouse for the last dump date # we'll fake the response here to have any date, such that we'll exercise # all the "no modified date" code. - responses.get( - "https://foo.bar/", - body="2023-05-03 15:47:39.331024+00:00" - ) + responses.get("https://foo.bar/", body="2023-05-03 15:47:39.331024+00:00") # Confirm that the string date we get back is a valid date sink = CourseOverviewSink(connection_overrides={}, log=logging.getLogger()) @@ -184,16 +202,17 @@ def test_should_dump_item(): assert "Course has been published since last dump time - " in reason -@responses.activate(registry=OrderedRegistry) # pylint: disable=unexpected-keyword-arg,no-value-for-parameter +@responses.activate( # pylint: disable=unexpected-keyword-arg,no-value-for-parameter + registry=OrderedRegistry +) def test_should_dump_item_not_in_clickhouse(): """ Test that a course gets dumped if it's never been dumped before """ - course_overview = fake_course_overview_factory(modified=datetime.now().strftime("%Y-%m-%d %H:%M:%S.%f+00:00")) - responses.get( - "https://foo.bar/", - body="" + course_overview = fake_course_overview_factory( + modified=datetime.now().strftime("%Y-%m-%d %H:%M:%S.%f+00:00") ) + responses.get("https://foo.bar/", body="") sink = CourseOverviewSink(connection_overrides={}, log=logging.getLogger()) should_dump_course, reason = sink.should_dump_item(course_overview) @@ -202,17 +221,16 @@ def test_should_dump_item_not_in_clickhouse(): assert "Course is not present in ClickHouse" == reason -@responses.activate(registry=OrderedRegistry) # pylint: disable=unexpected-keyword-arg,no-value-for-parameter +@responses.activate( # pylint: disable=unexpected-keyword-arg,no-value-for-parameter + registry=OrderedRegistry +) def test_should_dump_item_no_needs_dump(): """ Test that a course gets dumped if it's never been dumped before """ modified = datetime.now().strftime("%Y-%m-%d %H:%M:%S.%f+00:00") course_overview = fake_course_overview_factory(modified=modified) - responses.get( - "https://foo.bar/", - body=modified - ) + responses.get("https://foo.bar/", body=modified) sink = CourseOverviewSink(connection_overrides={}, log=logging.getLogger()) should_dump_course, reason = sink.should_dump_item(course_overview) @@ -221,7 +239,9 @@ def test_should_dump_item_no_needs_dump(): assert "Course has NOT been published since last dump time - " in reason -@responses.activate(registry=OrderedRegistry) # pylint: disable=unexpected-keyword-arg,no-value-for-parameter +@responses.activate( # pylint: disable=unexpected-keyword-arg,no-value-for-parameter + registry=OrderedRegistry +) def test_course_not_present_in_clickhouse(): """ Test that a course gets dumped if it's never been dumped before @@ -229,10 +249,7 @@ def test_course_not_present_in_clickhouse(): # Request our course last published date course_key = course_str_factory() - responses.get( - "https://foo.bar/", - body="" - ) + responses.get("https://foo.bar/", body="") # Confirm that the string date we get back is a valid date sink = CourseOverviewSink(connection_overrides={}, log=logging.getLogger()) @@ -240,7 +257,9 @@ def test_course_not_present_in_clickhouse(): assert last_published_date is None -@responses.activate(registry=OrderedRegistry) # pylint: disable=unexpected-keyword-arg,no-value-for-parameter +@responses.activate( # pylint: disable=unexpected-keyword-arg,no-value-for-parameter + registry=OrderedRegistry +) def test_get_last_dump_time(): """ Test that we return the expected thing from last dump time. @@ -250,10 +269,7 @@ def test_get_last_dump_time(): # Mock out the response we expect to get from ClickHouse, just a random # datetime in the correct format. - responses.get( - "https://foo.bar/", - body="2023-05-03 15:47:39.331024+00:00" - ) + responses.get("https://foo.bar/", body="2023-05-03 15:47:39.331024+00:00") # Confirm that the string date we get back is a valid date sink = CourseOverviewSink(connection_overrides={}, log=logging.getLogger()) @@ -277,13 +293,17 @@ def test_xblock_tree_structure(mock_modulestore, mock_detached): # Fake the "detached types" list since we can't import it here mock_detached.return_value = mock_detached_xblock_types() - fake_serialized_course_overview = fake_serialize_fake_course_overview(course_overview) + fake_serialized_course_overview = fake_serialize_fake_course_overview( + course_overview + ) sink = XBlockSink(connection_overrides={}, log=MagicMock()) initial_data = {"dump_id": "xyz", "time_last_dumped": "2023-09-05"} results = sink.serialize_item(fake_serialized_course_overview, initial=initial_data) - def _check_tree_location(block, expected_section=0, expected_subsection=0, expected_unit=0): + def _check_tree_location( + block, expected_section=0, expected_subsection=0, expected_unit=0 + ): """ Assert the expected values in certain returned blocks or print useful debug information. """ @@ -332,13 +352,17 @@ def test_xblock_graded_completable_mode(mock_modulestore, mock_detached): # Fake the "detached types" list since we can't import it here mock_detached.return_value = mock_detached_xblock_types() - fake_serialized_course_overview = fake_serialize_fake_course_overview(course_overview) + fake_serialized_course_overview = fake_serialize_fake_course_overview( + course_overview + ) sink = XBlockSink(connection_overrides={}, log=MagicMock()) initial_data = {"dump_id": "xyz", "time_last_dumped": "2023-09-05"} results = sink.serialize_item(fake_serialized_course_overview, initial=initial_data) - def _check_item_serialized_location(block, expected_graded=0, expected_completion_mode="unknown"): + def _check_item_serialized_location( + block, expected_graded=0, expected_completion_mode="unknown" + ): """ Assert the expected values in certain returned blocks or print useful debug information. """ diff --git a/platform_plugin_aspects/tests/test_external_id_sink.py b/platform_plugin_aspects/tests/test_external_id_sink.py index 90a191b..cb19872 100644 --- a/platform_plugin_aspects/tests/test_external_id_sink.py +++ b/platform_plugin_aspects/tests/test_external_id_sink.py @@ -17,4 +17,6 @@ def test_get_queryset(mock_get_queryset): sink.get_queryset() mock_get_queryset.assert_called_once_with(None) - mock_get_queryset.return_value.select_related.assert_called_once_with("user", "external_id_type") + mock_get_queryset.return_value.select_related.assert_called_once_with( + "user", "external_id_type" + ) diff --git a/platform_plugin_aspects/tests/test_settings.py b/platform_plugin_aspects/tests/test_settings.py index 0002b8a..5b937bd 100644 --- a/platform_plugin_aspects/tests/test_settings.py +++ b/platform_plugin_aspects/tests/test_settings.py @@ -49,13 +49,13 @@ def test_production_settings(self): "dashboard_uuid": "1d6bf904-f53f-47fd-b1c9-6cd7e284d286", }, "SUPERSET_EXTRA_FILTERS_FORMAT": [], - 'EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG': { + "EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG": { "url": test_url, "username": test_username, "password": test_password, "database": test_database, - "timeout_secs": test_timeout - } + "timeout_secs": test_timeout, + }, } production_setttings.plugin_settings(settings) self.assertEqual( diff --git a/platform_plugin_aspects/tests/test_signals.py b/platform_plugin_aspects/tests/test_signals.py index ac379b0..f00cf7d 100644 --- a/platform_plugin_aspects/tests/test_signals.py +++ b/platform_plugin_aspects/tests/test_signals.py @@ -1,11 +1,16 @@ """ Tests for signal handlers. """ + from unittest.mock import Mock, patch from django.test import TestCase -from platform_plugin_aspects.signals import on_externalid_saved, on_user_retirement, receive_course_publish +from platform_plugin_aspects.signals import ( + on_externalid_saved, + on_user_retirement, + receive_course_publish, +) from platform_plugin_aspects.sinks.external_id_sink import ExternalIdSink from platform_plugin_aspects.sinks.user_retire import UserRetirementSink diff --git a/platform_plugin_aspects/tests/test_tasks.py b/platform_plugin_aspects/tests/test_tasks.py index 7ab9822..79c2825 100644 --- a/platform_plugin_aspects/tests/test_tasks.py +++ b/platform_plugin_aspects/tests/test_tasks.py @@ -1,6 +1,7 @@ """ Tests for the tasks module. """ + import unittest from unittest.mock import MagicMock, patch @@ -37,9 +38,7 @@ def test_dump_data_to_clickhouse(self, mock_celery_log, mock_import_module): mock_Sink_instance.dump.assert_called_once_with("object_id") @patch("platform_plugin_aspects.tasks.import_module") - def test_dump_data_to_clickhouse_disabled_sink( - self, mock_import_module - ): + def test_dump_data_to_clickhouse_disabled_sink(self, mock_import_module): # Mock the required objects and methods mock_Sink_class = MagicMock() mock_Sink_class.is_enabled.return_value = False diff --git a/platform_plugin_aspects/tests/test_user_retire.py b/platform_plugin_aspects/tests/test_user_retire.py index 6612c76..9fb4a68 100644 --- a/platform_plugin_aspects/tests/test_user_retire.py +++ b/platform_plugin_aspects/tests/test_user_retire.py @@ -1,6 +1,7 @@ """ Tests for the user_retire sinks. """ + import logging from unittest.mock import patch diff --git a/platform_plugin_aspects/tests/test_utils.py b/platform_plugin_aspects/tests/test_utils.py index 8d751a7..01cd44d 100644 --- a/platform_plugin_aspects/tests/test_utils.py +++ b/platform_plugin_aspects/tests/test_utils.py @@ -8,7 +8,12 @@ from django.conf import settings -from platform_plugin_aspects.utils import get_ccx_courses, get_model, generate_superset_context +from platform_plugin_aspects.utils import ( + generate_superset_context, + get_ccx_courses, + get_model, +) + User = namedtuple("User", ["username"]) @@ -79,17 +84,16 @@ def test_get_model_missing_model_config(self): def test_get_ccx_courses(self, mock_get_model): mock_get_model.return_value = mock_model = Mock() - get_ccx_courses('dummy_key') + get_ccx_courses("dummy_key") - mock_model.objects.filter.assert_called_once_with(course_id='dummy_key') + mock_model.objects.filter.assert_called_once_with(course_id="dummy_key") @patch.object(settings, "FEATURES", {"CUSTOM_COURSES_EDX": False}) def test_get_ccx_courses_feature_disabled(self): - courses = get_ccx_courses('dummy_key') + courses = get_ccx_courses("dummy_key") self.assertEqual(list(courses), []) - @patch("platform_plugin_aspects.utils.generate_guest_token") def test_generate_superset_context(self, mock_generate_guest_token): """ @@ -112,7 +116,9 @@ def test_generate_superset_context(self, mock_generate_guest_token): self.assertNotIn("exception", context) @patch("platform_plugin_aspects.utils.SupersetClient") - def test_generate_superset_context_with_superset_client_exception(self, mock_superset_client): + def test_generate_superset_context_with_superset_client_exception( + self, mock_superset_client + ): """ Test generate_superset_context """ @@ -131,7 +137,9 @@ def test_generate_superset_context_with_superset_client_exception(self, mock_sup @patch("platform_plugin_aspects.utils.SupersetClient") @patch("platform_plugin_aspects.utils.get_current_user") - def test_generate_superset_context_succesful(self, mock_get_current_user, mock_superset_client): + def test_generate_superset_context_succesful( + self, mock_get_current_user, mock_superset_client + ): """ Test generate_superset_context """ diff --git a/platform_plugin_aspects/utils.py b/platform_plugin_aspects/utils.py index cb1732f..d17c767 100644 --- a/platform_plugin_aspects/utils.py +++ b/platform_plugin_aspects/utils.py @@ -17,9 +17,7 @@ def generate_superset_context( # pylint: disable=dangerous-default-value - context, - dashboard_uuid="", - filters=[] + context, dashboard_uuid="", filters=[] ): """ Update context with superset token and dashboard id. diff --git a/platform_plugin_aspects/waffle.py b/platform_plugin_aspects/waffle.py index 4ebba5e..cf55bb1 100644 --- a/platform_plugin_aspects/waffle.py +++ b/platform_plugin_aspects/waffle.py @@ -1,4 +1,5 @@ """ Configuration for event sink clickhouse. """ + WAFFLE_FLAG_NAMESPACE = "event_sink_clickhouse" diff --git a/setup.cfg b/setup.cfg index c1422d2..90e26ed 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,11 +1,11 @@ [isort] -include_trailing_comma = True indent = ' ' -line_length = 120 -multi_line_output = 3 skip= migrations profile = black +multi_line_output = 3 +include_trailing_comma = True +line_length = 88 [wheel] universal = 1 diff --git a/setup.py b/setup.py index f173ec7..76bf498 100755 --- a/setup.py +++ b/setup.py @@ -19,11 +19,10 @@ def get_version(*file_paths): """ filename = os.path.join(os.path.dirname(__file__), *file_paths) version_file = open(filename, encoding="utf8").read() - version_match = re.search(r"^__version__ = ['\"]([^'\"]*)['\"]", - version_file, re.M) + version_match = re.search(r"^__version__ = ['\"]([^'\"]*)['\"]", version_file, re.M) if version_match: return version_match.group(1) - raise RuntimeError('Unable to find version string.') + raise RuntimeError("Unable to find version string.") def load_requirements(*requirements_paths): @@ -46,14 +45,14 @@ def check_name_consistent(package): with extras we don't constrain it without mentioning the extras (since that too would interfere with matching constraints.) """ - canonical = package.lower().replace('_', '-').split('[')[0] + canonical = package.lower().replace("_", "-").split("[")[0] seen_spelling = by_canonical_name.get(canonical) if seen_spelling is None: by_canonical_name[canonical] = package elif seen_spelling != package: raise Exception( f'Encountered both "{seen_spelling}" and "{package}" in requirements ' - 'and constraints files; please use just one or the other.' + "and constraints files; please use just one or the other." ) requirements = {} @@ -67,7 +66,9 @@ def check_name_consistent(package): % (re_package_name_base_chars, re_package_name_base_chars) ) - def add_version_constraint_or_raise(current_line, current_requirements, add_if_not_present): + def add_version_constraint_or_raise( + current_line, current_requirements, add_if_not_present + ): regex_match = requirement_line_regex.match(current_line) if regex_match: package = regex_match.group(1) @@ -76,11 +77,16 @@ def add_version_constraint_or_raise(current_line, current_requirements, add_if_n existing_version_constraints = current_requirements.get(package, None) # It's fine to add constraints to an unconstrained package, # but raise an error if there are already constraints in place. - if existing_version_constraints and existing_version_constraints != version_constraints: - raise BaseException(f'Multiple constraint definitions found for {package}:' - f' "{existing_version_constraints}" and "{version_constraints}".' - f'Combine constraints into one location with {package}' - f'{existing_version_constraints},{version_constraints}.') + if ( + existing_version_constraints + and existing_version_constraints != version_constraints + ): + raise BaseException( + f"Multiple constraint definitions found for {package}:" + f' "{existing_version_constraints}" and "{version_constraints}".' + f"Combine constraints into one location with {package}" + f"{existing_version_constraints},{version_constraints}." + ) if add_if_not_present or package in current_requirements: current_requirements[package] = version_constraints @@ -91,8 +97,12 @@ def add_version_constraint_or_raise(current_line, current_requirements, add_if_n for line in reqs: if is_requirement(line): add_version_constraint_or_raise(line, requirements, True) - if line and line.startswith('-c') and not line.startswith('-c http'): - constraint_files.add(os.path.dirname(path) + '/' + line.split('#')[0].replace('-c', '').strip()) + if line and line.startswith("-c") and not line.startswith("-c http"): + constraint_files.add( + os.path.dirname(path) + + "/" + + line.split("#")[0].replace("-c", "").strip() + ) # process constraint files: add constraints to existing requirements for constraint_file in constraint_files: @@ -102,7 +112,9 @@ def add_version_constraint_or_raise(current_line, current_requirements, add_if_n add_version_constraint_or_raise(line, requirements, False) # process back into list of pkg><=constraints strings - constrained_requirements = [f'{pkg}{version or ""}' for (pkg, version) in sorted(requirements.items())] + constrained_requirements = [ + f'{pkg}{version or ""}' for (pkg, version) in sorted(requirements.items()) + ] return constrained_requirements @@ -114,55 +126,60 @@ def is_requirement(line): bool: True if the line is not blank, a comment, a URL, or an included file """ - return line and line.strip() and not line.startswith(("-r", "#", "-e", "git+", "-c")) + return ( + line and line.strip() and not line.startswith(("-r", "#", "-e", "git+", "-c")) + ) -VERSION = get_version('platform_plugin_aspects', '__init__.py') +VERSION = get_version("platform_plugin_aspects", "__init__.py") -if sys.argv[-1] == 'tag': +if sys.argv[-1] == "tag": print("Tagging the version on github:") os.system("git tag -a %s -m 'version %s'" % (VERSION, VERSION)) os.system("git push --tags") sys.exit() -README = open(os.path.join(os.path.dirname(__file__), 'README.rst'), encoding="utf8").read() -CHANGELOG = open(os.path.join(os.path.dirname(__file__), 'CHANGELOG.rst'), encoding="utf8").read() +README = open( + os.path.join(os.path.dirname(__file__), "README.rst"), encoding="utf8" +).read() +CHANGELOG = open( + os.path.join(os.path.dirname(__file__), "CHANGELOG.rst"), encoding="utf8" +).read() setup( - name='platform-plugin-aspects', + name="platform-plugin-aspects", version=VERSION, description="""Aspects plugins for edx-platform""", - long_description=README + '\n\n' + CHANGELOG, - author='Open edX Project', - author_email='oscm@openedx.org', - url='https://github.com/openedx/platform-plugin-aspects', + long_description=README + "\n\n" + CHANGELOG, + author="Open edX Project", + author_email="oscm@openedx.org", + url="https://github.com/openedx/platform-plugin-aspects", packages=find_packages( - include=['platform_plugin_aspects', 'platform_plugin_aspects.*'], + include=["platform_plugin_aspects", "platform_plugin_aspects.*"], exclude=["*tests"], ), - include_package_data=True, - install_requires=load_requirements('requirements/base.in'), + install_requires=load_requirements("requirements/base.in"), python_requires=">=3.8", license="AGPL 3.0", zip_safe=False, - keywords='Python edx', + keywords="Python edx", classifiers=[ - 'Development Status :: 3 - Alpha', - 'Framework :: Django', - 'Framework :: Django :: 3.2', - 'Intended Audience :: Developers', - 'License :: OSI Approved :: GNU Affero General Public License v3 or later (AGPLv3+)', - 'Natural Language :: English', - 'Programming Language :: Python :: 3', - 'Programming Language :: Python :: 3.8', + "Development Status :: 3 - Alpha", + "Framework :: Django", + "Framework :: Django :: 3.2", + "Intended Audience :: Developers", + "License :: OSI Approved :: GNU Affero General Public License v3 or later (AGPLv3+)", + "Natural Language :: English", + "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.8", ], entry_points={ - 'lms.djangoapp': [ - 'platform_plugin_aspects = platform_plugin_aspects.apps:PlatformPluginAspectsConfig', + "lms.djangoapp": [ + "platform_plugin_aspects = platform_plugin_aspects.apps:PlatformPluginAspectsConfig", ], - 'cms.djangoapp': [ - 'platform_plugin_aspects = platform_plugin_aspects.apps:PlatformPluginAspectsConfig', + "cms.djangoapp": [ + "platform_plugin_aspects = platform_plugin_aspects.apps:PlatformPluginAspectsConfig", ], }, ) diff --git a/test_settings.py b/test_settings.py index d1fc90b..9fb85b1 100644 --- a/test_settings.py +++ b/test_settings.py @@ -1,4 +1,3 @@ - """ These settings are here to use during tests, because django requires them. @@ -26,9 +25,7 @@ } -INSTALLED_APPS = ( - "platform_plugin_aspects", -) +INSTALLED_APPS = ("platform_plugin_aspects",) EVENT_SINK_CLICKHOUSE_MODEL_CONFIG = { "user_profile": { @@ -38,27 +35,29 @@ "course_overviews": { "module": "openedx.core.djangoapps.content.course_overviews.models", "model": "CourseOverview", - } + }, } EVENT_SINK_CLICKHOUSE_COURSE_OVERVIEWS_ENABLED = True FEATURES = { - 'CUSTOM_COURSES_EDX': True, + "CUSTOM_COURSES_EDX": True, } DEBUG = True -TEMPLATES = [{ - 'BACKEND': 'django.template.backends.django.DjangoTemplates', - 'APP_DIRS': False, - 'OPTIONS': { - 'context_processors': [ - 'django.contrib.auth.context_processors.auth', # this is required for admin - 'django.contrib.messages.context_processors.messages', # this is required for admin - ], - }, -}] +TEMPLATES = [ + { + "BACKEND": "django.template.backends.django.DjangoTemplates", + "APP_DIRS": False, + "OPTIONS": { + "context_processors": [ + "django.contrib.auth.context_processors.auth", # this is required for admin + "django.contrib.messages.context_processors.messages", # this is required for admin + ], + }, + } +] ASPECTS_INSTRUCTOR_DASHBOARD_UUID = "test-dashboard-uuid" diff --git a/test_utils/helpers.py b/test_utils/helpers.py index 682985c..8e09fa1 100644 --- a/test_utils/helpers.py +++ b/test_utils/helpers.py @@ -20,30 +20,33 @@ COURSE_RUN = "2023_Fall" FakeCourse = namedtuple("FakeCourse", ["id"]) -FakeCourseOverview = namedtuple("FakeCourseOverview", [ - # Key fields we keep at the top level - "id", - "org", - "display_name", - "start", - "end", - "enrollment_start", - "enrollment_end", - "self_paced", - "created", - "modified", - # Fields we stuff in JSON - "advertised_start", - "announcement", - "lowest_passing_grade", - "invitation_only", - "max_student_enrollments_allowed", - "effort", - "enable_proctored_exams", - "entrance_exam_enabled", - "external_id", - "language", -]) +FakeCourseOverview = namedtuple( + "FakeCourseOverview", + [ + # Key fields we keep at the top level + "id", + "org", + "display_name", + "start", + "end", + "enrollment_start", + "enrollment_end", + "self_paced", + "created", + "modified", + # Fields we stuff in JSON + "advertised_start", + "announcement", + "lowest_passing_grade", + "invitation_only", + "max_student_enrollments_allowed", + "effort", + "enable_proctored_exams", + "entrance_exam_enabled", + "external_id", + "language", + ], +) FakeUser = namedtuple("FakeUser", ["id"]) @@ -52,7 +55,10 @@ class FakeXBlock: """ Fakes the parameters of an XBlock that we care about. """ - def __init__(self, identifier, block_type="vertical", graded=False, completion_mode="unknown"): + + def __init__( + self, identifier, block_type="vertical", graded=False, completion_mode="unknown" + ): self.block_type = block_type self.scope_ids = Mock() self.scope_ids.usage_id.course_key = course_key_factory() @@ -91,8 +97,10 @@ def block_usage_locator_factory(): """ Create a BlockUsageLocator with a random id. """ - block_id = ''.join(random.choices(string.ascii_letters, k=10)) - return BlockUsageLocator(course_key_factory(), block_type="category", block_id=block_id, deprecated=True) + block_id = "".join(random.choices(string.ascii_letters, k=10)) + return BlockUsageLocator( + course_key_factory(), block_type="category", block_id=block_id, deprecated=True + ) def fake_course_overview_factory(modified=None): @@ -102,26 +110,26 @@ def fake_course_overview_factory(modified=None): Modified is overridable, but can also be None. """ return FakeCourseOverview( - course_key_factory(), # id - ORG, # org - "Test Course", # display_name - datetime.now() - timedelta(days=90), # start - datetime.now() + timedelta(days=90), # end - datetime.now() - timedelta(days=90), # enrollment_start - datetime.now() + timedelta(days=90), # enrollment_end - False, # self_paced + course_key_factory(), # id + ORG, # org + "Test Course", # display_name + datetime.now() - timedelta(days=90), # start + datetime.now() + timedelta(days=90), # end + datetime.now() - timedelta(days=90), # enrollment_start + datetime.now() + timedelta(days=90), # enrollment_end + False, # self_paced datetime.now() - timedelta(days=180), # created - modified, # modified - datetime.now() - timedelta(days=90), # advertised_start - datetime.now() - timedelta(days=90), # announcement - 71.05, # lowest_passing_grade - False, # invitation_only - 1000, # max_student_enrollments_allowed - "Pretty easy", # effort - False, # enable_proctored_exams - True, # entrance_exam_enabled - "abcd1234", # external_id - "Polish" # language + modified, # modified + datetime.now() - timedelta(days=90), # advertised_start + datetime.now() - timedelta(days=90), # announcement + 71.05, # lowest_passing_grade + False, # invitation_only + 1000, # max_student_enrollments_allowed + "Pretty easy", # effort + False, # enable_proctored_exams + True, # entrance_exam_enabled + "abcd1234", # external_id + "Polish", # language ) @@ -173,7 +181,7 @@ def mock_detached_xblock_types(): Mock the return results of xmodule.modulestore.store_utilities.DETACHED_XBLOCK_TYPES """ # Current values as of 2023-05-01 - return {'static_tab', 'about', 'course_info'} + return {"static_tab", "about", "course_info"} def get_clickhouse_http_params(): @@ -183,12 +191,12 @@ def get_clickhouse_http_params(): overview_params = { "input_format_allow_errors_num": 1, "input_format_allow_errors_ratio": 0.1, - "query": "INSERT INTO cool_data.course_overviews FORMAT CSV" + "query": "INSERT INTO cool_data.course_overviews FORMAT CSV", } blocks_params = { "input_format_allow_errors_num": 1, "input_format_allow_errors_ratio": 0.1, - "query": "INSERT INTO cool_data.course_blocks FORMAT CSV" + "query": "INSERT INTO cool_data.course_blocks FORMAT CSV", } return overview_params, blocks_params @@ -200,7 +208,9 @@ def course_factory(): """ # Create a base block top_block = FakeXBlock("top", block_type="course") - course = [top_block, ] + course = [ + top_block, + ] # Create a few sections for i in range(3): @@ -244,6 +254,7 @@ def check_overview_csv_matcher(course_overview): This is a matcher for the "responses" library. It returns a function that actually does the matching. """ + def match(request): body = request.body @@ -268,14 +279,27 @@ def match(request): # real JSON, compare values dumped_json = json.loads(row[8]) - assert dumped_json["advertised_start"] == str(course_overview.advertised_start) + assert dumped_json["advertised_start"] == str( + course_overview.advertised_start + ) assert dumped_json["announcement"] == str(course_overview.announcement) - assert dumped_json["lowest_passing_grade"] == float(course_overview.lowest_passing_grade) + assert dumped_json["lowest_passing_grade"] == float( + course_overview.lowest_passing_grade + ) assert dumped_json["invitation_only"] == course_overview.invitation_only - assert dumped_json["max_student_enrollments_allowed"] == course_overview.max_student_enrollments_allowed + assert ( + dumped_json["max_student_enrollments_allowed"] + == course_overview.max_student_enrollments_allowed + ) assert dumped_json["effort"] == course_overview.effort - assert dumped_json["enable_proctored_exams"] == course_overview.enable_proctored_exams - assert dumped_json["entrance_exam_enabled"] == course_overview.entrance_exam_enabled + assert ( + dumped_json["enable_proctored_exams"] + == course_overview.enable_proctored_exams + ) + assert ( + dumped_json["entrance_exam_enabled"] + == course_overview.entrance_exam_enabled + ) assert dumped_json["external_id"] == course_overview.external_id assert dumped_json["language"] == course_overview.language @@ -286,6 +310,7 @@ def match(request): except EOFError as e: return False, f"Mismatch in row {i}: {e}" return True, "" + return match @@ -296,6 +321,7 @@ def check_block_csv_matcher(course): This is a matcher for the "responses" library. It returns a function that actually does the matching. """ + def match(request): body = request.body.decode("utf-8") lines = body.split("\n")[:-1] @@ -319,9 +345,9 @@ def match(request): assert row[3] == block.display_name_with_default block_json_data = { - 'course': block.location.course, - 'run': block.location.run, - 'block_type': str(block.block_type), + "course": block.location.course, + "run": block.location.run, + "block_type": str(block.block_type), } csv_json = json.loads(row[4]) @@ -333,4 +359,5 @@ def match(request): except AssertionError as e: return False, f"Mismatch in row {i}: {e}" return True, "" + return match diff --git a/tox.ini b/tox.ini index 9afe852..e57b6ff 100644 --- a/tox.ini +++ b/tox.ini @@ -70,13 +70,15 @@ allowlist_externals = make rm touch + black deps = -r{toxinidir}/requirements/quality.txt commands = - pylint platform_plugin_aspects tests test_utils manage.py setup.py - pycodestyle platform_plugin_aspects tests manage.py setup.py - pydocstyle platform_plugin_aspects tests manage.py setup.py - isort --check-only --diff tests test_utils platform_plugin_aspects manage.py setup.py + pylint platform_plugin_aspects test_utils manage.py setup.py + pycodestyle platform_plugin_aspects manage.py setup.py + pydocstyle platform_plugin_aspects manage.py setup.py + isort --check-only --diff test_utils platform_plugin_aspects manage.py setup.py + black --check --diff platform_plugin_aspects test_utils manage.py setup.py make selfcheck [testenv:pii_check] @@ -86,4 +88,3 @@ deps = -r{toxinidir}/requirements/test.txt commands = code_annotations django_find_annotations --config_file .pii_annotations.yml --lint --report --coverage -