From 2948b512cde9d3e3d108370ecfb00e39c158f676 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Tue, 29 Sep 2026 11:15:17 -0400 Subject: [PATCH 1/9] Update pre-commit hooks and adapt to ruff 0.16 (#3993) --- .pre-commit-config.yaml | 8 +-- docs/articles-cdn-purge.md | 7 +- docs/how-to/articles-to-news.md | 67 ++++++++++--------- learning_resources/data/README-topics.md | 46 +++++++------ learning_resources/etl/utils.py | 4 +- .../0120_view_event_uuid_unique_index.py | 8 ++- learning_resources/utils.py | 2 +- learning_resources/utils_test.py | 12 ++-- learning_resources_search/api.py | 2 +- pyproject.toml | 7 +- uv.lock | 48 ++++++------- vector_search/views.py | 8 +-- 12 files changed, 118 insertions(+), 101 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index edaf1ca459..5c26b4fd94 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -53,7 +53,7 @@ repos: pass_filenames: false always_run: true - repo: https://github.com/scop/pre-commit-shfmt - rev: v3.13.1-1 + rev: v3.14.1-1 hooks: - id: shfmt - repo: https://github.com/adrienverge/yamllint.git @@ -90,7 +90,7 @@ repos: - "config/keycloak/realms/ol-local-realm.json" additional_dependencies: ["gibberish-detector"] - repo: https://github.com/astral-sh/ruff-pre-commit - rev: "v0.15.21" + rev: "v0.16.8" hooks: - id: ruff-format - id: ruff @@ -118,12 +118,12 @@ repos: exclude: node_modules/ require_serial: false - repo: https://github.com/shellcheck-py/shellcheck-py - rev: v0.11.0.1 + rev: v0.11.0.1-1 hooks: - id: shellcheck args: ["--severity=warning"] - repo: https://github.com/zizmorcore/zizmor-pre-commit - rev: v1.29.0 + rev: v1.30.1 hooks: - id: zizmor args: [--no-progress, --min-severity=medium, --min-confidence=medium] diff --git a/docs/articles-cdn-purge.md b/docs/articles-cdn-purge.md index 985d344722..41c134ef5a 100644 --- a/docs/articles-cdn-purge.md +++ b/docs/articles-cdn-purge.md @@ -150,7 +150,7 @@ article = Article.objects.create( title="New Article", content={"type": "doc", "content": []}, is_published=True, - user=some_user + user=some_user, ) # CDN purge is automatically queued! @@ -168,7 +168,7 @@ You can manually trigger CDN purges: from articles.tasks import ( fastly_purge_relative_url, fastly_purge_articles_list, - fastly_full_purge + fastly_full_purge, ) # Purge a specific URL immediately (blocking) @@ -192,7 +192,7 @@ For backwards compatibility, the following aliases are available but deprecated: # Old names (still work but discouraged) from articles.tasks import ( queue_fastly_purge_articles_list, # Use fastly_purge_articles_list - queue_fastly_full_purge, # Use fastly_full_purge + queue_fastly_full_purge, # Use fastly_full_purge ) ``` @@ -260,6 +260,7 @@ All CDN purge operations are logged using Python's standard logging: ```python import logging + logger = logging.getLogger("fastly_purge") ``` diff --git a/docs/how-to/articles-to-news.md b/docs/how-to/articles-to-news.md index 0ab3bfe08b..22fb05507b 100644 --- a/docs/how-to/articles-to-news.md +++ b/docs/how-to/articles-to-news.md @@ -144,23 +144,27 @@ Article.objects.filter(is_published=True) ### 2. Transform ```python -[{ - 'title': 'MIT Learn Articles', - 'url': '/articles', - 'feed_type': 'news', - 'items': [{ - 'guid': 'article-1', - 'title': 'My Article', - 'url': '/articles/my-article', - 'summary': 'First 500 chars...', - 'content': 'Full text...', - 'detail': { - 'authors': ['John Doe'], - 'topics': [], - 'publish_date': '2024-01-01T00:00:00Z', - } - }] -}] +[ + { + "title": "MIT Learn Articles", + "url": "/articles", + "feed_type": "news", + "items": [ + { + "guid": "article-1", + "title": "My Article", + "url": "/articles/my-article", + "summary": "First 500 chars...", + "content": "Full text...", + "detail": { + "authors": ["John Doe"], + "topics": [], + "publish_date": "2024-01-01T00:00:00Z", + }, + } + ], + } +] ``` ### 3. Load @@ -190,21 +194,20 @@ The `extract_text_from_content()` function needs customization based on your JSO ```python def extract_text_from_content(content_json: dict) -> str: # For Draft.js - blocks = content_json.get('blocks', []) - return ' '.join([block.get('text', '') for block in blocks]) + blocks = content_json.get("blocks", []) + return " ".join([block.get("text", "") for block in blocks]) + # For ProseMirror def walk_nodes(node): - if node.get('type') == 'text': - return node.get('text', '') - children = node.get('content', []) - return ' '.join(walk_nodes(child) for child in children) + if node.get("type") == "text": + return node.get("text", "") + children = node.get("content", []) + return " ".join(walk_nodes(child) for child in children) + return walk_nodes(content_json) # For EditorJS - blocks = content_json.get('blocks', []) - return ' '.join([ - block.get('data', {}).get('text', '') - for block in blocks - ]) + blocks = content_json.get("blocks", []) + return " ".join([block.get("data", {}).get("text", "") for block in blocks]) ``` ### 2. Add Image Support @@ -235,7 +238,8 @@ If you add topics to your Article model: ```python class Article(TimestampedModel): # ... existing fields ... - topics = models.ManyToManyField('Topic') + topics = models.ManyToManyField("Topic") + # In transform_items: entry = { @@ -294,7 +298,7 @@ article = Article.objects.create( title="Test Article", content={"blocks": [{"text": "Test content"}]}, user=user, - is_published=True + is_published=True, ) ``` @@ -317,6 +321,7 @@ result = pipelines.articles_news_etl() # Check results from news_events.models import FeedSource + source = FeedSource.objects.get(title="MIT Learn Articles") print(f"Found {source.feed_items.count()} articles in news feed") ``` @@ -340,6 +345,7 @@ print(f"Found {source.feed_items.count()} articles in news feed") 3. **Check for errors:** ```python from news_events.tasks import get_articles_news + get_articles_news() # Run synchronously to see errors ``` @@ -350,6 +356,7 @@ print(f"Found {source.feed_items.count()} articles in news feed") - Add debug logging: ```python import logging + log = logging.getLogger(__name__) log.info(f"Content structure: {content_json}") ``` diff --git a/learning_resources/data/README-topics.md b/learning_resources/data/README-topics.md index 1b73b287fd..6a7b49b96a 100644 --- a/learning_resources/data/README-topics.md +++ b/learning_resources/data/README-topics.md @@ -55,36 +55,40 @@ import csv import yaml from pathlib import Path + def _process_topic(topic, offeror_code): - if not topic["mappings"]: - topic["mappings"] = {} + if not topic["mappings"]: + topic["mappings"] = {} + + if not topic["mappings"] or offeror_code not in topic["mappings"]: + topic["mappings"][offeror_code] = [] - if not topic["mappings"] or offeror_code not in topic["mappings"]: - topic["mappings"][offeror_code] = [] + for csv_topic in csv_data: + if ( + topic["name"] == csv_topic[1] or topic["name"] == csv_topic[2] + ) and csv_topic[0] not in topic["mappings"][offeror_code]: + topic["mappings"][offeror_code].append(csv_topic[0].strip()) - for csv_topic in csv_data: - if (topic["name"] == csv_topic[1] or topic["name"] == csv_topic[2]) and csv_topic[0] not in topic["mappings"][offeror_code]: - topic["mappings"][offeror_code].append(csv_topic[0].strip()) + if "children" in topic and topic["children"] and len(topic["children"]) > 0: + for child_topic in topic["children"]: + _process_topic(child_topic, offeror_code) if child_topic else None - if "children" in topic and topic["children"] and len(topic["children"]) > 0: - for child_topic in topic["children"]: - _process_topic(child_topic, offeror_code) if child_topic else None def process_topics(topics_file_loc, csv_loc, offeror_code, output_location): - with Path(topics_file_loc).open() as yaml_input_file: - topic_data = yaml.safe_load(yaml_input_file.read()) + with Path(topics_file_loc).open() as yaml_input_file: + topic_data = yaml.safe_load(yaml_input_file.read()) - with Path(csv_loc).open() as csv_file: - csv_reader = csv.reader(csv_file) - csv_data = [] - for data in csv_reader: - csv_data.append(data) + with Path(csv_loc).open() as csv_file: + csv_reader = csv.reader(csv_file) + csv_data = [] + for data in csv_reader: + csv_data.append(data) - for topic in topic_data["topics"]: - _process_topic(topic, offeror_code) + for topic in topic_data["topics"]: + _process_topic(topic, offeror_code) - with Path(output_location).open("w") as yaml_output_file: - yaml.dump(topic_data, yaml_output_file) + with Path(output_location).open("w") as yaml_output_file: + yaml.dump(topic_data, yaml_output_file) ``` You can open a Django shell or a notebook and paste that in, then run `process_topics()` to process your datafile. The result of this will be written to the output location specified. diff --git a/learning_resources/etl/utils.py b/learning_resources/etl/utils.py index e4c3ec7e1c..731b59db75 100644 --- a/learning_resources/etl/utils.py +++ b/learning_resources/etl/utils.py @@ -865,7 +865,7 @@ def pdf_is_valid(pdf_path: Path) -> bool: if len(reader.pages) > 0: reader.pages[0].extract_text() return True - except Exception: # noqa: BLE001 + except Exception: # warning, not exception: the caller raises InvalidPDFError and # process_olx_path emits the single Sentry event for this file log.warning("PDF validation error for %s", pdf_path, exc_info=True) @@ -1052,7 +1052,7 @@ def get_title_for_content( return Path(source_path).stem.replace("_", " ").replace("-", " ").title() -def _build_result( # noqa: PLR0913 +def _build_result( # noqa: PLR0913, PLR0917 olx_path, metadata: dict, key: str, run, video_srt_metadata, content_dict: dict ) -> dict: """Build the final result dictionary.""" diff --git a/learning_resources/migrations/0120_view_event_uuid_unique_index.py b/learning_resources/migrations/0120_view_event_uuid_unique_index.py index e555527d01..f972353404 100644 --- a/learning_resources/migrations/0120_view_event_uuid_unique_index.py +++ b/learning_resources/migrations/0120_view_event_uuid_unique_index.py @@ -35,9 +35,11 @@ class Migration(migrations.Migration): # leftover so a rerun rebuilds and enforces uniqueness f"DROP INDEX CONCURRENTLY IF EXISTS {INDEX_NAME}", # Partial: the legacy NULL rows need no index entries - f"CREATE UNIQUE INDEX CONCURRENTLY {INDEX_NAME}" - f" ON {TABLE_NAME} (event_uuid)" - f" WHERE event_uuid IS NOT NULL", + ( + f"CREATE UNIQUE INDEX CONCURRENTLY {INDEX_NAME}" + f" ON {TABLE_NAME} (event_uuid)" + f" WHERE event_uuid IS NOT NULL" + ), ], reverse_sql=f"DROP INDEX CONCURRENTLY IF EXISTS {INDEX_NAME}", ), diff --git a/learning_resources/utils.py b/learning_resources/utils.py index 6f631c50fb..b3dcd32656 100644 --- a/learning_resources/utils.py +++ b/learning_resources/utils.py @@ -510,7 +510,7 @@ def offeror_delete_actions(offeror: LearningResourceOfferor): hook.offeror_delete(offeror=offeror) -def _walk_topic_map(topics: list, parent: None | LearningResourceTopic = None) -> None: +def _walk_topic_map(topics: list, parent: LearningResourceTopic | None = None) -> None: """ Walk the topic map provided and create topic records accordingly. diff --git a/learning_resources/utils_test.py b/learning_resources/utils_test.py index 5f57f1e3c8..f2c5db4262 100644 --- a/learning_resources/utils_test.py +++ b/learning_resources/utils_test.py @@ -1247,8 +1247,10 @@ def test_is_loggable_missing_content_id(edx_module_id, loggable): ("block-v1:34819-FA25 18.01L+canvas+type@g085f027c+block@2-dot-11", True), ("block-v1:33414-21H.363+canvas+type@+block@gfaf809b", True), ( - "block-v1:28770-15.060_FA24+canvas" - "+type@Discrete+Nonlinear_Optimization+block@x", + ( + "block-v1:28770-15.060_FA24+canvas" + "+type@Discrete+Nonlinear_Optimization+block@x" + ), True, ), ("asset-v1:MITxT+16.00x+0T2026+type@asset+block@lec_\t.srt", True), @@ -1260,8 +1262,10 @@ def test_is_loggable_missing_content_id(edx_module_id, loggable): ("block-v1:X+type@library_content+block@y", True), # Never-content block types are rejected (case-insensitive) ( - "block-v1:MITxT+18.03.2x+1T2025+type@discussion" - "+block@discussion_recitation13-tab3", + ( + "block-v1:MITxT+18.03.2x+1T2025+type@discussion" + "+block@discussion_recitation13-tab3" + ), False, ), ("block-v1:X+type@DISCUSSION+block@y", False), diff --git a/learning_resources_search/api.py b/learning_resources_search/api.py index c7a945abb7..0a18bfee8c 100644 --- a/learning_resources_search/api.py +++ b/learning_resources_search/api.py @@ -1123,7 +1123,7 @@ def get_similar_topics( return list(dict(counter.most_common(num_topics)).keys()) -def get_similar_resources( # noqa: PLR0913 +def get_similar_resources( # noqa: PLR0913, PLR0917 value_doc: dict, num_resources: int, min_term_freq: int, diff --git a/pyproject.toml b/pyproject.toml index 4e23a29323..e9fe48d612 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -148,7 +148,7 @@ dev = [ "pytest-mock>=3.10.0,<4", "pytest-repeat>=0.9.4", "responses>=0.25.0,<0.26", - "ruff==0.16.3", + "ruff==0.16.8", "safety>=3.0.0,<4", "semantic-version>=2.10.0,<3", "freezegun>=1.4.0,<2", @@ -193,7 +193,6 @@ lint.select = [ "C4", # flake8-comprehensions "C90", # mccabe # "COM", # flake8-commas - "CPY", # flake8-copyright "D", # pydocstyle "DJ", # flake8-django "DTZ", # flake8-datetimez @@ -275,8 +274,8 @@ inline-quotes = "double" "django.contrib.auth.models.User".msg = "use get_user_model() or settings.AUTH_USER_MODEL" [tool.ruff.lint.per-file-ignores] -"*_test.py" = ["ARG001", "E501", "S101", "PLR2004"] -"test_*.py" = ["ARG001", "E501", "S101", "PLR2004"] +"*_test.py" = ["ARG001", "E501", "S101", "PLR2004", "PLR0917"] +"test_*.py" = ["ARG001", "E501", "S101", "PLR2004", "PLR0917"] "**/migrations/**" = ["ARG001"] [tool.bumpversion] diff --git a/uv.lock b/uv.lock index 67e4f75e55..5a69163e7b 100644 --- a/uv.lock +++ b/uv.lock @@ -7,10 +7,10 @@ exclude-newer = "0001-01-01T00:00:00Z" # This has no effect and is included for exclude-newer-span = "P7D" [options.exclude-newer-package] -mitol-django-common = { timestamp = "0001-01-01T00:00:00Z", span = "PT0S" } +mitol-drf-lint = { timestamp = "0001-01-01T00:00:00Z", span = "PT0S" } mitol-django-observability = { timestamp = "0001-01-01T00:00:00Z", span = "PT0S" } mitol-django-scim = { timestamp = "0001-01-01T00:00:00Z", span = "PT0S" } -mitol-drf-lint = { timestamp = "0001-01-01T00:00:00Z", span = "PT0S" } +mitol-django-common = { timestamp = "0001-01-01T00:00:00Z", span = "PT0S" } [manifest] overrides = [{ name = "setuptools", specifier = "<80" }] @@ -2770,7 +2770,7 @@ dev = [ { name = "pytest-repeat", specifier = ">=0.9.4" }, { name = "pytest-xdist", extras = ["psutil"], specifier = ">=3.6.1,<4" }, { name = "responses", specifier = ">=0.25.0,<0.26" }, - { name = "ruff", specifier = "==0.16.3" }, + { name = "ruff", specifier = "==0.16.8" }, { name = "safety", specifier = ">=3.0.0,<4" }, { name = "semantic-version", specifier = ">=2.10.0,<3" }, { name = "traceback-with-variables", specifier = ">=2.1.1,<3" }, @@ -4491,27 +4491,27 @@ wheels = [ [[package]] name = "ruff" -version = "0.16.3" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/61/b3/3213589383f8f1b3938781bd1278713f6d18621a14992b3e81fefb8a5ef9/ruff-0.16.3.tar.gz", hash = "sha256:e76d33a347661a84b5be6d043d0347fdc745dfdcf825a8f4fed64b5e26eebdf2", size = 4891904, upload-time = "2026-08-13T15:17:13.381Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/bf/96/493770daebd68c0a67f1549fdf519f53be51fc435186c0585bcc272fd76c/ruff-0.16.3-py3-none-linux_armv6l.whl", hash = "sha256:0c5710e247a58a4521e66e124ba9a74655b414f61ba3a2e9e3811e11098f48f7", size = 10902799, upload-time = "2026-08-13T15:16:27.382Z" }, - { url = "https://files.pythonhosted.org/packages/5e/e6/2becf3942fddc29a29b8df47691d456fb1085391a694f74d84513251418c/ruff-0.16.3-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:fe155130631a2471fd2e14a7a664a4dfbd7194b8229c3d7b2a40b21178639081", size = 11135539, upload-time = "2026-08-13T15:16:30.87Z" }, - { url = "https://files.pythonhosted.org/packages/3e/1e/4b8b72f0d006dbf19326aa99f9ca0ee2ff374187c4d301cf529a51aa06fe/ruff-0.16.3-py3-none-macosx_11_0_arm64.whl", hash = "sha256:e2ed719e14aa64d895c2ee922594a90a43c861a93f0575a95ff8c47cdbd13eb9", size = 10475095, upload-time = "2026-08-13T15:16:33.259Z" }, - { url = "https://files.pythonhosted.org/packages/92/32/2201fa49ba1f6c101ee321e83f051ac7a4b8d07b0ef6b4d3f2772b302275/ruff-0.16.3-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:9e0b1da805eb043654645d74d5de1e5ce2edc686e40790d2b86f56d71cc06a84", size = 10668771, upload-time = "2026-08-13T15:16:35.65Z" }, - { url = "https://files.pythonhosted.org/packages/c3/66/4afc5c8363bd04d45effce1b7c8713ca037d7a6740b7451a2403a6e3a972/ruff-0.16.3-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:a37bdea0bbe21780f590bf437d6412c8c4e1b6cd010f91a65c2c40c5e5f5f870", size = 10699568, upload-time = "2026-08-13T15:16:38.195Z" }, - { url = "https://files.pythonhosted.org/packages/53/fd/c67d246bf36bf1698551c56de39e95cd07f70e64433e0098e6267d77061b/ruff-0.16.3-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:09571e6d1288ed9be475207a3ac04ada404f1cd898104be0f6ab8d7df438575b", size = 11499365, upload-time = "2026-08-13T15:16:40.623Z" }, - { url = "https://files.pythonhosted.org/packages/67/0b/00ecbceb99a263af7b12f6f05ac3c92bc47b905e91adc3f207a836e3bc01/ruff-0.16.3-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:2c18c5a101eb540010638cc1ff3c84944d3adb3df62b8d98ca8f22ba484d3413", size = 12311728, upload-time = "2026-08-13T15:16:43.564Z" }, - { url = "https://files.pythonhosted.org/packages/54/b2/b7b3bb54f4d3f7db504e476ad4ab8de530dceebe2c061384b2757ee419e8/ruff-0.16.3-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:8457c44f15033c85ddbb77b15d451df9e24e4bd03b628396dd3610cedc3b8f82", size = 11699896, upload-time = "2026-08-13T15:16:46.209Z" }, - { url = "https://files.pythonhosted.org/packages/c7/30/4c468429ac195addc5ee1b717b6ab1b66632786737ca3b2ed3443fb0c26a/ruff-0.16.3-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:294b95c4ae0cda9388525c2047778aa758d6b8d4bb876fd4e9eaa3ebc92343eb", size = 11058736, upload-time = "2026-08-13T15:16:48.823Z" }, - { url = "https://files.pythonhosted.org/packages/43/67/7a113cdaddf24b64d7f75b1242a99d04c82fcef4f6921fdbb832beaffb5f/ruff-0.16.3-py3-none-manylinux_2_31_riscv64.whl", hash = "sha256:3d0c7c40c87c2a820509c31ba007968da6e1306468c067b2d82fbfdbcd0e8474", size = 11586911, upload-time = "2026-08-13T15:16:51.913Z" }, - { url = "https://files.pythonhosted.org/packages/f1/c1/2e66f24c0f3ead25a5e660111778685e505e5da353c82802bf49f0cbe7b9/ruff-0.16.3-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:9f738c0fdfa8eed0b2ce7fb27ee7258208a92a68d7949e62aa15164bc7b389da", size = 10954265, upload-time = "2026-08-13T15:16:54.763Z" }, - { url = "https://files.pythonhosted.org/packages/c2/ba/4cee23bf52cba9a058d3726de623624daf50ef9638868edd86f4126157f6/ruff-0.16.3-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:fb785f0be25abe69d320415cd4f833b59e17ba7613d9ba6a958023b6bceb0a50", size = 10709886, upload-time = "2026-08-13T15:16:57.339Z" }, - { url = "https://files.pythonhosted.org/packages/82/df/7da7194fa5d9dc0a285f7e6fa5a4722e7c63faac0b45b614ded9314363a1/ruff-0.16.3-py3-none-musllinux_1_2_i686.whl", hash = "sha256:c5536e3acfbf9563085aa2be7b13c629c3077e902afc5b941ac44024dbb9f506", size = 11210392, upload-time = "2026-08-13T15:17:00.171Z" }, - { url = "https://files.pythonhosted.org/packages/35/85/7795f6e817af050e7517bf3e7aa9b061cce70ef33d280aad902c956c1ecf/ruff-0.16.3-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:a2d85c02f9b8e165d85e6779184d38c4132de12603dab59c51c28e22584f9e4d", size = 11626910, upload-time = "2026-08-13T15:17:03.299Z" }, - { url = "https://files.pythonhosted.org/packages/78/9b/475b927cf27a5cbbda3c7bafb69ed6ff77e1d7923d5d85f17c2749d7ae32/ruff-0.16.3-py3-none-win32.whl", hash = "sha256:388cdf2166642bd9b13d52b5932d3170f34f8abed7e8d9a855f1d84b83645a0a", size = 10931415, upload-time = "2026-08-13T15:17:05.726Z" }, - { url = "https://files.pythonhosted.org/packages/b2/99/e2a2bfc4fbf0a1e8a916bc9ebe6fe6c58cc34c28e0ffc6ce281d572d1c2e/ruff-0.16.3-py3-none-win_amd64.whl", hash = "sha256:e80a7d69ca2a6d1c4d352ec91458cdca6e56c83cdbcabd93e4abe1e53591d948", size = 11445993, upload-time = "2026-08-13T15:17:08.353Z" }, - { url = "https://files.pythonhosted.org/packages/69/3e/4132e539aed78c148854d4997a2685b0ed4dc4e87110b59ce528564e184e/ruff-0.16.3-py3-none-win_arm64.whl", hash = "sha256:b8ca152da82c1acc1fa8d5874b15951935f0eef46f10e6954c83859011b6178a", size = 11399302, upload-time = "2026-08-13T15:17:10.908Z" }, +version = "0.16.8" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/ba/78/449cb84790bd5cc3823b2652ee405a4558856e5c4195aee3a16bf7b3eb5d/ruff-0.16.8.tar.gz", hash = "sha256:9247bf92b5f04d825c8639a4fe423ec2e4222acd9222e58412b0dab7e442798b", size = 4938814, upload-time = "2026-09-16T15:54:46.688Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ac/25/6071aabc530e9be7e2c195e8fe3f7aea2735405b6cf447212832d7811831/ruff-0.16.8-py3-none-linux_armv6l.whl", hash = "sha256:6ffbd6d87383c1edf5f6fa890f10200950240d7c1a16052a19a09d3a2307dd38", size = 10048966, upload-time = "2026-09-16T15:53:57.605Z" }, + { url = "https://files.pythonhosted.org/packages/54/98/07f90ecbc74dd5fb5764f11f2bc774d6a7cffef92d2ff5f5b4e9e23c754e/ruff-0.16.8-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:42ed6b878ed61e3acca92f2730a17acff39286944ea82398544696366a6f925e", size = 10165498, upload-time = "2026-09-16T15:54:01.14Z" }, + { url = "https://files.pythonhosted.org/packages/fe/1f/e6a712e3b47cad4a40600134105ed193cb773f618a42eb7ba323cb812cc0/ruff-0.16.8-py3-none-macosx_11_0_arm64.whl", hash = "sha256:7ea781c7f2afba8c6a505ea0fb3f994020249e0c450635f5381286fea6b46170", size = 9830004, upload-time = "2026-09-16T15:54:03.998Z" }, + { url = "https://files.pythonhosted.org/packages/23/f2/311a08776d75d81c7676e20b6b020ae63cbe881fcdc7a8dd64e6e18bdd93/ruff-0.16.8-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:8efeae3bbe414a5efefda11a792dfb51ef90ac48d50c4830de2f644caf3e8659", size = 9986558, upload-time = "2026-09-16T15:54:06.804Z" }, + { url = "https://files.pythonhosted.org/packages/f3/ed/37b6cb3d3ba8c73e68ae3eb1d502383beb5aa05a582bb7bb3a922f929f54/ruff-0.16.8-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:3a79b795469fef7fc6e908b218eed2eb17332afd85031db6480dc864560e69b2", size = 9877332, upload-time = "2026-09-16T15:54:09.552Z" }, + { url = "https://files.pythonhosted.org/packages/22/cc/40873a8f36ad084cc540d55fcca7077264d5b13b24659e9180c176fb2b08/ruff-0.16.8-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:3fdc5563cdc50555e6fba39322850860e9267c1b3d12c26a74729d8604c3c812", size = 10507125, upload-time = "2026-09-16T15:54:12.152Z" }, + { url = "https://files.pythonhosted.org/packages/c3/e4/fc91a642b78ccbab6b9477720f3644ae7a10a9bcce69a934679cd64f62bc/ruff-0.16.8-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:34508983c70665578dab88f5223d8e6228307e1135398ca8bfc8b7e9501e282b", size = 11336694, upload-time = "2026-09-16T15:54:15.489Z" }, + { url = "https://files.pythonhosted.org/packages/c2/3d/bbd2a9a600a4e73dc3e7548a249c8d1671273464b55822c6fae50f602dff/ruff-0.16.8-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:644bb578569e0ffc575741232bd385dacdd6fbe123f1a729e7a225f54aa3957f", size = 10774448, upload-time = "2026-09-16T15:54:18.16Z" }, + { url = "https://files.pythonhosted.org/packages/1a/41/d83af9879a7b6e8bf5fe16b1da0b134049d2f5d3afac12defb0897cb84bd/ruff-0.16.8-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:15e7d226246961db9235098333caa13063906d3851136b84c2900b82f5daa1df", size = 10323796, upload-time = "2026-09-16T15:54:20.743Z" }, + { url = "https://files.pythonhosted.org/packages/f5/2c/cefd07bfe914b84943ea769ade8d607bd22750b965d3228eefd7cebd15d0/ruff-0.16.8-py3-none-manylinux_2_31_riscv64.whl", hash = "sha256:a2bf6bc3e9ebdd4449abc6f06cf64b98051a2c61cf94d2fe9596518c881f1a1e", size = 10514115, upload-time = "2026-09-16T15:54:23.497Z" }, + { url = "https://files.pythonhosted.org/packages/f3/9d/76a2e26c79a23be6e6e3664c57bec9e9fc8de155cfb9e4b67ea91b64f9d7/ruff-0.16.8-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:6ca111ba0849539165e9e59d2b442542f3c1e8060ebbdea82494f1ffbccb1e1f", size = 10072582, upload-time = "2026-09-16T15:54:26.185Z" }, + { url = "https://files.pythonhosted.org/packages/2e/d4/f42edddb39668af1a559ceafa3823aedd65633a48dc9768e775485faa2c1/ruff-0.16.8-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:359a1e5b495448ee1e91018064382ebc86f90e8aac2fed222c7d0e4e8df85fd2", size = 9879644, upload-time = "2026-09-16T15:54:29.278Z" }, + { url = "https://files.pythonhosted.org/packages/f8/d4/913e3195d95e0378786c6656945c865f534a3560e29139da4882aff630d1/ruff-0.16.8-py3-none-musllinux_1_2_i686.whl", hash = "sha256:59e8f5681349474110b24d62e93cfda6593f5fa3473446ca3705200cac1a08b9", size = 10231569, upload-time = "2026-09-16T15:54:32.036Z" }, + { url = "https://files.pythonhosted.org/packages/2b/c4/8aa6ea0bdcedbd1bf87397e2fc4ed8406448ea5842f8660bc6e5f163039d/ruff-0.16.8-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:efa3e7a16d1baaa79957888dfdf8be9ef2e44db81cb032af06d76632ab59e773", size = 10663666, upload-time = "2026-09-16T15:54:34.838Z" }, + { url = "https://files.pythonhosted.org/packages/3d/02/7f10ef4700bc223c30a3fdd10631a29830c45524b810a3c7ed947af64591/ruff-0.16.8-py3-none-win32.whl", hash = "sha256:55793ba85c69921e89be061426d91a78652d6e50317c962240922747a4eb713f", size = 10093472, upload-time = "2026-09-16T15:54:37.47Z" }, + { url = "https://files.pythonhosted.org/packages/1e/5d/a509c07d714b6da88f2c518b4637cf6f1d46b074be8f0f1e5fb9ff5126fe/ruff-0.16.8-py3-none-win_amd64.whl", hash = "sha256:a6b85621fd3c81e31fc5f5add09c9c078b430db3595ca632efafdec9e64ebfaa", size = 10586899, upload-time = "2026-09-16T15:54:40.488Z" }, + { url = "https://files.pythonhosted.org/packages/fe/a0/50787329e4f20bf9dc9f6230015d46ec69c51a97ace5bc202dae4755365d/ruff-0.16.8-py3-none-win_arm64.whl", hash = "sha256:d075e820af612102ce217f07cc93e69f9490b10ec13ea85fa87bd03d996cef8a", size = 10386316, upload-time = "2026-09-16T15:54:43.332Z" }, ] [[package]] diff --git a/vector_search/views.py b/vector_search/views.py index d329818af6..6305a9cae3 100644 --- a/vector_search/views.py +++ b/vector_search/views.py @@ -136,7 +136,7 @@ def _format_order_by(self, order_by_parameter): sort = models.Direction.DESC return models.OrderBy(key=order_by_parameter, direction=sort) - async def _build_search_params( # noqa: PLR0913 + async def _build_search_params( # noqa: PLR0913, PLR0917 self, query_string: str, search_collection: str, @@ -320,7 +320,7 @@ async def _scroll_missing_order_by_key(self, client, scroll_kwargs, key, limit): ) return page_points - async def _execute_scroll_search( # noqa: PLR0913 + async def _execute_scroll_search( # noqa: PLR0913, PLR0917 self, client, search_collection, @@ -391,7 +391,7 @@ async def _execute_scroll_search( # noqa: PLR0913 break return search_result[:limit] - async def _async_vector_hits( # noqa: PLR0913 + async def _async_vector_hits( # noqa: PLR0913, PLR0917 self, query_string: str, params: dict, @@ -601,7 +601,7 @@ async def _async_vector_counts( "aggregations": aggregations or {}, } - async def async_vector_search( # noqa: PLR0913 + async def async_vector_search( # noqa: PLR0913, PLR0917 self, query_string: str, params: dict, From 468d737eeeb8c3ba120a1ae38802c05fc0dd750a Mon Sep 17 00:00:00 2001 From: Sar <1447295+shaidar@users.noreply.github.com> Date: Tue, 29 Sep 2026 10:27:02 -0500 Subject: [PATCH 2/9] fix(widgets): remove the unused RSS Feed widget type (#4000) * fix(widgets): remove the unused RSS Feed widget type Per team confirmation on hq#13473, the RSS Feed widget was a channels/Open-Discussions-era feature with no current way to add one -- verified independently: the widget-list feature's frontend surface (hooks/widget_lists) has no consuming component anywhere in the frontend, so no widget type, RSS included, is reachable through any current UI. The backend registry (get_widget_classes) still fully accepted and processed RSS widgets via the API, though, including its unvalidated server-side feedparser.parse(url) fetch -- the SSRF reported in hq#13473. Rather than adding an SSRF allow-list to a feature nobody can reach or create, remove it outright: - widgets/serializers/rss.py and its tests (_fetch_rss, RssFeedWidgetConfigSerializer, RssFeedWidgetSerializer) - the registry entry in widgets/serializers/utils.py (get_widget_classes/get_widget_type_mapping/get_widget_type_names now only expose Markdown/URL/People) - the now-dead WIDGETS_RSS_CACHE_TTL setting (no env var override anywhere in mit-learn or ol-infrastructure) - the type_rss factory trait and the RSS entry in the available-widgets test fixture - regenerated openapi/specs/v0.yaml and the v0 TypeScript client to drop "RSS Feed" from the widget_type enum Markdown/URL/People widget types are untouched -- this is scoped to the specific dead, vulnerable feature this issue is about, not the broader (also currently frontend-unreachable) widget-list feature. Fixes https://github.com/mitodl/hq/issues/13473 Co-Authored-By: Claude Sonnet 5 * fix(openapi): acknowledge the intentional RSS Feed enum removal oasdiff correctly flags removing "RSS Feed" from widget_type's enum as a breaking change for PATCH/PUT /api/v0/widget_lists/{id}/ -- which is the intended effect here, not an oversight. Verified locally with the same tufin/oasdiff invocation the CI job runs (base=origin/main, head=this branch): passes with exit 0 once these entries are added. Co-Authored-By: Claude Sonnet 5 * fix(widgets): reject unsupported widget_type in update() instead of crashing Sentry review feedback on this PR: WidgetListSerializer.update() looked up the serializer class for a widget's widget_type and called it unconditionally. If the type isn't in the registry -- which, after this PR, "RSS Feed" now isn't -- the lookup returns None and calling it raises an unhandled TypeError: 'NoneType' object is not callable, a 500 instead of a clean validation error. Confirmed this is a pre-existing bug in update() (the read path, get_widgets, already guarded against unknown widget_type; update() never did), not something specific to RSS -- reproduced the exact TypeError directly against the current registry to confirm. This PR's removal of "RSS Feed" from the registry just makes it newly reachable for a value that used to be valid, so any client still sending it would now hit this crash instead of a clean rejection. Raise ValidationError instead when the widget_type has no matching serializer class (covers missing types too, not just unsupported ones). New parametrized test covers an unsupported string, "RSS Feed" specifically, and a missing widget_type key. Co-Authored-By: Claude Sonnet 5 --------- Co-authored-by: Claude Sonnet 5 --- frontends/api/src/generated/v0/api.ts | 7 +- main/settings.py | 3 - openapi/oasdiff-err-ignore.txt | 11 +++ openapi/specs/v0.yaml | 3 - widgets/factories.py | 5 -- widgets/serializers/rss.py | 115 ------------------------ widgets/serializers/rss_test.py | 102 --------------------- widgets/serializers/utils.py | 2 - widgets/serializers/widget_list.py | 6 +- widgets/serializers/widget_list_test.py | 17 ++++ widgets/views_test.py | 27 ------ 11 files changed, 34 insertions(+), 264 deletions(-) delete mode 100644 widgets/serializers/rss.py delete mode 100644 widgets/serializers/rss_test.py diff --git a/frontends/api/src/generated/v0/api.ts b/frontends/api/src/generated/v0/api.ts index bef5eb1592..6c81b6a9cc 100644 --- a/frontends/api/src/generated/v0/api.ts +++ b/frontends/api/src/generated/v0/api.ts @@ -2597,13 +2597,12 @@ export interface WidgetListRequest { widgets?: Array | null } /** - * * `Markdown` - Markdown * `URL` - URL * `RSS Feed` - RSS Feed * `People` - People + * * `Markdown` - Markdown * `URL` - URL * `People` - People */ export const WidgetTypeEnumDescriptions = { Markdown: "Markdown", URL: "URL", - "RSS Feed": "RSS Feed", People: "People", } as const @@ -2616,10 +2615,6 @@ export const WidgetTypeEnum = { * URL */ Url: "URL", - /** - * RSS Feed - */ - RssFeed: "RSS Feed", /** * People */ diff --git a/main/settings.py b/main/settings.py index 0f1f3e44d6..3454fa19e2 100644 --- a/main/settings.py +++ b/main/settings.py @@ -697,9 +697,6 @@ def get_all_config_keys(): # disable the anonymous user creation ANONYMOUS_USER_NAME = None -# Widgets -WIDGETS_RSS_CACHE_TTL = get_int("WIDGETS_RSS_CACHE_TTL", 15 * 60) - # x509 filenames MIT_WS_CERTIFICATE_FILE = os.path.join(STATIC_ROOT, "mit_x509.cert") # noqa: PTH118 MIT_WS_PRIVATE_KEY_FILE = os.path.join(STATIC_ROOT, "mit_x509.key") # noqa: PTH118 diff --git a/openapi/oasdiff-err-ignore.txt b/openapi/oasdiff-err-ignore.txt index 2eb053eb84..873b225332 100644 --- a/openapi/oasdiff-err-ignore.txt +++ b/openapi/oasdiff-err-ignore.txt @@ -30,3 +30,14 @@ 2026-09-15 GET /api/v0/users/{username}/ the response property `profile/email_optin` became nullable for the status `200` 2026-09-15 PATCH /api/v0/users/{username}/ the response property `profile/email_optin` became nullable for the status `200` 2026-09-15 PUT /api/v0/users/{username}/ the response property `profile/email_optin` became nullable for the status `200` + +# Removing the RSS Feed widget type entirely (dead Open-Discussions-era +# feature, no reachable frontend UI, confirmed with the team) -- +# nobody should be able to send this value anymore. +# https://github.com/mitodl/hq/issues/13473 +2026-09-28 PATCH /api/v0/widget_lists/{id}/ removed the enum value `RSS Feed` of the request property `widgets/items/widget_type` (media type: application/json) +2026-09-28 PATCH /api/v0/widget_lists/{id}/ removed the enum value `RSS Feed` of the request property `widgets/items/widget_type` (media type: application/x-www-form-urlencoded) +2026-09-28 PATCH /api/v0/widget_lists/{id}/ removed the enum value `RSS Feed` of the request property `widgets/items/widget_type` (media type: multipart/form-data) +2026-09-28 PUT /api/v0/widget_lists/{id}/ removed the enum value `RSS Feed` of the request property `widgets/items/widget_type` (media type: application/x-www-form-urlencoded) +2026-09-28 PUT /api/v0/widget_lists/{id}/ removed the enum value `RSS Feed` of the request property `widgets/items/widget_type` (media type: multipart/form-data) +2026-09-28 PUT /api/v0/widget_lists/{id}/ removed the enum value `RSS Feed` of the request property `widgets/items/widget_type` (media type: application/json) diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index adfed4b995..751bb132da 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -6956,16 +6956,13 @@ components: enum: - Markdown - URL - - RSS Feed - People type: string description: |- * `Markdown` - Markdown * `URL` - URL - * `RSS Feed` - RSS Feed * `People` - People x-enum-descriptions: - Markdown - URL - - RSS Feed - People diff --git a/widgets/factories.py b/widgets/factories.py index fe78073828..4cf321b963 100644 --- a/widgets/factories.py +++ b/widgets/factories.py @@ -42,11 +42,6 @@ class Params: title="Markdown Widget", configuration={"source": "*Here's some basic markdown*"}, ) - type_rss = factory.Trait( - widget_type="RSS Feed", - title="RSS Widget", - configuration={"url": "http://example.com", "feed_display_limit": 10}, - ) type_people = factory.Trait( widget_type="People", title="People Widget", diff --git a/widgets/serializers/rss.py b/widgets/serializers/rss.py deleted file mode 100644 index 8477bc6851..0000000000 --- a/widgets/serializers/rss.py +++ /dev/null @@ -1,115 +0,0 @@ -"""RSS widget""" - -import logging -import time - -import feedparser -from cache_memoize import cache_memoize -from django.conf import settings -from rest_framework.serializers import ValidationError - -from main.constants import ISOFORMAT -from widgets.serializers.react_fields import ReactIntegerField, ReactURLField -from widgets.serializers.widget_instance import ( - WidgetConfigSerializer, - WidgetInstanceSerializer, -) - -MAX_FEED_ITEMS = 10 - -log = logging.getLogger() - - -class RssFeedWidgetConfigSerializer(WidgetConfigSerializer): - """Serializer for RssFeedWidget config""" - - url = ReactURLField(help_text="RSS feed URL", label="URL") - feed_display_limit = ReactIntegerField( - min_value=1, max_value=MAX_FEED_ITEMS, default=5, label="Max number of items" - ) - - -@cache_memoize(settings.WIDGETS_RSS_CACHE_TTL, cache_alias="external_assets") -def _fetch_rss(url): - """ - Fetches the RSS feed data - - Args: - url (str): the RSS feed url to fetch - - Returns: - feedparser.FeedParserDict: rss feed data - """ # noqa: D401 - # NOTE: if you change what this function returns you need to ensure caches are evicted # noqa: E501 - # across all our environments, ideally in an automated way because cache_memoize # noqa: E501 - # won't know your implementation change. One possible way is to rename the function # noqa: E501 - # or change the arguments. - return feedparser.parse(url) - - -class RssFeedWidgetSerializer(WidgetInstanceSerializer): - """A basic rss feed widget""" - - configuration_serializer_class = RssFeedWidgetConfigSerializer - - name = "RSS Feed" - description = "RSS Feed" - - def get_json(self, instance): - """Obtains RSS feed data which will then be provided to the React component""" # noqa: D401 - try: - rss = _fetch_rss(instance.configuration["url"]) - entries = getattr(rss, "entries", []) - except: # pylint: disable=bare-except # noqa: E722 - # log an exception and coerce this to an empty list so the feed and UI don't crash # noqa: E501 - log.exception( - "Error trying to refresh cached RSS feed for widget id: %s", instance.id - ) - entries = [] - - timestamp_key = ( - "published_parsed" - if entries and "published_parsed" in entries[0] - else "updated_parsed" - ) - sorted_feed = sorted( - entries, reverse=True, key=lambda entry: entry[timestamp_key] - ) - display_limit = min( - instance.configuration["feed_display_limit"], MAX_FEED_ITEMS - ) - - return { - "title": instance.title, - "entries": [ - { - "title": entry.get("title"), - "description": entry.get("description"), - "link": entry.get("link"), - "timestamp": ( - time.strftime(ISOFORMAT, entry.get(timestamp_key)) - if entry.get(timestamp_key) - else None - ), - } - for entry in sorted_feed[:display_limit] - ], - } - - def save(self, **kwargs): - """Saves the widget settings""" # noqa: D401 - instance = super().save(**kwargs) - - url = instance.configuration["url"] - - try: - # force a cache refresh immediately after a successful save by invalidating and then loading it # noqa: E501 - _fetch_rss.invalidate(url) - _fetch_rss(url) - except: # pylint: disable=bare-except # noqa: E722 - log.exception("Error trying to load new RSS feed url: %s", url) - raise ValidationError( # noqa: B904 - {"configuration": f"Unable to load the RSS feed: '{url}'"} - ) - - return instance diff --git a/widgets/serializers/rss_test.py b/widgets/serializers/rss_test.py deleted file mode 100644 index fd0065f572..0000000000 --- a/widgets/serializers/rss_test.py +++ /dev/null @@ -1,102 +0,0 @@ -"""Tests for rss widget serializer""" - -import time - -import pytest -from rest_framework.serializers import ValidationError - -from main.test_utils import PickleableMock -from widgets.factories import WidgetInstanceFactory, WidgetListFactory -from widgets.serializers import rss - - -@pytest.mark.django_db -@pytest.mark.parametrize("raise_exception", [True, False]) -@pytest.mark.parametrize("timestamp_key", ["published_parsed", "updated_parsed"]) -@pytest.mark.parametrize("item_count", [0, 8, 15]) -@pytest.mark.parametrize("display_limit", [0, 6, 10, 14, 18]) -def test_url_widget_serialize( - mocker, raise_exception, timestamp_key, item_count, display_limit -): - """Tests that the rss widget serializes correctly""" - entries = sorted( - [ - { - "title": f"Title {idx}", - "description": f"Description {idx}", - "link": f"http://example.com/{idx}", - timestamp_key: time.gmtime(), - } - for idx in range(item_count) - ], - reverse=True, - key=lambda entry: entry[timestamp_key], - ) - mock_parse = mocker.patch("feedparser.parse") - if raise_exception: - mock_parse.side_effect = Exception("bad") - else: - mock_parse.return_value = PickleableMock(entries=entries) - widget_instance = WidgetInstanceFactory.create(type_rss=True) - widget_instance.configuration["feed_display_limit"] = display_limit - widget_instance.save() - - data = rss.RssFeedWidgetSerializer(widget_instance).data - - mock_parse.assert_called_once_with(widget_instance.configuration["url"]) - - assert data == { - "id": widget_instance.id, - "widget_type": "RSS Feed", - "title": widget_instance.title, - "configuration": widget_instance.configuration, - "json": { - "title": widget_instance.title, - "entries": ( - [] - if raise_exception - else [ - { - "title": entry["title"], - "description": entry["description"], - "link": entry["link"], - "timestamp": time.strftime( - "%Y-%m-%dT%H:%M:%SZ", entry[timestamp_key] - ), - } - for entry in entries[: min(rss.MAX_FEED_ITEMS, display_limit)] - ] - ), - }, - } - - -@pytest.mark.django_db -@pytest.mark.parametrize("raise_exception", [True, False]) -def test_url_widget_save(mocker, raise_exception): - """Tests that the rss widget serializes correctly""" - widget_list = WidgetListFactory.create() - url = "http://example.com" - data = { - "widget_list_id": widget_list.id, - "title": "Title", - "widget_type": "RSS Feed", - "position": 1, - "configuration": {"url": url}, - } - mock_parse = mocker.patch("feedparser.parse") - if raise_exception: - mock_parse.side_effect = Exception("bad") - else: - mock_parse.return_value = PickleableMock(entries=[]) - - serializer = rss.RssFeedWidgetSerializer(data=data) - serializer.is_valid() - - if raise_exception: - with pytest.raises(ValidationError): - serializer.save() - else: - serializer.save() - - mock_parse.assert_called_once_with(url) diff --git a/widgets/serializers/utils.py b/widgets/serializers/utils.py index a9d4144ccc..3ca649a283 100644 --- a/widgets/serializers/utils.py +++ b/widgets/serializers/utils.py @@ -11,13 +11,11 @@ def get_widget_classes(): # NOTE: these imports are inline to avoid circular imports from widgets.serializers.markdown import MarkdownWidgetSerializer from widgets.serializers.people import PeopleWidgetSerializer - from widgets.serializers.rss import RssFeedWidgetSerializer from widgets.serializers.url import URLWidgetSerializer return [ MarkdownWidgetSerializer, URLWidgetSerializer, - RssFeedWidgetSerializer, PeopleWidgetSerializer, ] diff --git a/widgets/serializers/widget_list.py b/widgets/serializers/widget_list.py index 31a4c54303..d81ee46881 100644 --- a/widgets/serializers/widget_list.py +++ b/widgets/serializers/widget_list.py @@ -80,7 +80,11 @@ def update(self, instance, validated_data): for data in widgets_data: widget_id = data.get("id", None) widget = existing_widgets_by_id.get(widget_id) - widget_serializer_cls = _serializer_for_widget_type(data["widget_type"]) + widget_type = data.get("widget_type") + widget_serializer_cls = _serializer_for_widget_type(widget_type) + if widget_serializer_cls is None: + msg = f"Unsupported widget_type: {widget_type!r}" + raise serializers.ValidationError({"widget_type": msg}) if not widget: # if the widget provided was not in the data, ensure the user isn't trying to set id or widget_list_id # noqa: E501 diff --git a/widgets/serializers/widget_list_test.py b/widgets/serializers/widget_list_test.py index baae443245..c098ad397f 100644 --- a/widgets/serializers/widget_list_test.py +++ b/widgets/serializers/widget_list_test.py @@ -1,6 +1,7 @@ """Tests for widget list serializer""" import pytest +from rest_framework.exceptions import ValidationError from widgets.factories import WidgetInstanceFactory, WidgetListFactory from widgets.serializers.widget_list import WidgetListSerializer @@ -18,3 +19,19 @@ def test_missing_widget_type(): "widgets": [], "id": widget_list.id, } + + +@pytest.mark.parametrize("widget_type", ["not_A_real_type", "RSS Feed", None]) +def test_update_with_unsupported_widget_type(widget_type): + """ + An unsupported widget_type on update() should raise a validation error, + not crash with TypeError trying to call a None serializer class. + """ + widget_list = WidgetListFactory.create() + data = {"widget_type": widget_type, "title": "x", "configuration": {}} + if widget_type is None: + del data["widget_type"] + serializer = WidgetListSerializer(widget_list, data={"widgets": [data]}) + assert serializer.is_valid(), serializer.errors + with pytest.raises(ValidationError): + serializer.save() diff --git a/widgets/views_test.py b/widgets/views_test.py index b5b413cd96..72ef56470f 100644 --- a/widgets/views_test.py +++ b/widgets/views_test.py @@ -65,33 +65,6 @@ "widget_type": "URL", "description": "Embedded URL", }, - { - "form_spec": [ - { - "field_name": "url", - "input_type": "url", - "label": "URL", - "under_text": None, - "props": { - "max_length": "", - "min_length": "", - "placeholder": "RSS feed URL", - "show_embed": False, - }, - "default": "", - }, - { - "field_name": "feed_display_limit", - "input_type": "number", - "label": "Max number of items", - "under_text": None, - "props": {"max": 10, "min": 1}, - "default": 5, - }, - ], - "widget_type": "RSS Feed", - "description": "RSS Feed", - }, { "description": "People", "form_spec": [ From b475e23d2c7ab96807882fde1b9a3c1e7ced72a7 Mon Sep 17 00:00:00 2001 From: Sar <1447295+shaidar@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:10:41 -0500 Subject: [PATCH 3/9] fix(learning_resources): gate summary/flashcards like content on ContentFileViewSet (#3999) ContentFileViewSet.private_fields only listed "content", so anonymous and non-privileged callers could read a ContentFile's LLM-generated .summary and .flashcards -- a full AI-condensed summary and ready-made study flashcards distilled from the same gated course material -- even though .content itself was correctly hidden. 1,823 of 191,818 rows in QA already have populated values being served this way today. summary/flashcards were added to the model in March 2025; the private_fields=["content"] gate was added afterward in June 2025 and just never included them. Derive private_fields from the existing CONTENT_FILE_LARGE_FIELDS constant (content, summary, flashcards) -- which already groups these three as equivalent-sensitivity elsewhere in the codebase -- instead of a separate hardcoded list, so they can't drift apart again the next time an LLM-derived field is added. Fixes https://github.com/mitodl/hq/issues/13461 Co-authored-by: Claude Sonnet 5 --- learning_resources/views.py | 7 ++++++- learning_resources/views_test.py | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/learning_resources/views.py b/learning_resources/views.py index 236ba2bed2..f88f5bb04d 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -34,6 +34,7 @@ from channels.models import Channel from learning_resources import permissions from learning_resources.constants import ( + CONTENT_FILE_LARGE_FIELDS, GROUP_CONTENT_FILE_CONTENT_VIEWERS, LearningResourceRelationTypes, LearningResourceType, @@ -1076,7 +1077,11 @@ class ContentFileViewSet(viewsets.ReadOnlyModelViewSet): ) filter_backends = [MultipleOptionsFilterBackend] filterset_class = ContentFileFilter - private_fields = ["content"] + # Derived from CONTENT_FILE_LARGE_FIELDS (rather than listing "content" + # alone) so summary/flashcards -- LLM-derived from the same gated content, + # and just as sensitive -- can't silently fall out of this gate again the + # next time a field is added there. + private_fields = list(CONTENT_FILE_LARGE_FIELDS) def get_serializer(self, *args, **kwargs): """ diff --git a/learning_resources/views_test.py b/learning_resources/views_test.py index a1089f7b81..76133cc124 100644 --- a/learning_resources/views_test.py +++ b/learning_resources/views_test.py @@ -16,6 +16,7 @@ from channels.models import Channel from learning_resources.api import update_resource_view_counts from learning_resources.constants import ( + CONTENT_FILE_LARGE_FIELDS, GROUP_CONTENT_FILE_CONTENT_VIEWERS, GROUP_TUTOR_PROBLEM_VIEWERS, LearningResourceRelationTypes, @@ -288,7 +289,11 @@ def test_list_content_files_list_endpoint(client, user_role, django_user_model): content_file_ids = [ cf.id for cf in ContentFileFactory.create_batch( - 2, run=course.learning_resource.runs.first(), content="some content" + 2, + run=course.learning_resource.runs.first(), + content="some content", + summary="some summary", + flashcards=[{"question": "q", "answer": "a"}], ) ] # this should be filtered out @@ -316,8 +321,12 @@ def test_list_content_files_list_endpoint(client, user_role, django_user_model): if user_role in ["admin", "group_content_file_content_viewer"]: assert result["content"] is not None + assert result["summary"] is not None + assert result["flashcards"] is not None else: assert result.get("content") is None + assert result.get("summary") is None + assert result.get("flashcards") is None def test_list_content_files_list_endpoint_with_no_runs(client): @@ -393,7 +402,8 @@ def test_get_contentfiles_detail_endpoint(client, user_role, django_user_model): assert resp.data == ContentFileSerializer(instance=content_file).data else: data = ContentFileSerializer(instance=content_file).data - data.pop("content") + for field in CONTENT_FILE_LARGE_FIELDS: + data.pop(field) assert resp.data == data From 4fdeccdbc70ce95b3a89c766db2ff883e216acd0 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Tue, 29 Sep 2026 12:44:55 -0400 Subject: [PATCH 4/9] Do not follow redirects when fetching OVS transcripts (#4003) --- learning_resources/etl/utils.py | 7 ++++++- learning_resources/etl/utils_test.py | 16 ++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/learning_resources/etl/utils.py b/learning_resources/etl/utils.py index 731b59db75..2af1f1e8c1 100644 --- a/learning_resources/etl/utils.py +++ b/learning_resources/etl/utils.py @@ -212,8 +212,13 @@ def extract_text_from_url(url, *, mime_type=None): Returns: str: The text contained in the URL content. """ - response = requests.get(url, timeout=30) + # ponytail: refuse redirects outright rather than re-validating each hop; + # add per-hop validation if a source ever needs to redirect legitimately + response = requests.get(url, timeout=30, allow_redirects=False) response.raise_for_status() + if response.is_redirect: + msg = f"Refusing to follow redirect from {url}" + raise requests.HTTPError(msg, response=response) if response.content: return extract_text_metadata( response.content, diff --git a/learning_resources/etl/utils_test.py b/learning_resources/etl/utils_test.py index b254aa1352..9bf26fbb28 100644 --- a/learning_resources/etl/utils_test.py +++ b/learning_resources/etl/utils_test.py @@ -8,6 +8,7 @@ import pypdf import pytest +import requests from defusedxml import ElementTree from learning_resources.constants import ( @@ -112,18 +113,29 @@ def test_extract_text_from_url(mocker, content): url = "http://test.edu/file.pdf" mock_request = mocker.patch( "learning_resources.etl.utils.requests.get", - return_value=mocker.Mock(content=content), + return_value=mocker.Mock(content=content, is_redirect=False), ) mock_extract = mocker.patch("learning_resources.etl.utils.extract_text_metadata") utils.extract_text_from_url(url, mime_type=mime_type) - mock_request.assert_called_once_with(url, timeout=30) + mock_request.assert_called_once_with(url, timeout=30, allow_redirects=False) if content: mock_extract.assert_called_once_with( content, other_headers={"Content-Type": mime_type} ) +def test_extract_text_from_url_refuses_redirect(mocked_responses): + """extract_text_from_url should not follow a redirect off the requested host""" + url = "https://abc.cloudfront.net/a.vtt" + mocked_responses.get( + url, status=302, headers={"Location": "http://169.254.169.254/latest/"} + ) + + with pytest.raises(requests.HTTPError): + utils.extract_text_from_url(url) + + @pytest.mark.parametrize( ("text", "readable_id"), [ From f60be1a4e68ade4eaf358c70ba6b44677a984d6d Mon Sep 17 00:00:00 2001 From: Ahtesham Quraish Date: Tue, 29 Sep 2026 22:25:55 +0500 Subject: [PATCH 5/9] feat(website-content): require SEO fields to publish, and align the published control bar (#4009) * Lay the published control bar out like the edit one It is the same bar in the same place, but the two looked unrelated: the published view ran the full width of the window with its buttons against the left edge and the status against the right, while edit mode puts the status at the left end and the actions at the right, both lined up with the breadcrumb and the text. The published view now reuses the same `ActionRow`, so it carries the article's 890px column and cannot drift from edit mode when that column changes. Also drops the flex wrapper around the read-only toolbar. It set gap and margin on a `position: fixed` child, which it could not lay out either way, and now that the button group uses the same container inside the row its presence there was actively misleading. Co-Authored-By: Claude Opus 5 (1M context) * Require an SEO title and description before publishing They are what a search result and a link preview show, and without them the page head falls back to the title and whatever the body happens to open with -- which is the thin description the SEO rules single out. So publishing now insists on both: the press opens the settings drawer instead, and resumes once they are written. Held to the same rule as topics, and implemented the same way, for the same reason -- it has to be the publish and not the save, because a draft writes itself every couple of seconds and autosave cannot stop to ask. Unlike topics this applies to news as well: news has no topics section, so the SEO fields are the only thing its publish can wait on. The drawer refuses to save while a publish is waiting on it and still has not got what it needs. Saving closes the drawer and closing forgets the press, so allowing it would drop the publish with nothing on screen to say why -- a hole the topics rule had too. `awaitingTopicsForPublish` becomes `awaitingSettingsForPublish`, since it now covers either requirement, and the resume tests what the drawer just handed over rather than state that has not landed. `!topicsRequired` in that test is what keeps news from stranding: its drawer sends no `topics` at all, so a bare length check would never resume. Messages key on whether the content is published rather than on whether the save is refused. The two used to coincide and no longer do -- a draft's save is refused too while a publish waits -- and telling a draft it is published would simply be wrong. Co-Authored-By: Claude Opus 5 (1M context) * Refuse the settings save while a required field is blank The drawer marked both SEO fields required, put an asterisk on each label, and said an SEO title and description were needed to publish -- then let Save Settings through with both empty. Same for topics. The refusal now follows the requirement, on a draft as much as on something public. That collapses `topicsMayNotBeEmptied` and `seoMayNotBeEmptied` into `topicsRequired` and `seoRequired`: the two were always going to agree, and a second prop that only ever restated the first was the reason the button and the label could disagree in the first place. `contentIsPublished` stays, since it still picks which sentence a section shows. A draft's *content* is untouched by this -- autosave keeps writing it, and only the drawer's own settings wait on being complete. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Ahtesham Quraish Co-authored-by: Claude Opus 5 (1M context) --- .../WebsiteContentEditPage.happydom.test.tsx | 3 + .../ArticleSettingsDrawer.test.tsx | 4 +- .../ArticleSettings/ArticleSettingsDrawer.tsx | 102 ++++-- .../article/ArticleEditor.happydom.test.tsx | 346 +++++++++++++++--- .../news/NewsEditor.happydom.test.tsx | 130 ++++++- .../core/WebsiteContentEditor.tsx | 116 +++--- 6 files changed, 587 insertions(+), 114 deletions(-) diff --git a/frontends/main/src/app-pages/WebsiteContent/WebsiteContentEditPage.happydom.test.tsx b/frontends/main/src/app-pages/WebsiteContent/WebsiteContentEditPage.happydom.test.tsx index ee2d814956..ab2dab81ba 100644 --- a/frontends/main/src/app-pages/WebsiteContent/WebsiteContentEditPage.happydom.test.tsx +++ b/frontends/main/src/app-pages/WebsiteContent/WebsiteContentEditPage.happydom.test.tsx @@ -101,6 +101,9 @@ const setup = async (id: number, autosaveDelayMs = AUTOSAVE_OFF) => { content, content_type: "article", is_published: false, + /* Required to save the drawer, and not what these tests are about. */ + seo_title: "A title for search", + seo_description: "A description for search results.", }) setMockResponse.get(detailUrl(id), article) diff --git a/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.test.tsx b/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.test.tsx index e5132b36ae..1935e68c5b 100644 --- a/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.test.tsx +++ b/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.test.tsx @@ -187,7 +187,7 @@ describe("ArticleSettingsDrawer SEO fields", () => { mockTopics() const { onSave } = renderDrawer() - const field = await screen.findByLabelText("SEO Title") + const field = await screen.findByLabelText(/^SEO Title/) expect(field).toHaveAttribute("maxLength", "255") await userEvent.type(field, "x".repeat(260)) @@ -238,7 +238,7 @@ describe("ArticleSettingsDrawer SEO fields", () => { expect(under).toHaveAttribute("data-over-budget", "false") await userEvent.type( - await screen.findByLabelText("SEO Title"), + await screen.findByLabelText(/^SEO Title/), "x".repeat(50), ) diff --git a/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.tsx b/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.tsx index f38743cd91..f48999651a 100644 --- a/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.tsx +++ b/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.tsx @@ -199,17 +199,21 @@ const FooterCta = styled.div({ /** * What the topics section says about itself, which depends on which rule is - * speaking: one that refuses to save without a topic, one that only wants them - * before publishing, or neither. + * speaking: content already public that cannot be left without them, content + * that only needs them before it goes public, or neither. + * + * Keyed on `published` rather than on whether the save is refused: the two no + * longer coincide -- a draft's save is refused too while a publish is waiting + * on the drawer -- and telling a draft it is published would be simply wrong. */ const topicsMessage = ( contentLabel: string, empty: boolean, required: boolean, - mayNotBeEmptied: boolean, + published: boolean, ) => { const noun = contentLabel.toLowerCase() - if (empty && mayNotBeEmptied) { + if (empty && published) { return `A published ${noun} needs at least one topic` } if (empty && required) { @@ -218,6 +222,31 @@ const topicsMessage = ( return `Select one or more topics for your ${noun}` } +/** + * What the SEO section says about itself, on the same three rules as topics. + * + * Named for what is missing rather than "these fields are required": the + * drawer opens on its own when a publish is held back, and the first thing + * the author needs to know is why it did. + */ +const seoMessage = ( + contentLabel: string, + missing: boolean, + required: boolean, + published: boolean, +) => { + const noun = contentLabel.toLowerCase() + const always = + "Both should be unique to this page, and they are the first thing someone reads in search results." + if (missing && published) { + return `A published ${noun} needs an SEO title and description. ${always}` + } + if (missing && required) { + return `Add an SEO title and description to publish your ${noun}. ${always}` + } + return `Add an SEO title and description to help search engines understand and display your ${noun}. ${always}` +} + /** Settings the drawer collects. Mirrors the fields in the design. */ export interface ArticleSettingsValues { /** @@ -261,21 +290,30 @@ export interface ArticleSettingsDrawerProps { */ showTopics?: boolean /** - * Whether the content needs a topic before it can go public. The section - * says so while none is picked, which is what tells an editor why the - * drawer opened on them when they pressed Publish. + * Whether the content needs at least one topic. + * + * The section says so while none is picked -- which is what tells an editor + * why the drawer opened on them when they pressed Publish -- and the save is + * refused until one is. Refused rather than merely announced because this + * drawer is the one place a selection can be taken away, and because a + * caller holding a publish back is waiting on this save: letting it through + * incomplete would close the drawer and forget the press. */ topicsRequired?: boolean /** - * Whether an empty selection may not be saved at all. + * Whether the content needs an SEO title and description, on exactly the + * same terms as `topicsRequired`. * - * This drawer is the one place a selection can be taken away, so a caller - * that gates only its own save buttons would still lose the topics through - * here. Separate from `topicsRequired` because the two do not coincide: - * content that is not public yet can be left without topics -- it is - * stopped at publishing -- while content already public cannot. + * Unlike topics this is not an article-only rule: a search result and a link + * preview are the editor's to write on news just as much. */ - topicsMayNotBeEmptied?: boolean + seoRequired?: boolean + /** + * Whether the content is already public, which is only a matter of wording: + * which sentence a section shows when something it needs is missing. What is + * required, and what the save refuses, does not depend on it. + */ + contentIsPublished?: boolean /** Values to open with. Re-read each time the drawer opens. */ initialValues?: Partial /** @@ -294,7 +332,8 @@ const ArticleSettingsDrawer = ({ contentLabel = "Article", showTopics = true, topicsRequired = false, - topicsMayNotBeEmptied = false, + seoRequired = false, + contentIsPublished = false, initialValues, onSave, }: ArticleSettingsDrawerProps) => { @@ -313,6 +352,13 @@ const ArticleSettingsDrawer = ({ * Read here rather than at module scope, where NEXT_PUBLIC_* values are not * set yet. Missing, there is no suffix to reserve for. */ + /** + * Whitespace does not count as provided: a space would satisfy a bare + * emptiness check and reach the page head as a blank title, which is worse + * than the fallback it displaced. + */ + const seoMissing = !seoTitle.trim() || !seoDescription.trim() + const siteName = env("NEXT_PUBLIC_SITE_NAME") const titleSuffix = siteName ? ` | ${siteName}` : "" const seoTitleBudget = SEO_TITLE_TAG_BUDGET - titleSuffix.length @@ -514,7 +560,7 @@ const ArticleSettingsDrawer = ({ contentLabel, selectedIds.length === 0, topicsRequired, - topicsMayNotBeEmptied, + contentIsPublished, )} @@ -600,10 +646,12 @@ const ArticleSettingsDrawer = ({ SEO Settings - Add an SEO title and description to help search engines - understand and display your {contentLabel.toLowerCase()}. Both - should be unique to this page, and they are the first thing - someone reads in search results. + {seoMessage( + contentLabel, + seoMissing, + seoRequired, + contentIsPublished, + )}
@@ -611,6 +659,7 @@ const ArticleSettingsDrawer = ({ name="seo_title" label="SEO Title" fullWidth + required={seoRequired} placeholder="Enter a title for search results" /* The budget belongs in the description, not only in the counter: otherwise it is discoverable only by being run @@ -635,6 +684,7 @@ const ArticleSettingsDrawer = ({ name="seo_description" label="SEO Description" fullWidth + required={seoRequired} multiline /* Sized to the budget rather than to the space: nine rows read as an invitation to write far more than will ever show. */ @@ -662,10 +712,16 @@ const ArticleSettingsDrawer = ({