Givewp: fix: display the correct receipt on a links from the donation history page

[kevinwhoffman]

@Benunc commented on Tue Oct 02 2018

User Story

As a recurring donor, I want the ability to see the individual receipt for both a renewal and the original parent donation so that I can reference them individually.

Current Behavior

I currently am only able to see the most recent donation when I click any link on the donation history page.

Expected Behavior

I expect for each link to go to a separate donation record, here:

The PDF receipt links work as intended, but all of the "view renewal" links take me to the most recent donation.

Steps to Reproduce

1. Logged in as a user with a subscription with at least one renewal, navigate to the donation history page, and click the link to view the renewal.
2. It will take you to the most recent donation on that renewal, no matter what.

Related

• https://secure.helpscout.net/conversation/676639136/26630/

Acceptance criteria

• [x] The URL of each receipt contains a unique donation_id e.g. https://example.com/donation-history?donation_id=123.
• [x] The View Receipt link takes you to receipt matching the donation_id URL parameter.
• [x] The View Renewal link takes you to individual, distinct renewal donations, not just the most recent donation.
• [x] The donor can only access the receipt if it is confirmed that the receipt belongs to them through one of the following scenarios:
  
  
  
  • [x] A logged-out user can view the receipt in the same session immediately following the donation. Once the session ends, the receipt URL will display email access form or login form per settings.
  
  
  • [x] A logged-out user can view the receipt through email access if the donation_id matches one of their past donations.
  
  
  • [x] A logged-in user can view the receipt at any time as long as the donation_id matches one of their past donations.
  
  
• [x] If a user clicks an old receipt link such as https://example.com/donation-history?payment_key=123456789 then the [donation_history] shortcode is displayed instead of an individual receipt. This will allow the user to click the intended receipt and we can ensure they see an accurate receipt instead of an incorrect renewal based on the old behavior.
• [x] Make sure all the above changes are implemented for [subscription_history] shortcode in recurring donations add-on.
• [x] Make sure all the above changes are implemented for PDF receipts add-on as well.
• [ ] Make sure to remove all the legacy functions related to payment_key.

Environment

WordPress System Info

### WordPress Environment ###

Home URL: https://livegive.wpsteward.com
Site URL: https://livegive.wpsteward.com
WP Version: 4.9.8
WP Multisite: –
WP Memory Limit: 256 MB
WP Debug Mode: –
WP Cron: ✔
Language: en_US
Permalink Structure: /%year%/%monthnum%/%day%/%postname%/
Show on Front: posts
Table Prefix Length: wp_8138bfdc07_
Table Prefix Length: 14
Table Prefix Status: Acceptable
Admin AJAX: Accessible
Registered Post Statuses: publish, future, draft, pending, private, trash, auto-draft, inherit, request-pending, request-confirmed, request-failed, request-completed, refunded, failed, revoked, cancelled, abandoned, processing, preapproval, give_subscription

Server Environment

Hosting Provider: DBH: localhost, SRV: livegive.wpsteward.com
TLS Connection: Connection uses TLS 1.2
TLS Connection: Probably Okay
Server Info: Apache/2.4.34 (Unix) OpenSSL/1.0.1f
PHP Version: 7.0.32
PHP Post Max Size: 1 GB
PHP Time Limit: 30
PHP Max Input Vars: 5000
PHP Max Upload Size: 512 MB
cURL Version: ❌ 7.35.0, OpenSSL/1.0.1f - We recommend a minimum cURL version of 7.40.
SUHOSIN Installed: –
MySQL Version: ❌ 5.5.61 - We recommend a minimum MySQL version of 5.6. See: WordPress Requirements
Default Timezone is UTC: ✔
fsockopen/cURL: ✔
SoapClient: ✔
DOMDocument: ✔
gzip: ✔
GD Graphics Library: ✔
Multibyte String: ✔
Remote Post: ✔
Remote Get: ✔

Give Configuration

Give Version: 2.2.5
Give Cache: Enabled
Database Updates: All DB Updates Completed.
Give Cache: Enabled
Give Cache: ✔New Donation✔Donation Receipt❌New Offline Donation❌Offline Donation Instructions✔New User Registration✔User Registration Information❌Email access✔Daily Email Report✔Weekly Email Report✔Monthly Email Report❌Renewal Receipt Email❌Subscription Cancelled Email❌Subscription Completed Email✔Subscription Reminder Email❌Subscriptions Email Access
Upgraded From: 2.2.1
Test Mode: Enabled
Currency Code: USD
Currency Position: After
Decimal Separator: .
Thousands Separator: ,
Success Page: https://livegive.wpsteward.com/donation-confirmation/
Failure Page: –
Donation History Page: https://livegive.wpsteward.com/donation-history/
Give Forms Slug: /donations/
Enabled Payment Gateways: Test Donation, Stripe - Credit Card, PayPal Payments Pro, Authorize.net, PayPal Standard
Default Payment Gateway: Stripe - Credit Card
PayPal IPN Verification: Disabled
PayPal IPN Notifications: IPN received for #1084 ( 8H4629943X7440214 ) on 09/04/2018 at 15:46. Status VERIFIED
Donor Email Access: Disabled

Active Give Add-ons

Give - Authorize.net Gateway: ✔ Licensed – by WordImpress – 1.4.3
Give - Email Reports: ✔ Licensed – by WordImpress – 1.1.2
Give - Form Field Manager: ✔ Licensed – by WordImpress – 1.3
Give - Manual Donations: ✔ Licensed – by WordImpress – 1.4.1
Give - PDF Receipts: ✔ Licensed – by WordImpress – 2.3
Give - Recurring Donations: ✔ Licensed – by WordImpress – 1.8.0
Give - Tributes: ✔ Licensed – by WordImpress – 1.5.0
Give - Zapier: ✔ Licensed – by WordImpress – 1.2.1

Other Active Plugins

Ben's Helper Functions: by BenUNC –
Give - Fee Recovery: by GiveWP – 1.7.2
Give - Google Analytics Donation Tracking: by GiveWP – 1.2.1
Give - PayPal Pro Gateway: by GiveWP – 1.2.0
Give - Stripe Gateway: by GiveWP – 2.1.1
Google Analytics for WordPress by MonsterInsights: by MonsterInsights – 7.1.0
Query Monitor: by John Blackbourn & contributors – 3.1.1
User Switching: by John Blackbourn & contributors – 1.3.1
WP Crontrol: by John Blackbourn & contributors – 1.6.2

Inactive Plugins

Akismet Anti-Spam: by Automattic – 4.0.8
Email Cop: by Ashfame – 0.1.1
Give - Database HealthCheck: by WordImpress – 0.0.2
Give - Mollie Gateway: by WordImpress – 1.1.1
Hello Dolly: by Matt Mullenweg – 1.6
Page Builder by SiteOrigin: by SiteOrigin – 2.8.2
RegistrationMagic Premium: by Registrationmagic – 3.8.1.7
Restrict Content Pro: by Restrict Content Pro Team – 2.9.10
Series Engine: by Eric Murrell (Volacious) – 2.6.1
SiteOrigin Widgets Bundle: by SiteOrigin – 1.12.1
The GDPR Framework: by Codelight – 1.0.11
WonderPlugin Tabs: by Magic Hills Pty Ltd – 5.8

Active MU Plugins

api-hacks.php: by –

Theme

Name: Twenty Seventeen
Version: 1.7
Author URL: https://wordpress.org/
Child Theme: No – If you're modifying Give on a parent theme you didn't build personally, then we recommend using a child theme. See: How to Create a Child Theme

---

@ravinderk commented on Tue Oct 02 2018

@Benunc: @mehul0810 already report this issue and i fixed this in give/release/2.2.6: https://github.com/impress-org/give/issues/3705#issuecomment-425108247

Let me know if you still can reproduce this otherwise close it.

---

@nishitlangaliya commented on Tue Oct 09 2018

Slack Call Summary

Participants: @nishitlangaliya , @ravinderk
Topic: Discussion regarding https://github.com/impress-org/give-recurring-donations/issues/761
Result: I had a confusion regarding reproduce the case so I talked with @ravinderk and as per suggestion I need to run npm and then need to re try the reproduce case and call back if still issue.

---

@nishitlangaliya commented on Tue Oct 09 2018

Slack Call Summary

Participants: @nishitlangaliya , @ravinderk
Topic: Discussion regarding https://github.com/impress-org/give-recurring-donations/issues/761
Result: Currently @ravinderk is looking into this and he will let me know if anything.

---

@nishitlangaliya commented on Tue Oct 09 2018

Slack Call Summary

Participants: @nishitlangaliya , @ravinderk
Topic: Discussion regarding https://github.com/impress-org/give-recurring-donations/issues/761
Result: We both went through the steps and were able to reproduce the case. And discussed why its happening. We found that adding a renewal manually from subscription tab in admin side it always takes a parent payment key as a payment key . So it always fetch the latest donation history item.

For more understanding I have prepared video for that:

https://www.useloom.com/share/fa85e81e2d984706aa1855f7a4544df9

For now I am moving this issue to blocked and will discuss more about this on tomorrow dev call.

---

@kevinwhoffman commented on Fri Oct 12 2018

Per dev team discussion, we likely need to modify the URL parameters of the receipt links so that we have a unique identifier with which to pull the correct receipt. It seems like donation_id could be used to fetch the receipt instead of payment_key since we know donation IDs should be unique for every donation, including renewals.

Let's regroup to discuss after Give 2.3.0 releases.

---

@kevinwhoffman commented on Wed Oct 17 2018

We're moving this to core issue and need to update acceptance criteria before beginning.

[kevinwhoffman]

This may need a recurring issue for subscription history shortcode.

[kevinwhoffman]

@mehul0810 Let's use our one-on-one call next week to work out this issue, the acceptance criteria, backwards compatibility, and effects on other add-ons.

[mehul0810]

@kevinwhoffman After investigating on this issue, I think that we should create a payment key using donation id instead of email as per line here https://github.com/impress-org/give/blob/13ca1052ddd74c467b3d87ba9a2ce8139ced99eb/includes/payments/class-give-payment.php#L650

It will help us to open donation receipts for renewals as well and the same will work with donation receipt and subscription history shortcodes. We need to make sure that payment key stored in renewals are not of parent donation as we'll be doing it based on donation id.

Also, we will need an upgrade routine to update the existing payment keys for backward compatibility.

Let me know your thoughts. If we agree with this, then I'll update the acceptance criteria in the issue description.

[kevinwhoffman]

@mehul0810 I would like to avoid another database upgrade if possible, especially one that would affect every donation. Let's see if we can come up with a solution using the existing data that will allow us to link to the correct receipt even for renewals.

We could use feedback from @DevinWalker and @ravinderk as well.

[ravinderk]

@kevinwhoffman I also think that we do not need database upgrade as we discussed before the simple solution will be adding donation id in receipt link: https://github.com/impress-org/give/blob/7c70f9374bd67dd64db76a335cebc01338446cb0/templates/history-donations.php#L211

@mehul0810 FYI we creates payment key before creating donation: https://github.com/impress-org/give/blob/13ca1052ddd74c467b3d87ba9a2ce8139ced99eb/includes/process-donation.php#L128

[kevinwhoffman]

Slack Chat Summary

Participants: @ravinderk @mehul0810 @kevinwhoffman
Topic: Whether we can use donation ID as URL parameter
Result: We agree that donation ID can be set as a URL parameter for the receipt links. Ravinder confirmed that all donation IDs (like WordPress post IDs) are unique so there is no concern about duplicate IDs. We have also discussed security concerns and how to handle old receipt URLs that may be in the donor's email inbox from past donations. These concerns are addressed in the updated acceptance criteria of this issue.

[mehul0810]

Slack Call Summary

Participants: @mehul0810 @ravinderk
Topic: Discussion about handling the donation id with purchase session
Result: I've discussed with Ravinder on how to handle the donation id which is not generated when we set the purchase session right now. Ravinder asked me to add the donation id to the session when the donation id is generated.

[mehul0810]

Slack Call Summary

Participants: @mehul0810 @ravinderk
Topic: Discussion about handling of ajaxified donation receipt
Result: Ravinder explained me how we are handling the donation receipt with AJAX and the parameter passed. I'll implement the changes for donation id replacing the payment key as required based on the change noticed.

[mehul0810]

Update: I've added 2 more acceptance criteria points which I think should be included.
(1) For PDF receipts add-on
(2) Remove legacy code related to payment_key

Please see updated acceptance criteria in the issue description.

[kevinwhoffman]

@mehul0810 On the team call, we mentioned that the subscription details table makes sense to display on the parent donation but not on each renewal. However a link is needed to get from the renewal to manage the subscription. I will review the subscription management user flow and get back to you with a better direction.

[mehul0810]

Slack Call Summary

Participants: @mehul0810 @ravinderk
Topic: Discussion about session handling with ajax based donation receipt
Result: Ravinder explained me how the ajax based donation receipt will show error when the session is not generated. So, when a user is logged in or using email access form just initialize the session to take benefit of it.

[mehul0810]

Slack Call Summary

Participants: @ravinderk @mehul0810
Topic: Discussion on the legacy functions related to the issue.
Result: We have discussed whether to remove legacy functions or not. Then, came to the conclusion that we will remove the legacy functions in another issue and create an issue for that. Also, email tags will follow the new donation receipt format.

[mehul0810]

Slack Call Summary

Participants: @mehul0810 @ravinderk
Topic: Discussion on nonce issue noticed while working on this issue
Result: We have discussed and tried to reproduce the same scenario again and still we feel to keep an eye on this issue as it is not exactly reproduced. I'll work on the rest of the improvements suggested by Ravinder on this issue and test the issue thoroughly with a new testing video and fix if there is any issue.