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.

Sign up for GitHub

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

Jump to bottom

dev/mail#90 - Allow re-use mailing on draft mailings and adhoc mailings #20058

Merged
merged 6 commits into from
Apr 19, 2021
Merged

dev/mail#90 - Allow re-use mailing on draft mailings and adhoc mailings #20058

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b4e1015
Allow re-use mailing on draft mailings
larssandergreen Apr 14, 2021
163aabb
Fixed bad bracket indent
larssandergreen Apr 15, 2021
5ebfe24
Add " - Copy" to the end of the new mailing name when cloning
larssandergreen Apr 16, 2021
ff73097
Changed " - Copy" append to "Copy of " prepend for consistency
larssandergreen Apr 16, 2021
b1a7daf
Update CRM/Mailing/Controller/Send.php
totten Apr 17, 2021
19a1f88
Change Re-use to Copy
larssandergreen Apr 17, 2021
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: 4 additions & 0 deletions CRM/Mailing/Controller/Send.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ public function __construct($title = NULL, $action = CRM_Core_Action::NONE, $mod
}
if ($mid && !$continue) {
$clone = civicrm_api3('Mailing', 'clone', ['id' => $mid]);
civicrm_api3('Mailing', 'create', [
'id' => $clone['id'],
'name' => "Copy of " . $clone['values'][$clone['id']]['name'],
totten marked this conversation as resolved.
Show resolved Hide resolved
]);
CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/a/', NULL, TRUE, '/mailing/' . $clone['id']));
}
}
Expand Down
22 changes: 10 additions & 12 deletions CRM/Mailing/Selector/Browse.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,12 +245,6 @@ public function &getRows($action, $offset, $rowCount, $sort, $output = NULL) {
'qs' => 'mid=%%mid%%&reset=1',
'title' => ts('View Mailing Report'),
],
CRM_Core_Action::UPDATE => [
'name' => ts('Re-Use'),
'url' => 'civicrm/mailing/send',
'qs' => 'mid=%%mid%%&reset=1',
'title' => ts('Re-Send Mailing'),
],
CRM_Core_Action::DISABLE => [
'name' => ts('Cancel'),
'url' => 'civicrm/mailing/browse',
Expand All @@ -264,6 +258,12 @@ public function &getRows($action, $offset, $rowCount, $sort, $output = NULL) {
'qs' => 'mid=%%mid%%&continue=true&reset=1',
'title' => ts('Continue Mailing'),
],
CRM_Core_Action::UPDATE => [
'name' => ts('Re-Use'),
Copy link
Member

Choose a reason for hiding this comment

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

How hard would it be to make this label adapt?

This is maybe a bit nitpicky, but "Re-Use" suggests that the mailing has been "used" before. A draft hasn't been used before.

If it's not too hard, maybe something like this would be more apropos:

  'name' => $thisItemIsADraft ? ts('Copy') : ts('Re-Use'),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I suggested in the issue changing the text to "Copy". My thought is that it would be better to keep the label consistent for all mailings and just change it to "Copy" throughout. Otherwise, users may wonder what the difference is between "Copy" and "Re-use". Copy is also consistent with events, contribution pages, price sets, etc which all use some variation of Copy, Make a Copy, Copy Price Set, etc.

Copy link
Member

Choose a reason for hiding this comment

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

Yup. "Copy" is a small UI change in near term (and we might want to tweak the docs after this PR), but I think you're right - "Copy" is more consistent in the big picture. It also matches the Copy of ... text.

'url' => 'civicrm/mailing/send',
'qs' => 'mid=%%mid%%&reset=1',
'title' => ts('Re-Send Mailing'),
],
CRM_Core_Action::DELETE => [
'name' => ts('Delete'),
'url' => 'civicrm/mailing/browse',
Expand Down Expand Up @@ -363,12 +363,6 @@ public function &getRows($action, $offset, $rowCount, $sort, $output = NULL) {
if ($allAccess || $showCreateLinks) {
$actionMask = CRM_Core_Action::VIEW;
}

if (!in_array($row['id'], $searchMailings)) {
if ($allAccess || $showCreateLinks) {
$actionMask |= CRM_Core_Action::UPDATE;
}
}
}
else {
if ($allAccess || ($showCreateLinks || $showScheduleLinks)) {
Expand Down Expand Up @@ -409,6 +403,10 @@ public function &getRows($action, $offset, $rowCount, $sort, $output = NULL) {
}
}

if ($allAccess || $showCreateLinks) {
$actionMask |= CRM_Core_Action::UPDATE;
}

// check for delete permission.
if ($allowToDelete) {
$actionMask |= CRM_Core_Action::DELETE;
Expand Down