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

Remove inheritance of MembershipConfig form from MembershipStatus form. #12184

Merged
merged 2 commits into from
May 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CRM/Admin/Form/RelationshipType.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class CRM_Admin_Form_RelationshipType extends CRM_Admin_Form {
* or is not a core field on the form's entity).
* - help (option) add help to the field - e.g ['id' => 'id-source', 'file' => 'CRM/Contact/Form/Contact']]
* - template - use a field specific template to render this field
* - required
* @var array
*/
protected $entityFields = [];
Expand All @@ -60,7 +61,8 @@ protected function setEntityFields() {
$this->entityFields = [
'label_a_b' => [
'name' => 'label_a_b',
'description' => ts("Label for the relationship from Contact A to Contact B. EXAMPLE: Contact A is 'Parent of' Contact B.")
'description' => ts("Label for the relationship from Contact A to Contact B. EXAMPLE: Contact A is 'Parent of' Contact B."),
'required' => TRUE,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accidentally changed to non-mandatory in previous commit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit should probably be in it's own PR as it's unrelated to the other changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RIght - it leverages the fact this PR adds the ability to set required. I'm OK to do that - I was just a bit scared that it could get lost - but if you feel the PR is otherwise mergeable I'll take it out

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, leave me to review/test the rest of the PR then first :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattwire Ok - I could pull that part out & do the required bit first & make this depend on that - either way but there is an interdependency

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed this change is correct. Let's leave it in the PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add 'required' to the docblock above too * - required (Is this field required or not?)

],
'label_b_a' => [
'name' => 'label_b_a',
Expand Down
2 changes: 1 addition & 1 deletion CRM/Core/Form/EntityFormTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ protected function setTranslatedFields() {
protected function addEntityFieldsToTemplate() {
foreach ($this->getEntityFields() as $fieldSpec) {
if (empty($fieldSpec['not-auto-addable'])) {
$this->addField($fieldSpec['name']);
$this->addField($fieldSpec['name'], [], CRM_Utils_Array::value('required', $fieldSpec));
}
}
}
Expand Down
19 changes: 17 additions & 2 deletions CRM/Member/DAO/MembershipStatus.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*
* Generated from xml/schema/CRM/Member/MembershipStatus.xml
* DO NOT EDIT. Generated by CRM_Core_CodeGen
* (GenCodeChecksum:dfe977e53a6b66703ab4eca3560048e6)
* (GenCodeChecksum:ea7435287a239f4851dd354150980069)
*/

/**
Expand Down Expand Up @@ -186,6 +186,9 @@ public static function &fields() {
'entity' => 'MembershipStatus',
'bao' => 'CRM_Member_BAO_MembershipStatus',
'localizable' => 1,
'html' => [
'type' => 'Text',
],
],
'start_event' => [
'name' => 'start_event',
Expand Down Expand Up @@ -292,12 +295,15 @@ public static function &fields() {
'is_admin' => [
'name' => 'is_admin',
'type' => CRM_Utils_Type::T_BOOLEAN,
'title' => ts('Admin Assigned Only?'),
'title' => ts('Administrator Only?'),
'description' => 'Is this status for admin/manual assignment only.',
'table_name' => 'civicrm_membership_status',
'entity' => 'MembershipStatus',
'bao' => 'CRM_Member_BAO_MembershipStatus',
'localizable' => 0,
'html' => [
'type' => 'CheckBox',
],
],
'weight' => [
'name' => 'weight',
Expand All @@ -317,6 +323,9 @@ public static function &fields() {
'entity' => 'MembershipStatus',
'bao' => 'CRM_Member_BAO_MembershipStatus',
'localizable' => 0,
'html' => [
'type' => 'CheckBox',
],
],
'is_active' => [
'name' => 'is_active',
Expand All @@ -328,6 +337,9 @@ public static function &fields() {
'entity' => 'MembershipStatus',
'bao' => 'CRM_Member_BAO_MembershipStatus',
'localizable' => 0,
'html' => [
'type' => 'CheckBox',
],
],
'is_reserved' => [
'name' => 'is_reserved',
Expand All @@ -338,6 +350,9 @@ public static function &fields() {
'entity' => 'MembershipStatus',
'bao' => 'CRM_Member_BAO_MembershipStatus',
'localizable' => 0,
'html' => [
'type' => 'CheckBox',
],
],
];
CRM_Core_DAO_AllCoreTables::invoke(__CLASS__, 'fields_callback', Civi::$statics[__CLASS__]['fields']);
Expand Down
5 changes: 0 additions & 5 deletions CRM/Member/Form/MembershipConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@ class CRM_Member_Form_MembershipConfig extends CRM_Core_Form {
*/
protected $_BAOName;

public function preProcess() {
$this->_id = $this->get('id');
$this->_BAOName = $this->get('BAOName');
}

/**
* Set default values for the form. MobileProvider that in edit/view mode
* the default values are retrieved from the database
Expand Down
82 changes: 64 additions & 18 deletions CRM/Member/Form/MembershipStatus.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@
/**
* This class generates form components for Membership Type
*/
class CRM_Member_Form_MembershipStatus extends CRM_Member_Form_MembershipConfig {
class CRM_Member_Form_MembershipStatus extends CRM_Core_Form {

use CRM_Core_Form_EntityFormTrait;

/**
* Explicitly declare the entity api name.
Expand All @@ -51,15 +52,71 @@ public function getDefaultContext() {
return 'create';
}

/**
* Fields for the entity to be assigned to the template.
*
* Fields may have keys
* - name (required to show in tpl from the array)
* - description (optional, will appear below the field)
* - not-auto-addable - this class will not attempt to add the field using addField.
* (this will be automatically set if the field does not have html in it's metadata
* or is not a core field on the form's entity).
* - help (optional) add help to the field - e.g ['id' => 'id-source', 'file' => 'CRM/Contact/Form/Contact']]
* - template - use a field specific template to render this field
* - required
* @var array
*/
protected $entityFields = [];

/**
* Set entity fields to be assigned to the form.
*/
protected function setEntityFields() {
$this->entityFields = [
'label' => [
'name' => 'label',
'description' => ts("Display name for this Membership status (e.g. New, Current, Grace, Expired...)."),
'required' => TRUE,
],
'is_admin' => [
'name' => 'is_admin',
'description' => ts("Check this box if this status is for use by administrative staff only. If checked, this status is never automatically assigned by CiviMember. It is assigned to a contact's Membership by checking the <strong>Status Override</strong> flag when adding or editing the Membership record. Start and End Event settings are ignored for Administrator statuses. EXAMPLE: This setting can be useful for special case statuses like 'Non-expiring', 'Barred' or 'Expelled', etc."),
],
];
}

/**
* Set the delete message.
*
* We do this from the constructor in order to do a translation.
*/
public function setDeleteMessage() {
$this->deleteMessage = ts('You will not be able to delete this membership status if there are existing memberships with this status. You will need to check all your membership status rules afterwards to ensure that a valid status will always be available.') . " " . ts('Do you want to continue?');
}

public function preProcess() {
$this->_id = $this->get('id');
$this->_BAOName = 'CRM_Member_BAO_MembershipStatus';
}

/**
* Set default values for the form. MobileProvider that in edit/view mode
* the default values are retrieved from the database
*
*
* @return void
* @return array
*/
public function setDefaultValues() {
$defaults = parent::setDefaultValues();
$defaults = array();

if ($this->getEntityId()) {
$params = array('id' => $this->getEntityId());
$baoName = $this->_BAOName;
$baoName::retrieve($params, $defaults);
}

if ($this->_action & CRM_Core_Action::ADD) {
$defaults['is_active'] = 1;
}

//finding default weight to be put
if (empty($defaults['weight'])) {
Expand All @@ -70,28 +127,22 @@ public function setDefaultValues() {

/**
* Build the form object.
*
* @return void
*/
public function buildQuickForm() {
self::buildQuickEntityForm();
parent::buildQuickForm();

if ($this->_action & CRM_Core_Action::DELETE) {
return;
}

$this->applyFilter('__ALL__', 'trim');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in self::buildQuickEntityForm();


if ($this->_id) {
$name = $this->add('text', 'name', ts('Name'),
CRM_Core_DAO::getAttribute('CRM_Member_DAO_MembershipStatus', 'name')
);
$name->freeze();
$this->assign('id', $this->_id);
}
$this->add('text', 'label', ts('Label'),
CRM_Core_DAO::getAttribute('CRM_Member_DAO_MembershipStatus', 'label'), TRUE
);
$this->addRule('label', ts('A membership status with this label already exists. Please select another label.'),
'objectExists', array('CRM_Member_DAO_MembershipStatus', $this->_id, 'name')
);
Expand All @@ -107,7 +158,6 @@ public function buildQuickForm() {
CRM_Core_DAO::getAttribute('CRM_Member_DAO_MembershipStatus', 'end_event_adjust_interval')
);
$this->add('checkbox', 'is_current_member', ts('Current Membership?'));
$this->add('checkbox', 'is_admin', ts('Administrator Only?'));

$this->add('text', 'weight', ts('Order'),
CRM_Core_DAO::getAttribute('CRM_Member_DAO_MembershipStatus', 'weight')
Expand All @@ -118,9 +168,6 @@ public function buildQuickForm() {

/**
* Process the form submission.
*
*
* @return void
*/
public function postProcess() {
if ($this->_action & CRM_Core_Action::DELETE) {
Expand All @@ -133,7 +180,6 @@ public function postProcess() {
CRM_Core_Session::setStatus(ts('Selected membership status has been deleted.'), ts('Record Deleted'), 'success');
}
else {
$params = $ids = array();
// store the submitted values in an array
$params = $this->exportValues();
$params['is_active'] = CRM_Utils_Array::value('is_active', $params, FALSE);
Expand All @@ -142,7 +188,7 @@ public function postProcess() {
$params['is_default'] = CRM_Utils_Array::value('is_default', $params, FALSE);

if ($this->_action & CRM_Core_Action::UPDATE) {
$ids['membershipStatus'] = $this->_id;
$params['id'] = $this->getEntityId();
}
$oldWeight = NULL;
if ($this->_id) {
Expand All @@ -155,7 +201,7 @@ public function postProcess() {
$params['name'] = $params['label'];
}

$membershipStatus = CRM_Member_BAO_MembershipStatus::add($params, $ids);
$membershipStatus = CRM_Member_BAO_MembershipStatus::add($params);
CRM_Core_Session::setStatus(ts('The membership status \'%1\' has been saved.',
array(1 => $membershipStatus->label)
), ts('Saved'), 'success');
Expand Down
12 changes: 6 additions & 6 deletions templates/CRM/Member/Form/MembershipStatus.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@
<div class="crm-block crm-form-block crm-membership-status-form-block" id=membership_status>
<fieldset><legend>{if $action eq 1}{ts}New Membership Status{/ts}{elseif $action eq 2}{ts}Edit Membership Status{/ts}{else}{ts}Delete Membership Status{/ts}{/if}</legend>
<div class="crm-submit-buttons">{include file="CRM/common/formButtons.tpl" location="top"}</div>
{if $action eq 8}
<div class="messages status no-popup">
<div class="icon inform-icon"></div>
{ts}WARNING: Deleting this option will result in the loss of all membership records of this status.{/ts} {ts}This may mean the loss of a substantial amount of data, and the action cannot be undone.{/ts} {ts}Do you want to continue?{/ts}
</div>
{else}
{if $action eq 8}
<div class="messages status no-popup">
<div class="icon inform-icon"></div>
{$deleteMessage|escape}
</div>
{else}
<table class="form-layout-compressed">
{if $action eq 2}
<tr class="crm-membership-status-form-block-name">
Expand Down
18 changes: 17 additions & 1 deletion xml/schema/Member/MembershipStatus.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,13 @@
</field>
<field>
<name>label</name>
<title>Label</title>
<type>varchar</type>
<length>128</length>
<localizable>true</localizable>
<html>
<type>Text</type>
</html>
<comment>Label for Membership Status</comment>
<add>3.2</add>
</field>
Expand Down Expand Up @@ -110,9 +114,12 @@
</field>
<field>
<name>is_admin</name>
<title>Admin Assigned Only?</title>
<title>Administrator Only?</title>
<type>boolean</type>
<comment>Is this status for admin/manual assignment only.</comment>
<html>
<type>CheckBox</type>
</html>
<add>1.5</add>
</field>
<field>
Expand All @@ -125,6 +132,9 @@
<name>is_default</name>
<title>Default Status?</title>
<type>boolean</type>
<html>
<type>CheckBox</type>
</html>
<comment>Assign this status to a membership record if no other status match is found.</comment>
<add>1.5</add>
</field>
Expand All @@ -133,6 +143,9 @@
<title>Is Active</title>
<type>boolean</type>
<default>1</default>
<html>
<type>CheckBox</type>
</html>
<comment>Is this membership_status enabled.</comment>
<add>1.5</add>
</field>
Expand All @@ -141,6 +154,9 @@
<title>Is Reserved</title>
<type>boolean</type>
<default>0</default>
<html>
<type>CheckBox</type>
</html>
<comment>Is this membership_status reserved.</comment>
<add>2.1</add>
</field>
Expand Down