From 28707beb3c5171fcb25cf7988edd4f9796711e43 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Mon, 23 Mar 2026 14:46:21 +0000 Subject: [PATCH 1/3] feat: propagate categories from parent/child questions to conditional posts When a conditional post is created or updated, categories from the condition and condition_child questions' posts are now automatically added to the conditional post. Includes a data migration to backfill existing conditional posts. Closes #4524 Co-authored-by: Sylvain --- .../0030_backfill_conditional_categories.py | 59 +++++++++ posts/services/common.py | 41 ++++++ tests/unit/test_posts/test_services.py | 123 ++++++++++++++++++ 3 files changed, 223 insertions(+) create mode 100644 posts/migrations/0030_backfill_conditional_categories.py create mode 100644 tests/unit/test_posts/test_services.py diff --git a/posts/migrations/0030_backfill_conditional_categories.py b/posts/migrations/0030_backfill_conditional_categories.py new file mode 100644 index 0000000000..aaba7b5010 --- /dev/null +++ b/posts/migrations/0030_backfill_conditional_categories.py @@ -0,0 +1,59 @@ +from django.db import migrations + + +def backfill_conditional_categories(apps, schema_editor): + """ + Populate categories on conditional posts from their condition + and condition_child posts. + """ + Post = apps.get_model("posts", "Post") + + category_type = "category" + + conditional_posts = Post.objects.filter( + conditional__isnull=False, + ).select_related( + "conditional__condition", + "conditional__condition_child", + ) + + for post in conditional_posts: + conditional = post.conditional + categories_to_add = set() + + # Get categories from condition's post + condition_post_id = conditional.condition.post_id + if condition_post_id: + condition_post = Post.objects.get(pk=condition_post_id) + categories_to_add.update( + condition_post.projects.filter(type=category_type) + ) + + # Get categories from condition_child's post + child_post_id = conditional.condition_child.post_id + if child_post_id: + child_post = Post.objects.get(pk=child_post_id) + categories_to_add.update( + child_post.projects.filter(type=category_type) + ) + + if categories_to_add: + existing = set(post.projects.filter(type=category_type)) + new_categories = categories_to_add - existing + if new_categories: + post.projects.add(*new_categories) + + +class Migration(migrations.Migration): + + dependencies = [ + ("posts", "0029_remove_notebook_markdown_summary_and_more"), + ("projects", "0021_projectindex_project_index_projectindexpost"), + ] + + operations = [ + migrations.RunPython( + backfill_conditional_categories, + migrations.RunPython.noop, + ), + ] diff --git a/posts/services/common.py b/posts/services/common.py index 169c198772..aefa4276fb 100644 --- a/posts/services/common.py +++ b/posts/services/common.py @@ -51,6 +51,41 @@ logger = logging.getLogger(__name__) +def get_conditional_categories(conditional) -> list[Project]: + """Get the union of categories from a conditional's condition and condition_child posts.""" + category_type = Project.ProjectTypes.CATEGORY + categories = set() + + condition_post = conditional.condition.get_post() + if condition_post: + categories.update( + condition_post.projects.filter(type=category_type) + ) + + condition_child_post = conditional.condition_child.get_post() + if condition_child_post: + categories.update( + condition_child_post.projects.filter(type=category_type) + ) + + return list(categories) + + +def sync_conditional_categories(post: Post): + """Sync the categories of a conditional post with its parent/child question categories.""" + if not post.conditional_id: + return + + conditional_categories = get_conditional_categories(post.conditional) + if conditional_categories: + existing_categories = set( + post.projects.filter(type=Project.ProjectTypes.CATEGORY) + ) + new_categories = set(conditional_categories) - existing_categories + if new_categories: + post.projects.add(*new_categories) + + def add_categories(categories: list[int], post: Post): existing = [x.pk for x in post.projects.filter(type=Project.ProjectTypes.CATEGORY)] categories = [x for x in categories if x not in existing] @@ -170,6 +205,10 @@ def create_post( obj.projects.add(*categories) + # Propagate categories from condition and condition_child posts + if obj.conditional_id: + sync_conditional_categories(obj) + # Update global leaderboard tags update_global_leaderboard_tags(obj) @@ -284,6 +323,8 @@ def update_post( raise ValidationError("Original post does is not a conditional") update_conditional(post.conditional, **conditional) + # Re-sync categories after conditional update + sync_conditional_categories(post) if group_of_questions: if not post.group_of_questions: diff --git a/tests/unit/test_posts/test_services.py b/tests/unit/test_posts/test_services.py new file mode 100644 index 0000000000..531c5b6fd5 --- /dev/null +++ b/tests/unit/test_posts/test_services.py @@ -0,0 +1,123 @@ +import pytest + +from posts.services.common import ( + get_conditional_categories, + sync_conditional_categories, +) +from projects.models import Project +from questions.models import Question +from tests.unit.test_posts.factories import factory_post +from tests.unit.test_projects.factories import factory_project +from tests.unit.test_questions.factories import create_conditional, create_question + + +class TestConditionalCategoryPropagation: + def _make_conditional_setup(self): + """Create condition and child questions with their own posts and categories.""" + cat_a = factory_project(type=Project.ProjectTypes.CATEGORY, name="Category A") + cat_b = factory_project(type=Project.ProjectTypes.CATEGORY, name="Category B") + cat_c = factory_project(type=Project.ProjectTypes.CATEGORY, name="Category C") + + condition = create_question( + question_type=Question.QuestionType.BINARY, + ) + condition_child = create_question( + question_type=Question.QuestionType.BINARY, + ) + + condition_post = factory_post( + question=condition, + projects=[cat_a, cat_b], + ) + child_post = factory_post( + question=condition_child, + projects=[cat_b, cat_c], + ) + + conditional = create_conditional( + condition=condition, + condition_child=condition_child, + question_yes=create_question( + question_type=Question.QuestionType.BINARY, + title="If Yes", + ), + question_no=create_question( + question_type=Question.QuestionType.BINARY, + title="If No", + ), + ) + + return conditional, cat_a, cat_b, cat_c + + def test_get_conditional_categories(self): + conditional, cat_a, cat_b, cat_c = self._make_conditional_setup() + + categories = get_conditional_categories(conditional) + category_ids = {c.id for c in categories} + + assert cat_a.id in category_ids + assert cat_b.id in category_ids + assert cat_c.id in category_ids + + def test_sync_conditional_categories(self): + conditional, cat_a, cat_b, cat_c = self._make_conditional_setup() + + post = factory_post(conditional=conditional) + sync_conditional_categories(post) + + post_categories = set( + post.projects.filter(type=Project.ProjectTypes.CATEGORY) + ) + assert cat_a in post_categories + assert cat_b in post_categories + assert cat_c in post_categories + + def test_sync_conditional_categories_no_duplicates(self): + conditional, cat_a, cat_b, cat_c = self._make_conditional_setup() + + post = factory_post(conditional=conditional, projects=[cat_a]) + sync_conditional_categories(post) + + cat_a_count = post.projects.filter( + type=Project.ProjectTypes.CATEGORY, id=cat_a.id + ).count() + assert cat_a_count == 1 + + def test_sync_noop_for_non_conditional(self): + question = create_question( + question_type=Question.QuestionType.BINARY, + ) + post = factory_post(question=question) + + # Should not raise + sync_conditional_categories(post) + + def test_get_conditional_categories_missing_posts(self): + """Categories should be collected even if one parent has no post.""" + condition = create_question( + question_type=Question.QuestionType.BINARY, + ) + condition_child = create_question( + question_type=Question.QuestionType.BINARY, + ) + + cat = factory_project(type=Project.ProjectTypes.CATEGORY, name="Only Cat") + factory_post(question=condition, projects=[cat]) + # condition_child has no post (post_id is None) + + conditional = create_conditional( + condition=condition, + condition_child=condition_child, + question_yes=create_question( + question_type=Question.QuestionType.BINARY, + title="If Yes", + ), + question_no=create_question( + question_type=Question.QuestionType.BINARY, + title="If No", + ), + ) + + categories = get_conditional_categories(conditional) + assert len(categories) == 1 + assert categories[0].id == cat.id From ba57d017054c95080debb82d0f4e599f6051054f Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Wed, 8 Apr 2026 16:10:38 +0000 Subject: [PATCH 2/3] fix: resolve ruff formatting/linting and test collection errors - Move test_services.py to test_services/test_conditional_categories.py to avoid pytest module name collision with existing test_services/ dir - Remove unused pytest import (F401) - Remove unused variable assignments condition_post, child_post (F841) - Fix ruff formatting (collapse single-line expressions) Co-authored-by: Sylvain --- posts/services/common.py | 8 ++------ .../test_conditional_categories.py} | 10 +++------- 2 files changed, 5 insertions(+), 13 deletions(-) rename tests/unit/test_posts/{test_services.py => test_services/test_conditional_categories.py} (95%) diff --git a/posts/services/common.py b/posts/services/common.py index aefa4276fb..e6db58996b 100644 --- a/posts/services/common.py +++ b/posts/services/common.py @@ -58,15 +58,11 @@ def get_conditional_categories(conditional) -> list[Project]: condition_post = conditional.condition.get_post() if condition_post: - categories.update( - condition_post.projects.filter(type=category_type) - ) + categories.update(condition_post.projects.filter(type=category_type)) condition_child_post = conditional.condition_child.get_post() if condition_child_post: - categories.update( - condition_child_post.projects.filter(type=category_type) - ) + categories.update(condition_child_post.projects.filter(type=category_type)) return list(categories) diff --git a/tests/unit/test_posts/test_services.py b/tests/unit/test_posts/test_services/test_conditional_categories.py similarity index 95% rename from tests/unit/test_posts/test_services.py rename to tests/unit/test_posts/test_services/test_conditional_categories.py index 531c5b6fd5..9e5475aa55 100644 --- a/tests/unit/test_posts/test_services.py +++ b/tests/unit/test_posts/test_services/test_conditional_categories.py @@ -1,5 +1,3 @@ -import pytest - from posts.services.common import ( get_conditional_categories, sync_conditional_categories, @@ -25,11 +23,11 @@ def _make_conditional_setup(self): question_type=Question.QuestionType.BINARY, ) - condition_post = factory_post( + factory_post( question=condition, projects=[cat_a, cat_b], ) - child_post = factory_post( + factory_post( question=condition_child, projects=[cat_b, cat_c], ) @@ -65,9 +63,7 @@ def test_sync_conditional_categories(self): post = factory_post(conditional=conditional) sync_conditional_categories(post) - post_categories = set( - post.projects.filter(type=Project.ProjectTypes.CATEGORY) - ) + post_categories = set(post.projects.filter(type=Project.ProjectTypes.CATEGORY)) assert cat_a in post_categories assert cat_b in post_categories assert cat_c in post_categories From 9bd63cd698d7ed3f0858340204b4e81e4afdf29e Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Wed, 15 Apr 2026 08:13:14 +0000 Subject: [PATCH 3/3] fix: re-sync conditional categories on any update_post call Move sync_conditional_categories out of the `if conditional:` branch so category-only edits (update_post(post, categories=[...])) don't drop categories inherited from the parent/child question posts. Adds a regression test that calls update_post with only a categories payload and asserts inherited categories survive. Co-authored-by: Sylvain --- posts/services/common.py | 7 +++- .../test_conditional_categories.py | 37 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/posts/services/common.py b/posts/services/common.py index e6db58996b..a3b1ff3a46 100644 --- a/posts/services/common.py +++ b/posts/services/common.py @@ -319,7 +319,12 @@ def update_post( raise ValidationError("Original post does is not a conditional") update_conditional(post.conditional, **conditional) - # Re-sync categories after conditional update + + # Re-sync inherited categories for conditional posts after any update. + # This runs outside the `if conditional:` branch so that category-only + # edits (which replace the category set via post.projects.set above) + # don't silently drop categories inherited from the parent/child posts. + if post.conditional_id: sync_conditional_categories(post) if group_of_questions: diff --git a/tests/unit/test_posts/test_services/test_conditional_categories.py b/tests/unit/test_posts/test_services/test_conditional_categories.py index 9e5475aa55..ea2bb47edc 100644 --- a/tests/unit/test_posts/test_services/test_conditional_categories.py +++ b/tests/unit/test_posts/test_services/test_conditional_categories.py @@ -1,6 +1,7 @@ from posts.services.common import ( get_conditional_categories, sync_conditional_categories, + update_post, ) from projects.models import Project from questions.models import Question @@ -88,6 +89,42 @@ def test_sync_noop_for_non_conditional(self): # Should not raise sync_conditional_categories(post) + def test_update_post_categories_preserves_inherited(self, mocker): + """ + Regression: calling update_post(..., categories=[...]) on a conditional + post must not drop categories inherited from parent/child questions, + even when no `conditional` payload is passed. + """ + # Avoid downstream side effects that aren't relevant to this test + mocker.patch("posts.tasks.run_post_indexing.send") + + conditional, cat_a, cat_b, cat_c = self._make_conditional_setup() + post = factory_post(conditional=conditional) + sync_conditional_categories(post) + + # Sanity check: inherited categories are present before the update + initial_categories = set( + post.projects.filter(type=Project.ProjectTypes.CATEGORY) + ) + assert {cat_a, cat_b, cat_c} <= initial_categories + + # User edits the post with only cat_a in the categories payload + update_post(post, categories=[cat_a]) + + post_categories = set( + post.projects.filter(type=Project.ProjectTypes.CATEGORY) + ) + # Inherited categories must still be present + assert cat_a in post_categories + assert cat_b in post_categories + assert cat_c in post_categories + + # And cat_a should not have been duplicated + cat_a_count = post.projects.filter( + type=Project.ProjectTypes.CATEGORY, id=cat_a.id + ).count() + assert cat_a_count == 1 + def test_get_conditional_categories_missing_posts(self): """Categories should be collected even if one parent has no post.""" condition = create_question(