diff --git a/.claude/skills/new-bill-feature/SKILL.md b/.claude/skills/new-bill-feature/SKILL.md index ac0b6bb..e00bcd5 100644 --- a/.claude/skills/new-bill-feature/SKILL.md +++ b/.claude/skills/new-bill-feature/SKILL.md @@ -48,7 +48,7 @@ The Bill system spans these files. Review each to determine if it needs changes: - `config/settings/base.py` — WEBISCITE_* settings (quorum, majority thresholds, voting period, GitHub token/repo) ## Key patterns to follow -- Bill status choices: DRAFT, OPEN, APPROVED, REJECTED, FAILED, CLOSED +- Bill status choices: DRAFT, OPEN, APPROVED, AMENDED, REJECTED, FAILED, CLOSED - Votes are M2M through Vote model with unique constraint on (bill, user) - Bill.vote() toggles existing votes; raises ClosedBillVoteError if bill not OPEN - Constitutional bills need WEBISCITE_SUPERMAJORITY (66.67%), normal need WEBISCITE_NORMAL_MAJORITY (50%) @@ -62,6 +62,8 @@ The Bill system spans these files. Review each to determine if it needs changes: - PullRequest.close() closes both open and draft bills - The submit PeriodicTask is created disabled for draft bills; Bill.publish() enables it and resets last_run_at so the voting period starts from publication - GitHub's `ready_for_review` webhook action triggers PullRequestHandler.ready_for_review(), which updates the PR and publishes the draft bill +- GitHub's `synchronize` webhook action (new commits pushed to PR) triggers PullRequestHandler.synchronize(), which updates the PR SHA and closes any open bill with AMENDED status; draft bills are not affected +- Bill.close() accepts an optional `status` parameter (default Bill.Status.CLOSED) to set a different terminal status (e.g. AMENDED) ## Steps 1. Read the relevant files from the checklist above diff --git a/democrasite/webiscite/migrations/0008_alter_bill_status_alter_historicalbill_status.py b/democrasite/webiscite/migrations/0008_alter_bill_status_alter_historicalbill_status.py new file mode 100644 index 0000000..e57f892 --- /dev/null +++ b/democrasite/webiscite/migrations/0008_alter_bill_status_alter_historicalbill_status.py @@ -0,0 +1,23 @@ +# Generated by Django 5.2.11 on 2026-02-24 20:20 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('webiscite', '0007_alter_bill__submit_task'), + ] + + operations = [ + migrations.AlterField( + model_name='bill', + name='status', + field=models.CharField(choices=[('draft', 'Draft'), ('open', 'Open'), ('approved', 'Approved'), ('amended', 'PR Amended'), ('rejected', 'Rejected'), ('failed', 'Not Enough Votes'), ('closed', 'PR Closed')], default='open', help_text='The current status of the bill', max_length=10), + ), + migrations.AlterField( + model_name='historicalbill', + name='status', + field=models.CharField(choices=[('draft', 'Draft'), ('open', 'Open'), ('approved', 'Approved'), ('amended', 'PR Amended'), ('rejected', 'Rejected'), ('failed', 'Not Enough Votes'), ('closed', 'PR Closed')], default='open', help_text='The current status of the bill', max_length=10), + ), + ] diff --git a/democrasite/webiscite/models.py b/democrasite/webiscite/models.py index abb0f46..1962efc 100644 --- a/democrasite/webiscite/models.py +++ b/democrasite/webiscite/models.py @@ -130,6 +130,7 @@ class Status(models.TextChoices): DRAFT = "draft", _("Draft") OPEN = "open", _("Open") APPROVED = "approved", _("Approved") + AMENDED = "amended", _("PR Amended") # PR updated with new commits REJECTED = "rejected", _("Rejected") FAILED = "failed", _("Not Enough Votes") # Failed to reach quorum # Translators: PR is short for "pull request" @@ -271,9 +272,9 @@ def _schedule_submit_task(self) -> None: super().save() self.log("Scheduled %s", self._submit_task.name) - def close(self) -> None: + def close(self, status: "Bill.Status" = Status.CLOSED) -> None: """Close the bill and disable its submit task""" - self.status = self.Status.CLOSED + self.status = status self.save() self.log("Closed") diff --git a/democrasite/webiscite/tests/test_models.py b/democrasite/webiscite/tests/test_models.py index e86aa80..e71cfe9 100644 --- a/democrasite/webiscite/tests/test_models.py +++ b/democrasite/webiscite/tests/test_models.py @@ -207,6 +207,17 @@ def test_close(self, bill: Bill): assert bill._submit_task is not None # noqa: SLF001 assert bill._submit_task.enabled is False # noqa: SLF001 + def test_close_amended(self, bill: Bill): + assert bill._submit_task is not None # noqa: SLF001 + assert bill._submit_task.enabled is True # noqa: SLF001 + + bill.close(status=Bill.Status.AMENDED) + + bill.refresh_from_db() + assert bill.status == Bill.Status.AMENDED + assert bill._submit_task is not None # noqa: SLF001 + assert bill._submit_task.enabled is False # noqa: SLF001 + class TestBillPublish: def test_publish(self): diff --git a/democrasite/webiscite/tests/test_webhooks.py b/democrasite/webiscite/tests/test_webhooks.py index c68f177..77b8f62 100644 --- a/democrasite/webiscite/tests/test_webhooks.py +++ b/democrasite/webiscite/tests/test_webhooks.py @@ -217,3 +217,25 @@ def test_closed(self, mock_close, pr_handler: PullRequestHandler, bill: Bill): assert response == (bill.pull_request, mock_close.return_value) mock_close.assert_called_once() + + def test_synchronize(self, pr_handler: PullRequestHandler): + bill = BillFactory.create(status=Bill.Status.OPEN) + pr = GithubPullRequestFactory.create(bill=bill) + + pull_request, amended_bill = pr_handler.synchronize(pr) + + assert pull_request is not None + assert amended_bill is not None + amended_bill.refresh_from_db() + assert amended_bill.status == Bill.Status.AMENDED + assert amended_bill._submit_task is not None # noqa: SLF001 + assert amended_bill._submit_task.enabled is False # noqa: SLF001 + + def test_synchronize_no_open_bill(self, pr_handler: PullRequestHandler): + # Default GithubPullRequestFactory creates a closed bill (no active bill) + pr = GithubPullRequestFactory.create() + + pull_request, bill = pr_handler.synchronize(pr) + + assert pull_request is not None + assert bill is None diff --git a/democrasite/webiscite/webhooks.py b/democrasite/webiscite/webhooks.py index 5e8bf21..271b275 100644 --- a/democrasite/webiscite/webhooks.py +++ b/democrasite/webiscite/webhooks.py @@ -115,6 +115,29 @@ def ready_for_review( bill.publish() return (pull_request, bill) + def synchronize(self, pr: dict[str, Any]) -> tuple[PullRequest, Bill | None]: + """Handle new commits pushed to an open pull request. + + Updates the pull request with the new SHA and closes any active bill + as amended, since votes on the old version no longer apply. + + Args: + pr: The parsed JSON object representing the pull request + + Returns: + A tuple containing the updated pull request and the closed bill, if any + """ + pull_request = PullRequest.objects.create_from_github(pr) + + try: + bill: Bill = pull_request.bill_set.get(status=Bill.Status.OPEN) + except Bill.DoesNotExist: + pull_request.log("No open bill found") + return (pull_request, None) + + bill.close(status=Bill.Status.AMENDED) + return (pull_request, bill) + def closed(self, pr: dict[str, Any]) -> tuple[PullRequest | None, Bill | None]: """Disables the open bill associated with the pull request diff --git a/docs/webiscite.rst b/docs/webiscite.rst index 73feb61..bf73557 100644 --- a/docs/webiscite.rst +++ b/docs/webiscite.rst @@ -41,5 +41,12 @@ If the votes for the Bill pass the threshold, the pull request is merged into the master branch on Github and automatically deployed, officially making it part of Democrasite. +If new commits are pushed to a pull request while a Bill is open for voting, +GitHub sends a ``synchronize`` event. +:meth:`~democrasite.webiscite.webhooks.PullRequestHandler.synchronize` +updates the stored pull request with the new SHA and closes the open Bill +with an ``AMENDED`` status, since the votes no longer reflect the current +code. Draft bills are not affected by synchronize events. + .. _GitHub: https://github.com/mfosterw/cookiestocracy .. _webhook: https://docs.github.com/en/developers/webhooks-and-events/webhooks/about-webhooks