Skip to content

Commit

Permalink
Rebuild recipient list and calculate count on demand, store result in
Browse files Browse the repository at this point in the history
  • Loading branch information
monishdeb committed Dec 4, 2017
1 parent eeda3fc commit 7a646a0
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 46 deletions.
10 changes: 10 additions & 0 deletions ang/crmMailing.css
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,13 @@ input[name=preview_test_email]:-ms-input-placeholder {
margin: 0.5em;
color: red;
}

.crmMailing-outdated-count {
color: red;
font-weight: bold;
}

.crmMailing-latest-count {
color: green;
font-weight: bold;
}
17 changes: 10 additions & 7 deletions ang/crmMailing/BlockRecipients.html
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
<div ng-controller="EditRecipCtrl" class="crm-mailing-recipients-row">
<div style="float: right;">
<div class="crmMailing-recip-est">
<a href="" ng-click="previewRecipients()" title="{{ts('Preview a List of Recipients')}}">{{getRecipientsEstimate()}}</a>
</div>
</div>
<input
type="hidden"
crm-mailing-recipients
ng-model="mailing.recipients"
crm-mandatory-groups="crmMailingConst.groupNames | filter:{is_hidden:1}"
crm-ui-id="{{crmMailingBlockRecipients.id}}"
name="{{crmMailingBlockRecipients.name}}"
ng-required="true"/>
<a crm-icon="fa-wrench" ng-click="editOptions(mailing)" class="crm-hover-button" title="{{ts('Edit Recipient Options')}}"></a>
ng-required="true" />
<a crm-icon="fa-wrench" ng-click="editOptions(mailing)" class="crm-hover-button" title="{{ts('Edit Recipient Options')}}"></a>
<div>

This comment has been minimized.

Copy link
@mukeshcompucorp

mukeshcompucorp Dec 26, 2017

Contributor

