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

Convert membership date fields to datepicker & form to entity form #12690

Merged
merged 1 commit into from
Sep 17, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Aug 20, 2018

Overview

As part of our ongoing effort to convert all date fields to datepicker field & a newer interest in cleaning up the membership form this converts 3 date fields on the Membership form. It uses the new & preferred entityFormTrait to add the fields

Before

Old style date fields

After

New style date fields

Technical Details

Removing jcalendar has been a pretty long-running initiative - although we still have 68 instances even after this :-( The new datepicker format requires less date wrangling & hence is more robust & allows simplification. Ditto the EntityFormTrait allows us to move to a more metadata oriented approach

Comments

@mattwire @seamuslee001 @colemanw

@civibot
Copy link

civibot bot commented Aug 20, 2018

(Standard links)

@@ -202,12 +202,23 @@ protected function setTranslatedFields() {
protected function addEntityFieldsToTemplate() {
foreach ($this->getEntityFields() as $fieldSpec) {
if (empty($fieldSpec['not-auto-addable'])) {
$element = $this->addField($fieldSpec['name'], [], CRM_Utils_Array::value('required', $fieldSpec));
$element = $this->addField($fieldSpec['name'], [], CRM_Utils_Array::value('required', $fieldSpec), FALSE);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The FALSE here denotes 'dont' use legacy date field type' - we don't have date fields in other entityTrait forms hence this wasn't set earlier

*/
public function getEntityId() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

trait has this function

@@ -562,19 +599,10 @@ public function buildQuickForm() {
$sel->freeze();
}

$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.

buildQuickEntityForm does this

*
* @return bool
*/
protected function isDeleteContext() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't turn out to need this extracted fn - but still - it's nice & clear :-)

@@ -182,6 +222,9 @@ public function preProcess() {
// This string makes up part of the class names, differentiating them (not sure why) from the membership fields.
$this->assign('formClass', 'membership');
parent::preProcess();
if ($this->isUpdateToExistingRecurringMembership()) {
$this->entityFields['end_date']['is_freeze'] = 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.

We can't do this freeze until preProcess as id is not retrieved from the form earlier. I did go down the path of trying to move the call to setEntityFields to the preProcess - but perhaps it makes sense to keep it generic & tweak it in preProcess - as done here.

There are other fields that will ideally be moved into this & into the setEntityFields

$dates = array('join_date', 'start_date', 'end_date');
foreach ($dates as $key) {
if (!empty($defaults[$key])) {
list($defaults[$key]) = CRM_Utils_Date::setDateDefaults(CRM_Utils_Array::value($key, $defaults));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just yay

@@ -1934,9 +1962,9 @@ protected function setStatusMessage($membership, $endDate, $receiptSend, $member
protected function isUpdateToExistingRecurringMembership() {
$isRecur = FALSE;
if ($this->_action & CRM_Core_Action::UPDATE
&& CRM_Core_DAO::getFieldValue('CRM_Member_DAO_Membership', $this->_id,
&& CRM_Core_DAO::getFieldValue('CRM_Member_DAO_Membership', $this->getEntityId(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just for form

@colemanw
Copy link
Member

@civicrm-builder retest this please

@eileenmcnaughton
Copy link
Contributor Author

@colemanw it passed now

@mattwire
Copy link
Contributor

mattwire commented Sep 8, 2018

Ok I've given this a quick test and am happy with it. @colemanw If you are happy please merge.

@colemanw colemanw merged commit ae6f0eb into civicrm:master Sep 17, 2018
@colemanw colemanw deleted the member_dates branch September 17, 2018 22:23
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.

4 participants