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

Fix metadata on member export #14916

Merged
merged 1 commit into from
Aug 20, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jul 30, 2019

Overview

This fixes the lack of headings on some fields when exporting memberships

Before

No headings for

member_is_override
membership_recur_id
max_related

After

Headings for exported fields

Technical Details

I added uniquenames to match the export / query fields but am having misgivings about membership_recur_id - I wonder if I should instead change it to membership_contribution_recur_id - but there is more to change / more risk then

Comments

I did some testing to see if the renaming would cause issues & found

  1. there are api tests covering get & create on is_override
  2. it's not possible to add is_overide or contribution_recur id to a profile (ie the 2 I added uniquenames for)

Screen Shot 2019-08-03 at 2 39 06 PM

3) I had to change the tests for import for is_override but this doesn't require a db update as (somewhat tragically) the LABELS are stored in the mapping (I might look to change that as a follow up)

Screen Shot 2019-08-03 at 2 43 38 PM

  1. after the change I edited a membership through the back office form to be override - all good

  2. I DID find that I need to update smart groups based on search builder for this for is_override. recur id is already no exportable

@civibot
Copy link

civibot bot commented Jul 30, 2019

(Standard links)

@civibot civibot bot added the master label Jul 30, 2019
@seamuslee001
Copy link
Contributor

@eileenmcnaughton why not make auto_renew exportable?

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 it's not a real field - the WHERE code is pretty complex & I'm not even really sure what it means

@seamuslee001
Copy link
Contributor

@eileenmcnaughton i thought it was the field that had the referece to civicrm_contribution_recur in it? but maybe not

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 well it does refer to that field but not as just a number - see the where

      case 'member_auto_renew':
        $op = '=';
        $where = $qill = [];
        foreach ($value as $val) {
          if ($val == 1) {
            $where[] = ' civicrm_membership.contribution_recur_id IS NULL';
            $qill[] = ts('Membership is NOT Auto-Renew');
          }
          elseif ($val == 2) {
            $where[] = ' civicrm_membership.contribution_recur_id IS NOT NULL AND ' . CRM_Contact_BAO_Query::buildClause(
                'ccr.contribution_status_id',
                $op,
                array_search(
                  'In Progress',
                  CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name')
                ),
                'Integer'
              );
            $qill[] = ts('Membership is Auto-Renew and In Progress');
          }
          elseif ($val == 3) {
            $where[] = ' civicrm_membership.contribution_recur_id IS NOT NULL AND ' .
              CRM_Contact_BAO_Query::buildClause(
                'ccr.contribution_status_id',
                $op,
                array_search(
                  'Failed',
                  CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name')
                ),
                'Integer'
              );
            $qill[] = ts('Membership is Auto-Renew and Failed');
          }
          elseif ($val == 4) {
            $where[] = ' civicrm_membership.contribution_recur_id IS NOT NULL AND ' .
              CRM_Contact_BAO_Query::buildClause(
                'ccr.contribution_status_id',
                $op,
                array_search(
                  'Cancelled',
                  CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name')
                ),
                'Integer'
              );
            $qill[] = ts('Membership is Auto-Renew and Cancelled');
          }
          elseif ($val == 5) {
            $where[] = ' civicrm_membership.contribution_recur_id IS NOT NULL AND ccr.end_date IS NOT NULL AND ccr.end_date < NOW()';
            $qill[] = ts('Membership is Auto-Renew and Ended');
          }
          elseif ($val == 6) {
            $where[] = ' civicrm_membership.contribution_recur_id IS NOT NULL';
            $qill[] = ts('Membership is Auto-Renew');
          }
        }

@eileenmcnaughton
Copy link
Contributor Author

OTOH the select has NADA for it but it DOES offer up the civicrm_membership.contribution_recur_id field

@eileenmcnaughton
Copy link
Contributor Author

related test failure noise - that's a good thing! It tells me we have useful cover on this

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 I pulled autorenew out . to #14956

Note auto_renew seems un-exportable so I removed it from return properties
@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 I also finally got this passing - my goal is to ensure metadata is defined for all exportable fields & then we won't get new ones added with inadequate info / random handling because the tests will e-notice away like mad once I can remove the lenient lines - I think there are only really simple ones left after this

@jitendrapurohit
Copy link
Contributor

Tested the export screen, search forms, membership recur contribution, auto-renew functionality, etc and have not yet seen any problems in the functioning of the same.

I was able to replicate the main issue of not printing the headers in the export file and confirm that this pr fixes the display of 3 headers - member_is_override, membership_recur_id, max_related.

I feel this is good to be merged.

@eileenmcnaughton
Copy link
Contributor Author

thanks @jitendrapurohit

@eileenmcnaughton eileenmcnaughton merged commit f5570df into civicrm:master Aug 20, 2019
@eileenmcnaughton eileenmcnaughton deleted the export_mem branch August 20, 2019 12:47
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