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

CRM-19556 : Allow to search on just being an active member #9314

Merged
merged 3 commits into from
Oct 27, 2016
Merged

CRM-19556 : Allow to search on just being an active member #9314

merged 3 commits into from
Oct 27, 2016

Conversation

liedekef
Copy link
Contributor

@liedekef liedekef commented Oct 24, 2016

case 'membership_is_current_member':
// We don't want to include all tests for sql OR CRM-7827
$query->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause("civicrm_membership_status.is_current_member", $op, $value, "Boolean");
$query->_qill[$grouping][] = ts('Active Member');
Copy link
Contributor

Choose a reason for hiding this comment

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

only thing this should change if $value = 0 then it should be Inactive Member

Copy link
Member

@monishdeb monishdeb Oct 25, 2016

Choose a reason for hiding this comment

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

something like

$query->_qill[$grouping][] = ts('Is a Active Member? %1', array(1  => $value ? 'Yes' : 'No'));

??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check my latest commit and see if that is ok

@monishdeb
Copy link
Member

@liedekef there is a style warning

Query.php:264, SpaceBeforeDoubleArrow, Priority: High
Expected 1 space between "1" and double arrow; 2 found

highlighted in the related test build failure https://test.civicrm.org/job/CiviCRM-Core-PR/12408/

@monishdeb
Copy link
Member

Changes look good, tested on local

  • For Active membership
    image
  • For Inactive membership
    image

@liedekef
Copy link
Contributor Author

Ok, I'll fix the styling issues asap. Does the "merge on pass" mean that the PR will be immediately merged if all checks have passed?

@monishdeb
Copy link
Member

Yup exactly :) As when you fix the style error and push commit, test will re-build and if everything goes well it will be merged by me or other civicrm mergers.

@monishdeb monishdeb merged commit bc0afc0 into civicrm:master Oct 27, 2016
@agh1
Copy link
Contributor

agh1 commented Nov 28, 2016

I opened #9457 to address some wording issues.

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

Successfully merging this pull request may close these issues.

5 participants