Fix: prevent generic block attributes from overriding intentional req…

…uired rule (#7228) Co-authored-by: Jon Waldstein <[email protected]>
impress-org · Feb 13, 2024 · 5f86e66 · 5f86e66
1 parent ed7b7a4
commit 5f86e66
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 40 deletions.
diff --git a/give.php b/give.php
@@ -6,7 +6,7 @@
  * Description: The most robust, flexible, and intuitive way to accept donations on WordPress.
  * Author: GiveWP
  * Author URI: https://givewp.com/
- * Version: 3.4.0
+ * Version: 3.4.1
  * Requires at least: 6.0
  * Requires PHP: 7.2
  * Text Domain: give
@@ -403,7 +403,7 @@ private function setup_constants()
     {
         // Plugin version.
         if (!defined('GIVE_VERSION')) {
-            define('GIVE_VERSION', '3.4.0');
+            define('GIVE_VERSION', '3.4.1');
         }
 
         // Plugin Root File.

diff --git a/readme.txt b/readme.txt
@@ -5,7 +5,7 @@ Tags: donation, donate, recurring donations, fundraising, crowdfunding
 Requires at least: 6.0
 Tested up to: 6.4
 Requires PHP: 7.2
-Stable tag: 3.4.0
+Stable tag: 3.4.1
 License: GPLv3
 License URI: http://www.gnu.org/licenses/gpl-3.0.html
 
@@ -262,6 +262,9 @@ The 2% fee on Stripe donations only applies to donations taken via our free Stri
 10. Use almost any payment gateway integration with GiveWP through our add-ons or by creating your own add-on.
 
 == Changelog ==
+= 3.4.1: February 13th, 2024 =
+* Fix: Resolved an issue with the default email block that ensures it is always a required field in the donation form. 
+
 = 3.4.0: February 8th, 2024 =
 * Fix: Resolved several issues with the billing address block including dynamically requiring certain fields and allowing state/county field input
 * Fix: Resolved an issue with multi step form layout where the title was missing on the first step when show header was disabled

diff --git a/src/DonationForms/Actions/ConvertDonationFormBlocksToFieldsApi.php b/src/DonationForms/Actions/ConvertDonationFormBlocksToFieldsApi.php
@@ -21,6 +21,7 @@
 use Give\Framework\FieldsAPI\Exceptions\EmptyNameException;
 use Give\Framework\FieldsAPI\Exceptions\NameCollisionException;
 use Give\Framework\FieldsAPI\Exceptions\TypeNotSupported;
+use Give\Framework\FieldsAPI\Field;
 use Give\Framework\FieldsAPI\Name;
 use Give\Framework\FieldsAPI\Paragraph;
 use Give\Framework\FieldsAPI\PaymentGateways;
@@ -43,14 +44,14 @@ class ConvertDonationFormBlocksToFieldsApi
      */
     protected $currency;
     /**
-     * @var array {blockClientId: {node: Node, block: BlockModel}}
+     * @var array{blockClientId: {node: Node, block: BlockModel}}
      */
     protected $blockNodeRelationships = [];
 
     /**
      * @since 3.0.0
      *
-     * @return array {DonationForm, array {blockClientId: {node: Node, block: BlockModel}}}
+     * @return array{form: DonationForm, array{clientId: string{node: Node, block: BlockModel}}}
      * @throws TypeNotSupported|NameCollisionException
      */
     public function __invoke(BlockCollection $blocks, int $formId): array
@@ -101,7 +102,6 @@ protected function convertTopLevelBlockToSection(BlockModel $block, int $blockIn
     }
 
     /**
-     * @unlreased add `givewp_donation_form_block_converted_to_node` action hook
      * @since 3.0.0
      *
      * @return Node|null
@@ -112,15 +112,17 @@ protected function convertInnerBlockToNode(BlockModel $block, int $blockIndex)
     {
         $node = $this->createNodeFromBlockWithUniqueAttributes($block, $blockIndex);
 
-        if ($node instanceof Node) {
-            $node = $this->mapGenericBlockAttributesToNode($block, $node);
-
-            $this->mapBlockToNodeRelationships($block, $node);
+        if (!$node instanceof Node) {
+            return null;
+        }
 
-            return $node;
+        if ($node instanceof Field) {
+            $node = $this->mapGenericBlockAttributesToField($block, $node);
         }
 
-        return null;
+         $this->mapBlockToNodeRelationships($block, $node);
+
+        return $node;
     }
 
     /**
@@ -365,36 +367,35 @@ protected function createNodeFromConsentBlock(BlockModel $block, int $blockIndex
     }
 
     /**
+     * @since 3.4.1 updated to be field specific and prevent overwriting of existing values
      * @since 3.0.0
      */
-    protected function mapGenericBlockAttributesToNode(BlockModel $block, Node $node): Node
+    protected function mapGenericBlockAttributesToField(BlockModel $block, Field $field): Node
     {
-        if ('field' === $node->getNodeType()) {
-            // Label
-            if ($block->hasAttribute('label')) {
-                $node->label($block->getAttribute('label'));
-            }
+        // Label
+        if ($block->hasAttribute('label') && method_exists($field, 'label')) {
+            $field->label($block->getAttribute('label'));
+        }
 
-            // Placeholder
-            if ($block->hasAttribute('placeholder')) {
-                $node->placeholder($block->getAttribute('placeholder'));
-            }
+        // Placeholder
+        if ($block->hasAttribute('placeholder') && method_exists($field, 'placeholder')) {
+            $field->placeholder($block->getAttribute('placeholder'));
+        }
 
-            // Required
-            if ($block->hasAttribute('isRequired')) {
-                $node->required($block->getAttribute('isRequired'));
-            }
+        // Required
+        if ($block->hasAttribute('isRequired') && method_exists($field, 'required') && !$field->isRequired()) {
+            $field->required($block->getAttribute('isRequired'));
+        }
 
-            if ($block->hasAttribute('displayInAdmin') && $block->getAttribute('displayInAdmin')) {
-                $node->showInAdmin($block->getAttribute('displayInAdmin'));
-            }
+        if ($block->hasAttribute('displayInAdmin') && $block->getAttribute('displayInAdmin')) {
+            $field->showInAdmin($block->getAttribute('displayInAdmin'));
+        }
 
-            if ($block->hasAttribute('displayInReceipt') && $block->getAttribute('displayInReceipt')) {
-                $node->showInReceipt($block->getAttribute('displayInReceipt'));
-            }
+        if ($block->hasAttribute('displayInReceipt') && $block->getAttribute('displayInReceipt')) {
+            $field->showInReceipt($block->getAttribute('displayInReceipt'));
         }
 
-        return $node;
+        return $field;
     }
 
     /**

diff --git a/src/FormBuilder/resources/js/form-builder/src/blocks/fields/email/settings.tsx b/src/FormBuilder/resources/js/form-builder/src/blocks/fields/email/settings.tsx
@@ -4,7 +4,12 @@ import defaultSettings from '../settings';
 import FieldSettings from '@givewp/form-builder/blocks/fields/settings/Edit';
 import {FieldBlock} from '@givewp/form-builder/types';
 import {Path, SVG} from '@wordpress/components';
+import {BlockEditProps} from '@wordpress/blocks';
 
+/**
+ * @since 3.4.1 updated default required attribute to be true on block and edit component
+ * @since 3.0.0
+ */
 const settings: FieldBlock['settings'] = {
     ...defaultSettings,
     title: __('Email', 'give'),
@@ -17,6 +22,11 @@ const settings: FieldBlock['settings'] = {
         label: {
             default: __('Email Address'),
         },
+        isRequired: {
+            type: 'boolean',
+            source: 'attribute',
+            default: true,
+        },
     },
     icon: () => (
         <Icon
@@ -31,7 +41,7 @@ const settings: FieldBlock['settings'] = {
         />
     ),
 
-    edit: (props) => <FieldSettings showRequired={false} {...props} />,
+    edit: (props: BlockEditProps<any>) => <FieldSettings showRequired={false} {...props} attributes={{...props.attributes, isRequired: true}} />,
 };
 
 export default settings;
diff --git a/tests/Unit/DonationForms/Actions/TestConvertDonationFormBlocksToFieldsApi.php b/tests/Unit/DonationForms/Actions/TestConvertDonationFormBlocksToFieldsApi.php
@@ -13,17 +13,20 @@
 use Give\Tests\TestCase;
 use Give\Tests\TestTraits\RefreshDatabase;
 
+/**
+ * @since 3.0.0
+ * @covers \Give\DonationForms\Actions\ConvertDonationFormBlocksToFieldsApi
+ */
 final class TestConvertDonationFormBlocksToFieldsApi extends TestCase
 {
     use RefreshDatabase;
 
     /**
      * @since 3.0.0
      *
-     * @return void
      * @throws Exception
      */
-    public function testShouldReturnFormSchema()
+    public function testShouldReturnFormSchema(): void
     {
         $block = BlockModel::make([
             'clientId' => '8371d4c7-0e8d-4aff-a1a1-b4520f008132',
@@ -52,7 +55,7 @@ public function testShouldReturnFormSchema()
 
         $blocks = BlockCollection::make([$block]);
 
-        list($formSchema, $blockNodeRelationships) = (new ConvertDonationFormBlocksToFieldsApi())($blocks, $formId);
+        [$formSchema, $blockNodeRelationships] = (new ConvertDonationFormBlocksToFieldsApi())($blocks, $formId);
 
         $form = new DonationFormNode('donation-form');
         $form->defaultCurrency('USD');
@@ -74,10 +77,9 @@ public function testShouldReturnFormSchema()
     /**
      * @since 3.0.0
      *
-     * @return void
      * @throws Exception
      */
-    public function testShouldReturnFormSchemaUsingFilter()
+    public function testShouldReturnFormSchemaUsingFilter(): void
     {
         $section = BlockModel::make([
             'clientId' => '8371d4c7-0e8d-4aff-a1a1-b4520f008132',
@@ -118,7 +120,7 @@ static function ($node, BlockModel $block, int $blockIndex) {
             3
         );
 
-        list($formSchema, $blockNodeRelationships) = (new ConvertDonationFormBlocksToFieldsApi())($blocks, $formId);
+        [$formSchema, $blockNodeRelationships] = (new ConvertDonationFormBlocksToFieldsApi())($blocks, $formId);
 
         $form = new DonationFormNode('donation-form');
         $form->defaultCurrency('USD');
@@ -131,4 +133,50 @@ static function ($node, BlockModel $block, int $blockIndex) {
 
         $this->assertEquals($formSchema, $form);
     }
+
+    /**
+     * @unreleased
+     *
+     * @throws Exception
+     */
+    public function testMapGenericFieldAttributesShouldRespectExistingRules(): void
+    {
+        $section = BlockModel::make([
+            'clientId' => '8371d4c7-0e8d-4aff-a1a1-b4520f008132',
+            'name' => 'givewp/section',
+            'isValid' => true,
+            'attributes' => [
+                'title' => 'custom section title',
+                'description' => 'custom section description',
+            ],
+            'innerBlocks' => [
+                [
+                    'clientId' => 'bddaa0ea-29bf-4143-b62d-aae3396e9b0f',
+                    'name' => 'givewp/givewp-custom-block',
+                    'isValid' => true,
+                    'attributes' => [
+                        'label' => 'GiveWP Custom Block',
+                        'isRequired' => false,
+                    ],
+                ],
+            ],
+        ]);
+
+        $formId = 1;
+
+        $blocks = BlockCollection::make([$section]);
+
+        add_filter(
+            'givewp_donation_form_block_render_givewp/givewp-custom-block',
+            static function ($node, BlockModel $block, int $blockIndex) {
+                return Email::make('givewp-custom-block')->required();
+            },
+            10,
+            3
+        );
+
+        [$form] = (new ConvertDonationFormBlocksToFieldsApi())($blocks, $formId);
+
+        $this->assertTrue($form->getNodeByName('givewp-custom-block')->isRequired());
+    }
 }