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

feat(donation): implement sequential ordering #2912

Closed
ravinderk opened this Issue Mar 14, 2018 · 9 comments

Comments

Projects
None yet
4 participants
@ravinderk
Collaborator

ravinderk commented Mar 14, 2018

User Story

As an admin, I want sequential ordering feature in core so that I can see properly ordered donation on donation listing and other places and also helpful for tax purpose.

Tasks

  • Store donation number as donation title and create the custom table to store sequential number.
  • Display donation number on donation listing page
  • Display donation number on View Donation Details
  • Display donation number on donation receipt
  • Display donation number on donation receipt email
  • Display donation number in donation history (for a donor)
  • Display donation number in log listing page
  • Add donation number to support for history export
  • Integrate with tools
@kevinwhoffman

This comment has been minimized.

Member

kevinwhoffman commented Mar 14, 2018

@ravinderk I looked at how EDD implements sequential ordering, and it works like this:

  1. There is an option in the wp_options table called edd_last_payment_number.
  2. That number from the option is saved to a post meta_key called _edd_payment_number.
  3. Once the the post meta is saved, the option is incremented so it's ready for the next payment.

Is that along the lines of what you were thinking?

@ravinderk

This comment has been minimized.

Collaborator

ravinderk commented Mar 14, 2018

@kevinwhoffman I am not thinking to store latest donation max number to option table, rather I will dig into payment meta table to get max donation number. This seems to me like the safest way to avoid donation number clashes.

@ravinderk

This comment has been minimized.

Collaborator

ravinderk commented Mar 14, 2018

@mathetos @DevinWalker @kevinwhoffman

  1. abandoned donations delete automatically weekly. Do you think we can exclude them from sequential logic?
  2. Do you think donation number will be added at the time of donation creation means you do not have an issue from an admin point of view about seeing donation number with any status in donation list, or donation number is only for completed?
@mathetos

This comment has been minimized.

Member

mathetos commented Mar 14, 2018

@ravinderk would it be possible to only generate the Give Donation ID after it's marked as Complete? That would help alleviate a lot of confusion regarding pending donations, abandoned, etc. Until it's Complete, donations can be tracked with their respective transaction_id.

There might be other nuances that I'm not taking into account, but off the top of my head that seems like the most straight-forward way to tackle this.

@ravinderk

This comment has been minimized.

Collaborator

ravinderk commented Mar 14, 2018

@mathetos that is possible.

@mathetos

This comment has been minimized.

Member

mathetos commented Mar 14, 2018

@ravinderk If we go that route, we'd need to test and confirm the following as well:

  1. Donations marked manually as Complete (like Offline Donations) get a Donation ID in the right sequential order
  2. Donations that were Complete but get set to a different status do NOT lose their Donation ID at all.

Another consideration is how we will list these in the Donations screen. Currently they are all listed according to their Donation ID. What will replace the Donation ID for Pending Donations, Abandoned Donations, etc? Perhaps prefixed Ids, like "#p-102" (for pending) or "#a-103" (for abandoned).

@kevinwhoffman

This comment has been minimized.

Member

kevinwhoffman commented Mar 14, 2018

Edit: Disregard this proposal as we later decided that all attempted donations should receive a sequential ID in #2912 (comment).

Behavior

I agree with @mathetos regarding the behavior. Remember that the reason customers want this is to prevent gaps in the sequence, so we should not assign sequential IDs until a donation is complete.

To reiterate, the sequential ID should be assigned the first time that the donation achieves completed status (whether manually or through a gateway). We can ensure this happens by hooking into status change.

If any change in status happens after the donation was completed, then the ID remains in place.

Presentation

Another consideration is how we will list these in the Donations screen.

See mockup below. Since pending/abandoned donations will not have sequential IDs, I recommend we list them as Pending or Abandoned until they have reached a Complete status, at which point they would display the donation ID (e.g. #1) as usual. Note how the gap between donations #1 and #2 is handled below.

image

Implementation

The behavior described above assigns sequential IDs based on the Complete status, so it is possible that sequential IDs may not fall in chronological order according to date. For that reason, we cannot assume that the most recent donation post contains the latest sequential ID. That's why I believe it's important to have a single reference to the next donation ID maintained in the options table.

@kevinwhoffman

This comment has been minimized.

Member

kevinwhoffman commented Mar 14, 2018

Slack Chat Summary

Participants: @DevinWalker, @ravinderk, @kevinwhoffman
Topic: Discuss whether all attempted donations, or only completed donations, should receive a sequential ID.
Result: Devin asked to see how EDD handles sequential ordering. Kevin looked into it and found that EDD assigns sequential ID to every attempted payment. This is probably the simplest approach that ensures every attempted donation gets a sequential ID. Even if that payment fails or never goes through, there is a record of it and that's what matters to the customer.

image

@kevinwhoffman

This comment has been minimized.

Member

kevinwhoffman commented Mar 14, 2018

@ravinderk Devin and I are in agreement with the following approach which should simplify your tasks. Please assign a sequential ID to all attempted donations regardless of status. See my rationale below:

Now that I’ve thought it through, it seems like adding a bunch of conditions to determine which donations get sequential IDs will only add unnecessary complications. It also creates the potential for sequential IDs to not follow chronological order, since an old pending payment might be completed days later. Ultimately I think customers want to be able to export a list of donations with every sequence number accounted for, whether it was successful or not, and list the amount of the donation that resulted. If we can accomplish that then we’ve done our job.

DevinWalker added a commit that referenced this issue Mar 26, 2018

Merge pull request #2936 from /issues/2912
feat: enable sequential ordering #244
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment