Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#65 Fix issue where source for participant could be entered w… #12014

Merged
merged 1 commit into from
Apr 23, 2018

Conversation

seamuslee001
Copy link
Contributor

…hich is longer than allowable in the databse

Overview

Previously it was just a text field accepting any length of data. Now accepts up to what the db allows.

Before

DB error if data length > 128 char

After

No db length as field restricted to 128 char

ping @monishdeb @eileenmcnaughton would one of you be able to review this?

@monishdeb
Copy link
Member

Before:
screen shot 2018-04-22 at 4 13 55 pm

After:
screen shot 2018-04-22 at 4 13 13 pm

NOTE: The two new attributes maxlength and size is added.

@monishdeb
Copy link
Member

But @seamuslee001 the correct approach would be to use addField() to render form element which automatically sets the desired attributes implicitly. And here's the patch:

diff --git a/CRM/Event/DAO/Participant.php b/CRM/Event/DAO/Participant.php
index 0b7a3c2..a25c71b 100644
--- a/CRM/Event/DAO/Participant.php
+++ b/CRM/Event/DAO/Participant.php
@@ -6,7 +6,7 @@
  *
  * Generated from xml/schema/CRM/Event/Participant.xml
  * DO NOT EDIT.  Generated by CRM_Core_CodeGen
- * (GenCodeChecksum:444d0ee69773ce242341f8544e192087)
+ * (GenCodeChecksum:2fe76154a50c0317faace43c47c3456a)
  */
 
 /**
@@ -333,6 +333,9 @@ class CRM_Event_DAO_Participant extends CRM_Core_DAO {
           'entity' => 'Participant',
           'bao' => 'CRM_Event_BAO_Participant',
           'localizable' => 0,
+          'html' => [
+            'type' => 'Text',
+          ],
         ],
         'participant_fee_level' => [
           'name' => 'fee_level',
diff --git a/CRM/Event/Form/Participant.php b/CRM/Event/Form/Participant.php
index 342ed6c..d9f7bd2 100644
--- a/CRM/Event/Form/Participant.php
+++ b/CRM/Event/Form/Participant.php
@@ -202,6 +202,10 @@ class CRM_Event_Form_Participant extends CRM_Contribute_Form_AbstractEditPayment
     return 'Participant';
   }
 
+  public function getDefaultContext() {
+    return 'create';
+  }
+
   /**
    * Set variables up before form is built.
    *
@@ -733,7 +737,7 @@ class CRM_Event_Form_Participant extends CRM_Contribute_Form_AbstractEditPayment
 
     $this->addElement('checkbox', 'is_notify', ts('Send Notification'), NULL);
 
-    $this->add('text', 'source', ts('Event Source'));
+    $this->addField('source', array('entity' => 'Participant', 'name' => 'source'));
     $noteAttributes = CRM_Core_DAO::getAttribute('CRM_Core_DAO_Note');
     $this->add('textarea', 'note', ts('Notes'), $noteAttributes['note']);
 
diff --git a/xml/schema/Event/Participant.xml b/xml/schema/Event/Participant.xml
index 5d34101..30f9a31 100644
--- a/xml/schema/Event/Participant.xml
+++ b/xml/schema/Event/Participant.xml
@@ -135,6 +135,9 @@
     <type>varchar</type>
     <length>128</length>
     <comment>Source of this event registration.</comment>
+    <html>
+      <type>Text</type>
+    </html>
     <add>1.7</add>
   </field>
   <field>

As you can see the I have added the HTML metadata to this column and later fetched by API to rendered it is it's attribute.

@seamuslee001
Copy link
Contributor Author

hmm i guess so, i did a grep and noticed there were a number of other cases going with the fix i did but i guess yours is probably better from a lets build forms using metadata approach right @eileenmcnaughton ?

@eileenmcnaughton
Copy link
Contributor

@monishdeb @seamuslee001 yes - @monishdeb fix DOES look like the way to go

…hich is longer than allowable in the databse

Update files to use metadata approach as per monish and Eileen
@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton @monishdeb has been updated now

@monishdeb
Copy link
Member

Yes, it looks good now :) Merging it.

1 critical issue down. Thanks @seamuslee001 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants