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 user-defined donation prices being saved with zero value #1363

Merged
merged 2 commits into from
Jun 14, 2024
Merged

Fix user-defined donation prices being saved with zero value #1363

merged 2 commits into from
Jun 14, 2024

Conversation

shanebrowncs
Copy link
Contributor

@shanebrowncs shanebrowncs commented Jun 9, 2024

Hi,

After updating my alf.io instance from 2.0-M4-2304 to 2.0-M4-2402-3 I noticed donation fields with user-defined pricing no longer work properly.

Here is an example of a ticket which costs $5.50 with an attached donation of $7.00:
2024-06-09_01

When I reach the summary screen you can see the donation value in the table is $0.00:
2024-06-09_02

Upon investigating I found this is being caused by the incorrect field being inserted into additional_service_item.src_price_cts. When the event/{eventName}/reserve-tickets endpoint is called, eventually we get to the 'AdditionalServiceManager.bookAdditionalServiceItems' method where we are inserting the additional_service_item rows to the database. For the src_price_cts column we are inserting as.getSrcPriceCts() which when the price is NOT fixed has a value of 0. When calling 'AdditionalServicePriceContainer.getSrcPriceCts' it already has it's own method of dealing with this issue by checking whether the price is fixed:

    @Override
    public int getSrcPriceCts() {
        if(additionalService.isFixPrice()) {
            return additionalService.getSrcPriceCts();
        }
        return Optional.ofNullable(customAmount).map(a -> MonetaryUtil.unitToCents(a, currencyCode)).orElse(0);
    }

Because of this it seemed appropriate to just substitute the call to as.getSrcPriceCts() with pc.getSrcPriceCts(). If you could please let me know if this is something you would be interested in merging. I am not very familiar with the codebase but would be happy to look at implementing further required changes if necessary.

Thank you.

This change uses AdditionalServicePriceContainer to determine
the source price regardless of whether the price is fixed or not
when saving an 'additional_service_item'. This will stop the value
'0' from being saved to the 'additional_service_item.src_price_cts'
field when the 'additional_service' entry has a non-fixed price.
@shanebrowncs shanebrowncs marked this pull request as ready for review June 9, 2024 19:33
@syjer
Copy link
Member

syjer commented Jun 10, 2024

Hi @shanebrowncs , thank you for noticing and opening a PR.

If it's possible, could you also add a test? (My guess a unit test would be enough)

The added unit test verifies that the 'bookAdditionalServiceItems'
method in 'AdditionalServiceManager.java' properly commits the user
specified amount to 'additional_service_item.src_price_cts' when
the additional service price is NOT fixed.
@shanebrowncs
Copy link
Contributor Author

Hi @syjer, I have added an additional commit with a new unit test file for AdditionalServiceManager. It tests both main code paths in bookAdditionalServiceItems(). Please let me know if this is acceptable.

Is there any chance this could be included in a patch release? I already have upgraded my instance to 2.0-M4-2402-3 and rely on this feature.

Thank you.

@syjer
Copy link
Member

syjer commented Jun 11, 2024

hi @shanebrowncs , everything looks fine, thank you for the PR! We will merge it, cherry pick on the m4 branch and do a patch release.

@shanebrowncs
Copy link
Contributor Author

Great, thank you!

@cbellone cbellone merged commit d5c1c58 into alfio-event:main Jun 14, 2024
9 of 11 checks passed
@cbellone
Copy link
Member

merged. Thank you!

cbellone pushed a commit that referenced this pull request Jun 16, 2024
* Fix user-defined donation prices being saved with zero value.

This change uses AdditionalServicePriceContainer to determine
the source price regardless of whether the price is fixed or not
when saving an 'additional_service_item'. This will stop the value
'0' from being saved to the 'additional_service_item.src_price_cts'
field when the 'additional_service' entry has a non-fixed price.

* Add testNonFixedPriceCommitted unit test for AdditionalServiceManager

The added unit test verifies that the 'bookAdditionalServiceItems'
method in 'AdditionalServiceManager.java' properly commits the user
specified amount to 'additional_service_item.src_price_cts' when
the additional service price is NOT fixed.

(cherry picked from commit d5c1c58)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants