fix(spp_base_common): keep canCreate on the form New button (#582)

e2e/tests/01-spp-starter-spmis.spec.ts

@@ -16,13 +16,15 @@
 //   13 - Creates an Approval Definition (Change request → HQ validator) and assigns it as the Approval Workflow for Edit Individual Information, Edit Group Information, and Update ID Document change request types
 //   14 - Manually defines 5 Change Request Document Type vocabulary codes for the Philippines (PSA Birth Certificate, PhilSys National ID, Barangay Certificate of Residency, PSA Marriage Certificate, Proof of Income), matching what spp_mis_demo_v2's demo generator seeds for "phl", by adding rows to the existing vocabulary's Codes tab via Settings > Vocabularies
 //   15 - Manually defines 5 more Change Request Document Type vocabulary codes for the Philippines (BIR Form 2316, Academic Calendar, Authorization Letter, Certificate of Enrolment, Valid ID of Parent), matching the e2e fixture PDFs in e2e/fixtures/, by adding rows to the same vocabulary's Codes tab as test 14
-//   16 - Creates an "Edit Individual Information" change request for Santos, Jose Miguel (as admin), uploads a supporting document, and submits it for approval
+//   16 - Creates an "Edit Individual Information" change request for Santos, Jose Miguel (as admin), checks that the detail form the wizard opens with create disabled in its context renders no "New" button, uploads a supporting document, and submits it for approval
 //   17 - Creates an "Edit Group Information" change request for Santos Family (as admin), uploads a supporting document, and submits it for approval
 //   18 - Creates an "Update ID Document" change request for Santos, Jose Miguel (as admin), selecting the registrant by keyboard only and checking that the live region does not receive a late range announcement after the results are gone, sets a National ID with tomorrow's expiry date, uploads a supporting document, and submits it for approval
 //   19 - Creates an HQ validator user (hqval@mail.com) with the "CR HQ Validator" role and sets its password
 //   20 - Logs in as the HQ validator and resolves all three pending change requests: approves
 //        Edit Individual Information, rejects Edit Group Information (with a reason), and
-//        requests revision on Update ID Document (with revision notes)
+//        requests revision on Update ID Document (with revision notes). First checks that an
+//        approval review form (create="0" on the arch, no create access for the role) renders
+//        no "New" button at all
 //   21 - Logs back in as admin and confirms the three resolutions actually took effect: the
 //        approved name change is reflected on the registrant, and the Change Requests list
 //        shows Rejected / Needs Changes for the other two
@@ -1082,6 +1084,15 @@ test.describe.serial("OpenSPP Starter SP-MIS", () => {
     });
     console.log("✅ Change request draft opened for editing");
 
+    // The wizard opens this detail form with `create: False` in the action
+    // context (nothing on the arch or in a FormController patch hides the button
+    // here), so Odoo's canCreate is false and the form's "New" button must not be
+    // rendered at all. Assert on the DOM count, not on visibility: spp_programs'
+    // FormController patch hides the same button with `display: none`, which
+    // would mask a template that renders it anyway (#582).
+    await expect(page.locator(".o_form_button_create")).toHaveCount(0);
+    console.log("✅ No New button on the change request detail form");
+
     await page.getByRole("textbox", {name: "Given Name"}).fill("Jose Miguel Updated");
     await page.getByRole("textbox", {name: "Address Line 1"}).fill("updated");
     await page.getByRole("textbox", {name: "Address Line 2"}).fill("updated");
@@ -1351,6 +1362,22 @@ test.describe.serial("OpenSPP Starter SP-MIS", () => {
     await expect(page.getByRole("row", {name: `${crNumber} Update ID`})).toBeVisible();
     console.log("✅ Logged in as HQ validator (hqval@mail.com)");
 
+    // The approval review form declares create="0" on its arch, and the HQ
+    // validator role has no create ACL on reviews either, so Odoo's canCreate is
+    // false and the form's "New" button must not be rendered at all. Assert on
+    // the DOM count, not on visibility: spp_programs' FormController patch hides
+    // the same button with `display: none`, which would mask a template that
+    // renders it anyway (#582).
+    await page.getByRole("list").getByRole("menuitem", {name: "Approvals"}).click();
+    await page.getByRole("button", {name: "My Approvals"}).click();
+    await page.getByRole("menuitem", {name: "Pending Approvals"}).click();
+    await expect(page.locator(".o_data_row").first()).toBeVisible();
+    await page.locator(".o_data_row").first().click();
+    await expect(page.locator(".o_form_view")).toBeVisible();
+    await expect(page.locator(".o_form_button_create")).toHaveCount(0);
+    console.log("✅ No New button on the approval review form");
+
+    await page.getByRole("menuitem", {name: "Change Requests"}).click();
     await page.getByRole("menuitem", {name: "All Requests"}).click();
     await page.getByRole("cell", {name: "Edit Individual Information"}).click();
     await expect(

spp_base_common/README.rst

@@ -120,6 +120,24 @@ Dependencies
 Changelog
 =========
 
+19.0.2.0.3
+~~~~~~~~~~
+
+- fix(ui): the form view's **New** button respects Odoo's create guard
+  again. The ``web.FormView`` extension that lets a module hide the
+  button with ``hideFormCreateButton`` replaced the button's condition
+  instead of extending it, dropping Odoo's ``canCreate``. As a result
+  every form opened with create disabled — ``{'create': False}`` in the
+  action context, ``create="0"`` on the form arch, or a role without
+  create access — still rendered a working **New** button that opened a
+  blank editable record, or failed with an access error on save (visible
+  on installs without ``spp_programs``; with it, a workaround hid the
+  rendered button after the fact). The condition is now
+  ``canCreate and !hideFormCreateButton``: the flag can only hide the
+  button, never show it, and modules using it keep working unchanged.
+  Forms shown in dialogs use a separate template and were never affected
+  (#582).
+
 19.0.2.0.2
 ~~~~~~~~~~
 

spp_base_common/__manifest__.py

@@ -5,7 +5,7 @@
 {
     "name": "OpenSPP Base (Common)",
     "category": "OpenSPP/Core",
-    "version": "19.0.2.0.2",
+    "version": "19.0.2.0.3",
     "sequence": 1,
     "author": "OpenSPP.org",
     "website": "https://github.com/OpenSPP/OpenSPP2",

spp_base_common/readme/HISTORY.md

@@ -1,3 +1,7 @@
+### 19.0.2.0.3
+
+- fix(ui): the form view's **New** button respects Odoo's create guard again. The `web.FormView` extension that lets a module hide the button with `hideFormCreateButton` replaced the button's condition instead of extending it, dropping Odoo's `canCreate`. As a result every form opened with create disabled — `{'create': False}` in the action context, `create="0"` on the form arch, or a role without create access — still rendered a working **New** button that opened a blank editable record, or failed with an access error on save (visible on installs without `spp_programs`; with it, a workaround hid the rendered button after the fact). The condition is now `canCreate and !hideFormCreateButton`: the flag can only hide the button, never show it, and modules using it keep working unchanged. Forms shown in dialogs use a separate template and were never affected (#582).
+
 ### 19.0.2.0.2
 
 - test: add a regression test guarding the PDF backend selected by `odoo.tools.pdf`. The Docker image accidentally shipped legacy PyPDF2 3.x next to pypdf; Odoo prefers PyPDF2 when importable, and its removed 1.x API (`numPages`/`getPage`) crashes multi-record PDF printing with a `DeprecationError` (OP#1168). The fix is in `docker/Dockerfile` (`--no-deps` on the Odoo editable install); this test fails on any image that regresses.

spp_base_common/static/description/index.html

@@ -495,6 +495,25 @@ <h2><a class="toc-backref" href="#toc-entry-1">Changelog</a></h2>
 </div>
 </div>
 <div class="section" id="section-1">
+<h1>19.0.2.0.3</h1>
+<ul class="simple">
+<li>fix(ui): the form view’s <strong>New</strong> button respects Odoo’s create guard
+again. The <tt class="docutils literal">web.FormView</tt> extension that lets a module hide the
+button with <tt class="docutils literal">hideFormCreateButton</tt> replaced the button’s condition
+instead of extending it, dropping Odoo’s <tt class="docutils literal">canCreate</tt>. As a result
+every form opened with create disabled — <tt class="docutils literal">{'create': False}</tt> in the
+action context, <tt class="docutils literal"><span class="pre">create=&quot;0&quot;</span></tt> on the form arch, or a role without
+create access — still rendered a working <strong>New</strong> button that opened a
+blank editable record, or failed with an access error on save (visible
+on installs without <tt class="docutils literal">spp_programs</tt>; with it, a workaround hid the
+rendered button after the fact). The condition is now
+<tt class="docutils literal">canCreate and !hideFormCreateButton</tt>: the flag can only hide the
+button, never show it, and modules using it keep working unchanged.
+Forms shown in dialogs use a separate template and were never affected
+(#582).</li>
+</ul>
+</div>
+<div class="section" id="section-2">
 <h1>19.0.2.0.2</h1>
 <ul class="simple">
 <li>test: add a regression test guarding the PDF backend selected by
@@ -506,7 +525,7 @@ <h1>19.0.2.0.2</h1>
 this test fails on any image that regresses.</li>
 </ul>
 </div>
-<div class="section" id="section-2">
+<div class="section" id="section-3">
 <h1>19.0.2.0.1</h1>
 <ul class="simple">
 <li>fix(security): add <tt class="docutils literal"><span class="pre">groups=&quot;base.group_system&quot;</span></tt> to the existing
@@ -522,7 +541,7 @@ <h1>19.0.2.0.1</h1>
 <tt class="docutils literal">group_ids</tt> to empty.</li>
 </ul>
 </div>
-<div class="section" id="section-3">
+<div class="section" id="section-4">
 <h1>19.0.2.0.0</h1>
 <ul class="simple">
 <li>Initial migration to OpenSPP2</li>

spp_base_common/static/src/xml/custom_list_create_template.xml

@@ -42,10 +42,14 @@
 
     <!-- Generic extensible form create button hide.
          Any module can set this.hideFormCreateButton = true in their
-         FormController patch to hide the default "New" button. -->
+         FormController patch to hide the default "New" button.
+         The flag can only hide the button, never show it: Odoo's own
+         canCreate guard stays in the condition, so a form opened with
+         create disabled (action context {'create': False}, create="0" on
+         the arch, no create ACL) keeps hiding "New". -->
     <t t-inherit="web.FormView" t-inherit-mode="extension">
         <xpath expr="//button[hasclass('o_form_button_create')]" position="attributes">
-            <attribute name="t-if">!hideFormCreateButton</attribute>
+            <attribute name="t-if">canCreate and !hideFormCreateButton</attribute>
         </xpath>
     </t>
 

spp_base_common/tests/__init__.py

@@ -1,3 +1,4 @@
+from . import test_form_create_button_template
 from . import test_ir_module_module
 from . import test_pdf_backend
 from . import test_phone_number_validation

spp_base_common/tests/test_form_create_button_template.py

@@ -0,0 +1,89 @@
+"""Guard the form-view "New" button extension in custom_list_create_template.xml.
+
+spp_base_common extends the ``web.FormView`` template so that any module can set
+``this.hideFormCreateButton = true`` on its FormController patch to hide the
+control-panel "New" button. Stock Odoo renders that button with
+``t-if="canCreate"``, and ``canCreate`` is the single switch behind every way a
+form can be opened without create: ``{'create': False}`` in the action context,
+``create="0"`` on the form arch, and a missing create ACL. Replacing the
+condition instead of extending it rendered a live "New" button in all three
+cases (#582). The fourth input to ``canCreate``, the ``preventCreate`` prop, is
+only passed by form dialogs, which do not render the control panel at all.
+
+The web client applies template inheritance in the browser, which the test image
+cannot run. Odoo ships the same xpath/position engine server-side in
+``odoo.tools.template_inheritance``, so this test applies the extension to the
+real stock template and checks the resulting condition.
+"""
+
+import copy
+
+from lxml import etree
+
+from odoo.modules.module import get_manifest
+from odoo.tests import TransactionCase
+from odoo.tools.misc import file_open
+from odoo.tools.template_inheritance import apply_inheritance_specs
+
+STOCK_FORM_TEMPLATES = "web/static/src/views/form/form_controller.xml"
+EXTENSION_TEMPLATES = "spp_base_common/static/src/xml/custom_list_create_template.xml"
+# Relative to the template element: an absolute "//" xpath on an lxml element
+# searches the whole document, i.e. every template in the file.
+CREATE_BUTTON_XPATH = ".//button[hasclass('o_form_button_create')]"
+
+
+def _load_templates(path):
+    with file_open(path, "rb") as template_file:
+        return etree.fromstring(template_file.read())
+
+
+class TestFormCreateButtonTemplate(TransactionCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.stock = _load_templates(STOCK_FORM_TEMPLATES)
+        cls.extension = _load_templates(EXTENSION_TEMPLATES)
+
+    def _apply_extension(self, template_name):
+        """Return the stock template after every spec that inherits it is applied.
+
+        The template is detached from its file first so that the spec's own
+        "//" xpath is scoped to this one template, as the web client scopes it.
+        """
+        (source,) = self.stock.xpath(f"./t[@t-name='{template_name}']")
+        source = copy.deepcopy(source)
+        specs = [
+            spec
+            for inherit in self.extension.xpath(f"./t[@t-inherit='{template_name}']")
+            for spec in inherit
+            if isinstance(spec.tag, str)
+        ]
+        return apply_inheritance_specs(source, specs)
+
+    def test_control_panel_new_button_keeps_can_create(self):
+        form_view = self._apply_extension("web.FormView")
+        (button,) = form_view.xpath(CREATE_BUTTON_XPATH)
+        condition = button.get("t-if")
+
+        # Exact match: the condition is a one-line expression under our control,
+        # and a token check would accept "canCreate or !hideFormCreateButton",
+        # which reintroduces the bug.
+        self.assertEqual(
+            condition,
+            "canCreate and !hideFormCreateButton",
+            "the form New button must keep Odoo's canCreate guard "
+            '(action context, create="0", ACL, preventCreate) and the '
+            "hideFormCreateButton opt-out must still hide the button",
+        )
+
+    def test_extension_is_shipped_in_the_backend_bundle(self):
+        """The template only takes effect if the manifest still lists it in web.assets_backend."""
+        manifest = get_manifest("spp_base_common")
+        self.assertIn(EXTENSION_TEMPLATES, manifest["assets"]["web.assets_backend"])
+
+    def test_dialog_new_button_is_not_touched(self):
+        """The extension targets web.FormView only; dialog footers keep stock markup."""
+        self.assertFalse(self.extension.xpath("./t[@t-inherit='web.FormView.Buttons']"))
+        (source,) = self.stock.xpath("./t[@t-name='web.FormView.Buttons']")
+        (button,) = source.xpath(CREATE_BUTTON_XPATH)
+        self.assertRegex(button.get("t-if"), r"\bcanCreate\b")