Change bill creation flow (#298)

* Refactor bill creation flow and make submit task nullable

- Move submit task creation into Bill.save() override instead of the
  BillManager context manager, keeping the logic self-contained in the model
- Make _submit_task nullable; draft bills no longer get a task on creation —
  _schedule_submit_task() is called from save() for non-draft bills and from
  publish() when a draft is opened for voting
- Simplify BillManager.create_from_github and remove TaskFactory from tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Moved logic from view to manager

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: mfosterw

democrasite/webiscite/managers.py

@@ -1,22 +1,14 @@
-import json
-from collections.abc import Iterator
-from contextlib import contextmanager
-from logging import getLogger
+from logging import WARNING
 from typing import TYPE_CHECKING
 from typing import Any
 
-from django.conf import settings
+import requests
 from django.db import models
-from django.utils import timezone
-from django_celery_beat.models import IntervalSchedule
-from django_celery_beat.models import PeriodicTask
 
 from democrasite.users.models import User
 
 from .constitution import is_constitutional
 
-logger = getLogger(__name__)
-
 if TYPE_CHECKING:
     from .models import Bill  # pragma: no cover
     from .models import PullRequest  # pragma: no cover
@@ -102,85 +94,49 @@ def annotate_user_vote(
 
     def create_from_github(
         self,
-        title: str,
-        body: str,
-        author: User,
-        diff_text: str,
         pull_request: "PullRequest",
-    ) -> T:
+        desc: str,
+        author_uid: str,
+    ) -> T | None:
         """Validate and create a :class:`~democrasite.webiscite.models.Bill` from a
         GitHub pull request
 
         Args:
-            title: The title of the pull request
-            body: The body of the pull request
-            author: The user who created the pull request
-            diff_text: The text of the diff of the pull request
             pull_request: The pull request instance to associate with the bill
+            desc: The description of the bill (pull request body)
+            author_uid: The GitHub UID of the pull request author
 
         Returns:
-            The new bill instance
+            The new bill instance, or None if the author has no linked account
         """
-        draft = pull_request.draft
-        with self._create_submit_task(enabled=not draft) as submit_task:
-            self._bill: Bill = self.model(
-                name=title,
-                description=body,
-                author=author,
-                pull_request=pull_request,
-                status=self.model.Status.DRAFT if draft else self.model.Status.OPEN,
-                constitutional=bool(is_constitutional(diff_text)),
-                _submit_task=submit_task,
+        try:
+            author = User.objects.filter(socialaccount__provider="github").get(
+                socialaccount__uid=author_uid
             )
-            self._bill.full_clean()
-            self._bill.save()
-            bill = self._bill
-            bill.log("Created")
-
-        return bill
-
-    @contextmanager
-    def _create_submit_task(self, *, enabled: bool = True) -> Iterator[PeriodicTask]:
-        """Schedule a task to submit this bill for voting
+        except User.DoesNotExist:
+            # If the creator of the pull request does not have a linked account,
+            # a Bill cannot be created and the pr is ignored.
+            pull_request.log("No bill created (user does not exist)", level=WARNING)
+            return None
 
-        Args:
-            enabled: Whether the task should be enabled immediately. Set to False
-                for draft bills whose voting period hasn't started yet.
+        diff_text = requests.get(pull_request.diff_url, timeout=10).text
+        constitutional = bool(is_constitutional(diff_text))
 
-        Returns:
-            The task that was scheduled
-        """
-        # This might be better as a signal but I want to keep it localized
-        voting_ends, __ = IntervalSchedule.objects.get_or_create(
-            every=settings.WEBISCITE_VOTING_PERIOD, period=IntervalSchedule.DAYS
+        status = (
+            self.model.Status.DRAFT if pull_request.draft else self.model.Status.OPEN
         )
 
-        submit_task = PeriodicTask.objects.create(
-            interval=voting_ends,
-            name="bill_submit:temp",
-            task="democrasite.webiscite.tasks.submit_bill",
-            one_off=True,
-            enabled=enabled,
-            # If last_run_at is not set, the task will run relative to when the
-            # scheduler started, not when it was created
-            last_run_at=timezone.now(),
+        bill = self.model(
+            name=pull_request.title,
+            description=desc,
+            author=author,
+            pull_request=pull_request,
+            status=status,
+            constitutional=constitutional,
         )
-        try:
-            yield submit_task
-        finally:
-            if not (
-                hasattr(self, "_bill")
-                and hasattr(self._bill, "id")
-                and isinstance(self._bill.id, int)
-            ):
-                raise AttributeError(
-                    "self._bill was not saved in the submit task context"
-                )
-
-            submit_task.name = f"bill_submit:{self._bill.id}"
-            submit_task.args = json.dumps([self._bill.id])
-            submit_task.save()
-            self._bill.log("Scheduled %s", submit_task.name)
-
-            # Attribute could be shared between model instances
-            del self._bill
+
+        bill.full_clean()
+        bill.save()
+        bill.log("Created")
+
+        return bill

democrasite/webiscite/migrations/0007_alter_bill__submit_task.py

@@ -0,0 +1,20 @@
+# Generated by Django 5.2.11 on 2026-02-22 17:54
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('django_celery_beat', '0019_alter_periodictasks_options'),
+        ('webiscite', '0006_remove_bill_unique_open_pull_request_and_more'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='bill',
+            name='_submit_task',
+            field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, to='django_celery_beat.periodictask'),
+        ),
+    ]

democrasite/webiscite/models.py

@@ -1,12 +1,14 @@
 """Models for the webiscite app"""
 
+import json
 import logging
 
 from django.conf import settings
 from django.db import models
 from django.urls import reverse
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
+from django_celery_beat.models import IntervalSchedule
 from django_celery_beat.models import PeriodicTask
 from model_utils.models import TimeStampedModel
 from simple_history.models import HistoricalRecords
@@ -147,7 +149,9 @@ class Status(models.TextChoices):
 
     # Automatic fields
     votes = models.ManyToManyField(User, through=Vote, related_name="votes", blank=True)
-    _submit_task = models.OneToOneField(PeriodicTask, on_delete=models.PROTECT)
+    _submit_task = models.OneToOneField(
+        PeriodicTask, on_delete=models.PROTECT, null=True, blank=True
+    )
 
     history = HistoricalRecords()
 
@@ -244,15 +248,39 @@ def user_supports(self, user: User) -> bool | None:
         else:
             return vote.support
 
+    def save(self, *args, **kwargs):
+        created = self._state.adding
+        super().save(*args, **kwargs)
+        if created and self.status != self.Status.DRAFT:
+            self._schedule_submit_task()
+
+    def _schedule_submit_task(self) -> None:
+        """Create a periodic task to submit this bill after the voting period."""
+
+        voting_ends, __ = IntervalSchedule.objects.get_or_create(
+            every=settings.WEBISCITE_VOTING_PERIOD, period=IntervalSchedule.DAYS
+        )
+        self._submit_task = PeriodicTask.objects.create(
+            interval=voting_ends,
+            name=f"bill_submit:{self.id}",
+            task="democrasite.webiscite.tasks.submit_bill",
+            args=json.dumps([self.id]),
+            one_off=True,
+            last_run_at=timezone.now(),
+        )
+        super().save()
+        self.log("Scheduled %s", self._submit_task.name)
+
     def close(self) -> None:
         """Close the bill and disable its submit task"""
         self.status = self.Status.CLOSED
         self.save()
         self.log("Closed")
 
-        self._submit_task.enabled = False
-        self._submit_task.save()
-        self.log("Submit task disabled")
+        if self._submit_task is not None:
+            self._submit_task.enabled = False
+            self._submit_task.save()
+            self.log("Submit task disabled")
 
     def publish(self) -> None:
         """Transition a draft bill to open, enabling voting and scheduling submission"""
@@ -261,10 +289,7 @@ def publish(self) -> None:
 
         self.status = self.Status.OPEN
         self.save()
-
-        self._submit_task.enabled = True
-        self._submit_task.last_run_at = timezone.now()
-        self._submit_task.save()
+        self._schedule_submit_task()
         self.log("Published")
 
     def submit(self) -> None:

democrasite/webiscite/tests/factories.py

@@ -1,8 +1,6 @@
 from typing import Any
 
 import factory
-from django_celery_beat.models import IntervalSchedule
-from django_celery_beat.models import PeriodicTask
 
 from democrasite.users.tests.factories import UserFactory
 from democrasite.webiscite.models import Bill
@@ -25,18 +23,6 @@ class Meta:
         model = PullRequest
 
 
-class TaskFactory(factory.django.DjangoModelFactory[PeriodicTask]):
-    interval = factory.LazyFunction(
-        lambda: IntervalSchedule.objects.get_or_create(
-            every=999, period=IntervalSchedule.DAYS
-        )[0]
-    )
-    name = factory.Faker("text", max_nb_chars=50)
-
-    class Meta:
-        model = PeriodicTask
-
-
 class BillFactory(factory.django.DjangoModelFactory[Bill]):
     name = factory.Faker("text", max_nb_chars=50)
     description = factory.Faker("paragraph")
@@ -45,9 +31,7 @@ class BillFactory(factory.django.DjangoModelFactory[Bill]):
     # Fields with defaults
     status = Bill.Status.OPEN
     constitutional = False
-    _submit_task = factory.SubFactory(TaskFactory)
-    # Currently yes_votes and no_votes are initialized as empty. If values are needed
-    # for them, a post-generation hook can be written to generate and insert the users
+    # _submit_task is created by Bill.save() for non-draft bills
 
     class Meta:
         model = Bill

democrasite/webiscite/tests/test_models.py

@@ -1,4 +1,7 @@
+from unittest.mock import patch
+
 import pytest
+from allauth.socialaccount.models import SocialAccount
 from django.conf import settings
 from django.db import IntegrityError
 from factory.faker import faker
@@ -125,43 +128,43 @@ def test_annotate_user_vote(self, bill: Bill, user: User):
         bill.vote(user, support=False)
         assert Bill.objects.annotate_user_vote(user).first().user_vote is None
 
-    def test_create_from_github(self, user: User):
+    @patch("requests.get")
+    def test_create_from_github(self, mock_get, user: User):
+        mock_get.return_value.text = ""
         pr = PullRequestFactory.create()
-        bill = Bill.objects.create_from_github(
-            pr.title,
-            FAKE.text(),
-            user,
-            FAKE.text(),
-            pr,
-        )
+        uid = SocialAccount.objects.create(
+            user=user, provider="github", uid=FAKE.random_int()
+        ).uid
+
+        bill = Bill.objects.create_from_github(pr, FAKE.text(), uid)
 
+        assert bill is not None
         assert bill.pk is not None
+        bill.refresh_from_db()
+        assert bill._submit_task is not None  # noqa: SLF001
         assert bill._submit_task.enabled is True  # noqa: SLF001
 
-    def test_create_from_github_draft(self, user: User):
+    @patch("requests.get")
+    def test_create_from_github_draft(self, mock_get, user: User):
+        mock_get.return_value.text = ""
         pr = PullRequestFactory.create(draft=True)
-        bill = Bill.objects.create_from_github(
-            pr.title,
-            FAKE.text(),
-            user,
-            FAKE.text(),
-            pr,
-        )
+        uid = SocialAccount.objects.create(
+            user=user, provider="github", uid=FAKE.random_int()
+        ).uid
 
+        bill = Bill.objects.create_from_github(pr, FAKE.text(), uid)
+
+        assert bill is not None
         assert bill.pk is not None
         assert bill.status == Bill.Status.DRAFT
-        assert bill._submit_task.enabled is False  # noqa: SLF001
+        assert bill._submit_task is None  # noqa: SLF001
+
+    def test_create_from_github_no_user(self):
+        pr = PullRequestFactory.create()
+
+        result = Bill.objects.create_from_github(pr, FAKE.text(), "nonexistent-uid")
 
-    def test__create_submit_task(self):
-        # just hit the error branch of finally clause
-        with (
-            pytest.raises(
-                AttributeError,
-                match=r"self._bill was not saved in the submit task context",
-            ),
-            Bill.objects._create_submit_task(),  # noqa: SLF001
-        ):
-            pass
+        assert result is None
 
 
 class TestBill:
@@ -194,25 +197,27 @@ def test_get_vote_url(self, bill: Bill):
         assert bill.get_vote_url() == f"/bills/{bill.id}/vote/"
 
     def test_close(self, bill: Bill):
+        assert bill._submit_task is not None  # noqa: SLF001
         assert bill._submit_task.enabled is True  # noqa: SLF001
 
         bill.close()
 
         bill.refresh_from_db()
         assert bill.status == Bill.Status.CLOSED
+        assert bill._submit_task is not None  # noqa: SLF001
         assert bill._submit_task.enabled is False  # noqa: SLF001
 
 
 class TestBillPublish:
     def test_publish(self):
         bill = BillFactory.create(status=Bill.Status.DRAFT)
-        bill._submit_task.enabled = False  # noqa: SLF001
-        bill._submit_task.save()  # noqa: SLF001
+        assert bill._submit_task is None  # noqa: SLF001
 
         bill.publish()
 
         bill.refresh_from_db()
         assert bill.status == Bill.Status.OPEN
+        assert bill._submit_task is not None  # noqa: SLF001
         assert bill._submit_task.enabled is True  # noqa: SLF001
         assert bill._submit_task.last_run_at is not None  # noqa: SLF001