@monishdeb can you add the class="crmMailing-recip-est" in here which you removed from line 3 (7a646a0#diff-998154734e54e44344b0ac767fc62e70L3)

It's essential that we keep such classes as it is while modifying as it can broke other extensions or scripts targeting the code

This comment has been minimized.

Copy link
@monishdeb

monishdeb Dec 26, 2017

Author Member

@mukeshcompucorp this was done intentionally to remove the yellow background and other style adjustments there were earlier made through crmMailing-recip-est class. As per the latest change, this style class was replaced by dynamic class appended by ng-style here .

Retaining the crmMailing-recip-est class
screen shot 2017-12-26 at 5 50 35 pm
will bring the 'Recipients' count text below the recipient's field, highlight the background in yellow, which will revert one of the style fix that was discussed among CT and other reviewers. Please check the discussion in PR #11091 .

This comment has been minimized.

Copy link
@mukeshcompucorp

mukeshcompucorp Dec 26, 2017

Contributor

@monishdeb in this case it's advised that we style it under a class in css instead of inline style.
so, you can use ng-class instead of that as logic and as for the style of background yellow, can that be changed in css instead of removing class?

This comment has been minimized.

Copy link
@monishdeb

monishdeb Dec 26, 2017

Author Member

I would suggest, bringing the changes in extension instead, by replacing the usage of crmMailing-recip-est class with inline-block as because, for core, crmMailing-recip-est class has no other significance and isn't used anywhere in core. As a general approach, we always tend to maintain the ext code as per latest changes made in core (because we cannot be certain about codebase instance and its dependency on extension(s)) until and unless the core changes leads to any regression or unintended behaivour which isn't in this case. What you think?

<button ng-click="rebuildRecipients()" class="ng-binding crm-button" title="{{ts('Preview a List of Recipients')}}">{{getRecipientsEstimate()}}</button>
<a ng-click="previewRecipients()" class="crm-hover-button" title="{{ts('Click to refresh recipient count')}}">
<span class="{{((outdated || recipient == 0) ? 'crmMailing-outdated-count' : 'crmMailing-latest-count')}}">
{{getRecipientCount()}}
</span>
</a>
</div>
</div>
66 changes: 44 additions & 22 deletions ang/crmMailing/EditRecipCtrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// Scope members:
// - [input] mailing: object
// - [output] recipients: array of recipient records
angular.module('crmMailing').controller('EditRecipCtrl', function EditRecipCtrl($scope, dialogService, crmApi, crmMailingMgr, $q, crmMetadata, crmStatus) {
angular.module('crmMailing').controller('EditRecipCtrl', function EditRecipCtrl($scope, dialogService, crmApi, crmMailingMgr, $q, crmMetadata, crmStatus, crmMailingCache) {
// Time to wait before triggering AJAX update to recipients list
var RECIPIENTS_DEBOUNCE_MS = 100;
var RECIPIENTS_PREVIEW_LIMIT = 50;
Expand All @@ -18,29 +18,36 @@
};

$scope.recipients = null;
$scope.outdated = null;

$scope.getRecipientsEstimate = function() {
var ts = $scope.ts;
if ($scope.recipients === null) {
return ts('(Estimating)');
}
if ($scope.recipients === 0) {
return ts('No recipients');
return ts('Estimate recipient count');
}
if ($scope.recipients === 1) {
return ts('~1 recipient');
return ts('Refresh recipient count');
};

$scope.getRecipientCount = function() {
var ts = $scope.ts;
if ($scope.recipients === 0) {
return ts('(unknown)');
}
return ts('~%1 recipients', {1: $scope.recipients});
return ($scope.recipients === 1) ? ts('~1 recipient') : ts('~%1 recipients', {1 : $scope.recipients});
};

// We monitor four fields -- use debounce so that changes across the
// four fields can settle-down before AJAX.
var refreshRecipients = _.debounce(function() {
$scope.$apply(function() {
$scope.recipients = null;
if (!$scope.mailing) {
return;
}
crmMailingMgr.previewRecipientCount($scope.mailing).then(function(recipients) {
crmMailingMgr.previewRecipientCount($scope.mailing, crmMailingCache, false).then(function(recipients) {
$scope.outdated = (_.difference($scope.mailing.recipients, crmMailingCache.get('mailing-' + $scope.mailing.id + '-recipient-params')) !== 0);
$scope.recipients = recipients;
});
});
Expand All @@ -54,21 +61,36 @@
$scope.$watchCollection("mailing.recipients.mailings.exclude", refreshRecipients);

$scope.previewRecipients = function previewRecipients() {
return crmStatus({start: ts('Previewing...'), success: ''}, crmMailingMgr.previewRecipients($scope.mailing, RECIPIENTS_PREVIEW_LIMIT).then(function(recipients) {
var model = {
count: $scope.recipients,
sample: recipients,
sampleLimit: RECIPIENTS_PREVIEW_LIMIT
};
var options = CRM.utils.adjustDialogDefaults({
width: '40%',
autoOpen: false,
title: ts('Preview (%1)', {
1: $scope.getRecipientsEstimate()
})
});
dialogService.open('recipDialog', '~/crmMailing/PreviewRecipCtrl.html', model, options);
}));
var model = {
count: $scope.recipients,
sample: crmMailingCache.get('mailing-' + $scope.mailing.id + '-recipient-list'),
sampleLimit: RECIPIENTS_PREVIEW_LIMIT
};
var options = CRM.utils.adjustDialogDefaults({
width: '40%',
autoOpen: false,
title: ts('Preview (%1)', {1: $scope.getRecipientCount()})
});

// don't open preview dialog if there is no recipient to show.
if ($scope.recipients !== 0) {
if (!_.isEmpty(model.sample)) {
dialogService.open('recipDialog', '~/crmMailing/PreviewRecipCtrl.html', model, options);
}
else {
return crmStatus({start: ts('Previewing...'), success: ''}, crmMailingMgr.previewRecipients($scope.mailing, RECIPIENTS_PREVIEW_LIMIT).then(function(recipients) {
model.sample = recipients;
dialogService.open('recipDialog', '~/crmMailing/PreviewRecipCtrl.html', model, options);
}));
}
}
};

$scope.rebuildRecipients = function rebuildRecipients() {
return crmMailingMgr.previewRecipientCount($scope.mailing, crmMailingCache, true).then(function(recipients) {
$scope.outdated = (recipients === 0) ? true : false;
$scope.recipients = recipients;
});
};

// Open a dialog for editing the advanced recipient options.
Expand Down
60 changes: 43 additions & 17 deletions ang/crmMailing/services.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,24 +310,46 @@
});
},

previewRecipientCount: function previewRecipientCount(mailing) {
// To get list of recipients, we tentatively save the mailing and
// get the resulting recipients -- then rollback any changes.
var params = angular.extend({}, mailing, mailing.recipients, {
name: 'placeholder', // for previewing recipients on new, incomplete mailing
subject: 'placeholder', // for previewing recipients on new, incomplete mailing
options: {force_rollback: 1},
'api.mailing_job.create': 1, // note: exact match to API default
'api.MailingRecipients.getcount': {
mailing_id: '$value.id'
previewRecipientCount: function previewRecipientCount(mailing, crmMailingCache, rebuild) {
var cachekey = 'mailing-' + mailing.id + '-recipient-count';
var recipientCount = crmMailingCache.get(cachekey);
if (rebuild || _.isEmpty(recipientCount)) {
// To get list of recipients, we tentatively save the mailing and
// get the resulting recipients -- then rollback any changes.
var params = angular.extend({}, mailing, mailing.recipients, {
name: 'placeholder', // for previewing recipients on new, incomplete mailing
subject: 'placeholder', // for previewing recipients on new, incomplete mailing
options: {force_rollback: 1},
'api.mailing_job.create': 1, // note: exact match to API default
'api.MailingRecipients.getcount': {
mailing_id: '$value.id'
}
});
// if this service is executed on rebuild then also fetch the recipients list
if (rebuild) {
params = angular.extend(params, {
'api.MailingRecipients.get': {
mailing_id: '$value.id',
options: {limit: 50},
'api.contact.getvalue': {'return': 'display_name'},
'api.email.getvalue': {'return': 'email'}
}
});
crmMailingCache.put('mailing-' + mailing.id + '-recipient-params', params.recipients);
}
});
delete params.recipients; // the content was merged in
return qApi('Mailing', 'create', params).then(function (recipResult) {
// changes rolled back, so we don't care about updating mailing
mailing.modified_date = recipResult.values[recipResult.id].modified_date;
return recipResult.values[recipResult.id]['api.MailingRecipients.getcount'];
});
delete params.recipients; // the content was merged in
recipientCount = qApi('Mailing', 'create', params).then(function (recipResult) {
// changes rolled back, so we don't care about updating mailing
mailing.modified_date = recipResult.values[recipResult.id].modified_date;
if (rebuild) {
crmMailingCache.put('mailing-' + mailing.id + '-recipient-list', recipResult.values[recipResult.id]['api.MailingRecipients.get'].values);
}
return recipResult.values[recipResult.id]['api.MailingRecipients.getcount'];
});
crmMailingCache.put(cachekey, recipientCount);
}

return recipientCount;
},

// Save a (draft) mailing
Expand Down Expand Up @@ -555,4 +577,8 @@
};
});

angular.module('crmMailing').factory('crmMailingCache', ['$cacheFactory', function($cacheFactory) {
return $cacheFactory('crmMailingCache');
}]);

})(angular, CRM.$, CRM._);

0 comments on commit 7a646a0

Please sign in to comment.