fix: run checks when policies attach, drop duplicated approval reasons, trim the markdown hint

Author: antoinekh

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## Unreleased
+
+### Fixed
+
+- Pre-merge checks sat at **pending** until somebody pressed Re-run. A change request is created before its policies are matched, and every built-in check is policy-scoped, so the run at creation found no checks to run. Attaching or detaching a policy now runs them.
+- The approval panel repeated each rule's shortfall in prose below a table that already showed the same counts. The page now shows only what the table cannot: a rejection, stale reviews, or no rules applying at all. The merge gate and the REST API still get the full text, having no table to read.
+- The review form's Markdown hint listed examples the live preview already demonstrates.
+
 ## 0.1.0 - 2026-08-27
 
 First release. Policy-driven change control and mandatory review for NetBox branches.

netbox_change_control/policy.py

@@ -75,7 +75,24 @@ def outstanding(self):
 
     def reasons(self):
         """
-        Human readable explanations of why the request is not satisfied.
+        Every reason the request is not satisfied, including each rule's shortfall.
+
+        For callers with no rule table in front of them: the merge gate's refusal message,
+        the REST API, a log line.
+        """
+        return self.other_reasons() + [
+            f'Rule "{rule.rule.name}" needs {rule.outstanding} more approval(s) of {rule.required}.'
+            for rule in self.rules
+            if not rule.satisfied
+        ]
+
+    def other_reasons(self):
+        """
+        Only the reasons a rule table does not already show.
+
+        The change request page lists every rule with its count, so repeating "needs 1 more
+        approval of 1" underneath says the same thing twice. A rejection, a stale review or
+        the absence of any rule is not visible in that table, so it belongs here.
         """
         messages = []
         for review in self.rejections:
@@ -85,11 +102,6 @@ def reasons(self):
             messages.append(f'The branch changed after these reviews were submitted, so they no longer count: {names}.')
         if not self.rules:
             messages.append('No policy rules apply to this change request.')
-        for rule in self.rules:
-            if not rule.satisfied:
-                messages.append(
-                    f'Rule "{rule.rule.name}" needs {rule.outstanding} more approval(s) of {rule.required}.'
-                )
         return messages
 
 

netbox_change_control/signal_receivers.py

@@ -189,7 +189,22 @@ def emit_on_review_submitted(sender, instance, created, **kwargs):
 
 @receiver([post_save, post_delete], sender=ChangeRequestPolicy)
 def refresh_on_policy_binding_change(sender, instance, **kwargs):
-    _refresh(instance.change_request_id)
+    """
+    Attaching or detaching a policy changes which checks apply, not just who must approve.
+
+    Every built-in check is policy-scoped, and a change request is created before its
+    policies are matched, so the run at creation sees no policies and therefore no checks.
+    Without running them here the panel sits at "pending" until somebody presses Re-run.
+    """
+    if _is_being_deleted(instance.change_request_id):
+        return
+    change_request = ChangeRequest.objects.filter(pk=instance.change_request_id).first()
+    if change_request is None:
+        return
+
+    # Status first: run_checks may auto-merge, and it should decide against a current status.
+    refresh_status(change_request)
+    run_checks(change_request)
 
 
 def _refresh(change_request_id):

netbox_change_control/templates/netbox_change_control/changerequest.html

@@ -147,11 +147,13 @@ <h2 class="card-header d-flex justify-content-between align-items-center">
         <tr><td colspan="2" class="text-muted">{% trans "No policy rules apply yet. Submit the request for review." %}</td></tr>
         {% endfor %}
       </table>
-      {% if not evaluation.satisfied %}
+      {% with notes=evaluation.other_reasons %}
+      {% if notes %}
       <div class="card-footer small text-muted">
-        {% for reason in evaluation.reasons %}<div>{{ reason }}</div>{% endfor %}
+        {% for note in notes %}<div>{{ note }}</div>{% endfor %}
       </div>
       {% endif %}
+      {% endwith %}
       <div class="card-footer p-0">
         <table class="table table-hover attr-table mb-0">
           <tr><th colspan="2" class="small text-uppercase text-muted">{% trans "Pre-merge checks" %}</th></tr>

netbox_change_control/templates/netbox_change_control/inc/review_form.html

@@ -19,7 +19,7 @@
                 placeholder="{% trans 'Comment' %}">{{ own_review.comment|default:'' }}</textarea>
       <div class="form-text">
         <i class="mdi mdi-information-outline" aria-hidden="true"></i>
-        {% trans "Markdown syntax is supported: **bold**, `code`, lists, and links." %}
+        {% trans "Markdown syntax is supported." %}
       </div>
     </div>
     <button type="submit" class="btn btn-success">

