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 fee amount when contributions have multiple associated participants #24576

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MegaphoneJon
Copy link
Contributor

@MegaphoneJon MegaphoneJon commented Sep 20, 2022

Overview

See https://lab.civicrm.org/dev/core/-/issues/3859 for replication steps and a description of the bug.

This PR handles the most immediate issue - when a contribution has a single line item, the existing code works. With multiple line items, it changes a value to be incorrect. This ensures the code only works on contributions with a single line item.

Before

Editing a contribution (through the UI) changes civicrm_participant.fee_amount to match the value of the entire contribution - even when there are multiple line items.

After

The fee_amount is only changed when a single line item exists.

Comments

This is just an if statement, with a tiny bit of strict type enforcement while there are only two places isSingleLineItem() is referenced.

@civibot
Copy link

civibot bot commented Sep 20, 2022

(Standard links)

@civibot civibot bot added the master label Sep 20, 2022
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 19, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 19, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 20, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 23, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 23, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 23, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 23, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 23, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 23, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 23, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 23, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 24, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 24, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 24, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
mattwire pushed a commit to mattwire/civicrm-core that referenced this pull request Jun 25, 2023
This updates the default online event template to use the variables assigned by
the workflow template

This works to standardise the variables previously in dataArray
, lineItem, totalAmount, taxAmount to reflect the
approach in other templates per
https://docs.civicrm.org/user/en/latest/email/message-templates/#variables-and-tokens-in-workflow-message-templates

However, for participants, per the helpful discussion at
civicrm#24576

the expectation is that the primaryParticipant gets the values for
all participants whereas the others only get line items,
tax breakdowns, totals that relate to them.

Hence I have assigned an array participants that holds detials for all participants
and the template iterates through them showing all or just the
one that relates to the assigned participant id depending on
whether it is primary.

This allows the template to display in the Message preview and should
also mean that those values that I have addressed will always reflect
the participantID being used. This also addresses some notices
and incompatibility with secure smarty.

However, there are still values I haven't made sense of, or otherwise left
out of the scope of this PR in the template.

Also note I updated the taxBreakdown in the contribution trait - I decided it
was cloberring the amount as it iterated through the line items - rather than
doing a running total.
@MegaphoneJon
Copy link
Contributor Author

test this please

@mlutfy
Copy link
Member

mlutfy commented Sep 26, 2024

jenkins, test this please

1 similar comment
@mlutfy
Copy link
Member

mlutfy commented Nov 7, 2024

jenkins, test this please

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.

2 participants