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

dev/core#4248 - Fix missing price-set usage table #26090

Merged
merged 1 commit into from
May 2, 2023
Merged

dev/core#4248 - Fix missing price-set usage table #26090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 1 addition & 5 deletions CRM/Price/Page/Field.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,8 @@ public function run() {
);

if ($this->_sid) {
$usedByDefaults = [
'civicrm_event' => FALSE,
'civicrm_contribution_page' => FALSE,
];
$usedBy = CRM_Price_BAO_PriceSet::getUsedBy($this->_sid);
$this->assign('usedBy', array_intersect_key($usedByDefaults, $usedBy));
$this->assign('usedBy', $usedBy);
$this->_isSetReserved = CRM_Core_DAO::getFieldValue('CRM_Price_DAO_PriceSet', $this->_sid, 'is_reserved');
$this->assign('isReserved', $this->_isSetReserved);

Expand Down
6 changes: 2 additions & 4 deletions templates/CRM/Price/Page/Field.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,14 @@
{include file="CRM/Price/Form/DeleteField.tpl"}
{elseif $action eq 1024 }
{include file="CRM/Price/Form/Preview.tpl"}
{elseif ($usedBy and $action eq 8) or $usedBy.civicrm_event or $usedBy.civicrm_contribution_page}
{elseif $usedBy}
<div id="price_set_used_by" class="messages status no-popup">
{icon icon="fa-info-circle"}{/icon}
{if $action eq 8}
{ts 1=$usedPriceSetTitle}Unable to delete the '%1' Price Field - it is currently in use by one or more active events or contribution pages or contributions or event templates.{/ts}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{ts 1=$usedPriceSetTitle}Unable to delete the '%1' Price Field - it is currently in use by one or more active events or contribution pages or contributions or event templates.{/ts}
{ts 1=$usedPriceSetTitle}Unable to delete the '%1' Price Field - it is currently in use by one or more active events or contribution pages or contributions or event templates.{/ts}

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a similar extra space here as well (before or contribution pages), while you're in there.

{/if}

{if $usedBy.civicrm_event or $usedBy.civicrm_contribution_page or $usedBy.civicrm_event_template}
{include file="CRM/Price/Page/table.tpl"}
{/if}
{include file="CRM/Price/Page/table.tpl"}
</div>
{/if}

Expand Down
4 changes: 1 addition & 3 deletions templates/CRM/Price/Page/Set.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@
{ts 1=$usedPriceSetTitle}Unable to delete the '%1' price set - it is currently in use by one or more active events or contribution pages or contributions or event templates.{/ts}
{/if}

{if $usedBy.civicrm_event or $usedBy.civicrm_contribution_page or $usedBy.civicrm_event_template}
{include file="CRM/Price/Page/table.tpl"}
{/if}
{include file="CRM/Price/Page/table.tpl"}
</div>
{/if}

Expand Down
11 changes: 11 additions & 0 deletions templates/CRM/Price/Page/table.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
| and copyright information, see https://civicrm.org/licensing |
+--------------------------------------------------------------------+
*}
{* The price field can be used somewhere but not necessarily in a page/event. In that case we still want to display some message. *}
{assign var='showGenericMessage' value=true}
{foreach from=$contexts item=context}
{if $context EQ "Event"}
{assign var='showGenericMessage' value=false}
{if $action eq 8}
{ts}If you no longer want to use this price set, click the event title below, and modify the fees for that event.{/ts}
{else}
Expand All @@ -34,6 +37,7 @@
</table>
{/if}
{if $context EQ "Contribution"}
{assign var='showGenericMessage' value=false}
{if $action eq 8}
{ts}If you no longer want to use this price set, click the contribution page title below, and modify the Amounts or Membership tab configuration.{/ts}
{else}
Expand All @@ -57,6 +61,7 @@
</table>
{/if}
{if $context EQ "EventTemplate"}
{assign var='showGenericMessage' value=false}
{if $action eq 8}
{ts}If you no longer want to use this price set, click the event template title below, and modify the fees for that event.{/ts}
{else}
Expand All @@ -79,3 +84,9 @@
</table>
{/if}
{/foreach}
{if $showGenericMessage}
{if $action neq 8}
{* We don't have to do anything for delete action because the calling tpl already displays something. *}
{ts}This price set is used by at least one contribution, but is not used by any active events or contribution pages or event templates.{/ts}
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically could be a contribution or participant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @larssandergreen. I tried to re-use the wording that was used in all the other messages. And in the future it might be even more things, which is also one reason I was trying to get rid of the hardcode of all the possible array keys in multiple places (e.g. the addition of event template missed some places, so it would likely happen again).

I'm open to changing it but then maybe to something more generic. What it's actually checking is usage in line items and in civicrm_price_set_entity.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just "is used at least one record" or "is used by at least one existing record"? Or contribution, not a big deal.

{/if}
{/if}