netbox_change_control/tests/test_checks.py

@@ -754,3 +754,50 @@ def test_a_builtin_only_runs_where_a_policy_asks(self):
         ChangeRequestPolicy.objects.create(change_request=cr, policy=policy)
         run_checks(cr)
         self.assertTrue(cr.checks.filter(name='threads-resolved').exists())
+
+
+class ChecksRunWhenPoliciesAttachTest(TestCase):
+    """
+    A change request is created before its policies are matched, and every built-in check is
+    policy-scoped. The run at creation therefore sees no policies and no checks, so unless
+    attaching a policy runs them the panel sits at "pending" until somebody presses Re-run.
+    """
+
+    def setUp(self):
+        self._saved = dict(_registry)
+        _registry.clear()
+        from netbox_change_control.checks import register_builtin_checks
+
+        register_builtin_checks(['has-changes'])
+        self.requester = User.objects.create(username='attach-requester')
+        self.policy = Policy.objects.create(name='Wants has-changes', checks=['has-changes'])
+
+    def tearDown(self):
+        _registry.clear()
+        _registry.update(self._saved)
+
+    def test_attaching_a_policy_runs_its_checks(self):
+        cr = ChangeRequest.objects.create(
+            branch=make_branch('attach', self._testMethodName),
+            title='T',
+            requester=self.requester,
+        )
+        self.assertFalse(cr.checks.exists(), 'no policy yet, so no check applies')
+
+        ChangeRequestPolicy.objects.create(change_request=cr, policy=self.policy)
+
+        check = cr.checks.get(name='has-changes')
+        self.assertNotEqual(check.status, MergeCheckStatusChoices.PENDING)
+        self.assertIsNotNone(check.completed)
+
+    def test_detaching_the_policy_removes_its_checks(self):
+        cr = ChangeRequest.objects.create(
+            branch=make_branch('detach', self._testMethodName),
+            title='T',
+            requester=self.requester,
+        )
+        ChangeRequestPolicy.objects.create(change_request=cr, policy=self.policy)
+        self.assertTrue(cr.checks.filter(name='has-changes').exists())
+
+        ChangeRequestPolicy.objects.filter(change_request=cr).delete()
+        self.assertFalse(cr.checks.filter(name='has-changes').exists())

netbox_change_control/tests/test_policy.py

@@ -11,7 +11,7 @@
 from netbox_change_control.choices import ChangeRequestStatusChoices, ReviewDecisionChoices
 from netbox_change_control.models import ChangeRequest, ChangeRequestPolicy, Policy, Review
 from netbox_change_control.policy import evaluate_change_request
-from netbox_change_control.tests.base import add_rule, approve, make_branch, make_policy
+from netbox_change_control.tests.base import ChangeControlTestCase, add_rule, approve, make_branch, make_policy
 
 
 class PolicyRuleEligibilityTest(TestCase):
@@ -246,3 +246,39 @@ def test_it_does_not_weaken_another_policy(self):
         approve(cr, engineer)
         cr.refresh_from_db()
         self.assertEqual(cr.status, ChangeRequestStatusChoices.APPROVED)
+
+
+class ReasonsAudienceTest(ChangeControlTestCase):
+    """
+    The change request page lists every rule with its count, so repeating each shortfall in
+    prose underneath said the same thing twice. The merge gate has no such table, so it still
+    needs the full text.
+    """
+
+    branch_prefix = 'reasons'
+
+    def test_a_plain_shortfall_adds_nothing_to_the_page(self):
+        evaluation = self.cr.evaluate()
+        self.assertEqual(evaluation.other_reasons(), [])
+        self.assertTrue(any('needs 1 more' in r for r in evaluation.reasons()))
+
+    def test_a_rejection_is_shown_because_no_rule_row_carries_it(self):
+        Review.objects.create(
+            change_request=self.cr,
+            reviewer=self.reviewer,
+            decision=ReviewDecisionChoices.REJECT,
+            comment='not yet',
+        )
+        notes = self.cr.evaluate().other_reasons()
+        self.assertEqual(len(notes), 1)
+        self.assertIn('requested changes', notes[0])
+
+    def test_having_no_rules_at_all_is_shown(self):
+        ChangeRequestPolicy.objects.filter(change_request=self.cr).delete()
+        self.cr.refresh_from_db()
+        self.assertIn('No policy rules apply', ' '.join(self.cr.evaluate().other_reasons()))
+
+    def test_the_merge_gate_still_names_every_unmet_rule(self):
+        message = self.cr.merge_blocked_reason
+        self.assertIn('not approved', message)
+        self.assertTrue(any('needs' in r for r in self.cr.evaluate().reasons()))