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

Clarify structure of payment meta #1132

Closed
kevinwhoffman opened this issue Oct 18, 2016 · 10 comments

Comments

Projects
None yet
4 participants
@kevinwhoffman
Copy link
Member

commented Oct 18, 2016

Issue Overview

The existing payment meta array exhibits some unexpected behavior outlined below.

1. Form email is stored twice

Both ['email'] and ['user_info']['email'] contain the email submitted in the form. It would be logical for ['user_info']['email'] to reflect the email associated with the user ID.

2. First and Last Name reflect name used on form, not user name

['user_info']['first_name'] and ['user_info']['last_name'] reflect the name submitted in the form, not the name associated with the user ID. Need to decide if we want to store both user first/last names and first/last names entered on form.

3. Address is billing address, not user address

Looking at the payment meta array, ['user_info']['address'] appears to be the address of the user (aka donor). In fact, ['user_info']['address'] is the billing address entered on the form. It makes more sense in its own ['billing_address'] key, but that may present backwards compatibility issues. This decision affects #370.

Example _give_payment_meta

See comments below that highlight the above discrepancies.

array(
    'key' => '8090dbc3144ae8e8460ede4b9459688f',
    'form_title' => 'All Fields Test Form',
    'email' => 'test@example.com', // email is from form, could be different from user
    'date' => '2016-10-09 13:07:55',
    'user_info' => array(
        'first_name' => 'Kevin', // name is from form, could be different from user
        'last_name' => 'Hoffman', // name is from form, could be different from user
        'id' => 1,
        'email' => 'test@example.com', // duplicate email is from form, could be different from user
        'address' => array( // this is actually billing address, not user address
            'line1' => '123 Billing St'
            'line2' => 'Apt 1'
            'city' => 'Pittsurgh'
            'state' => 'PA'
            'country' => 'US'
            'zip' => '15212'
        ),
    ),
    'form_id' => '220',
    'price_id' => '',
    'fees' => array(),
    'currency' => 'USD',
    'ffm_text' => 'Text Value',
    'ffm_textarea' => 'Textarea Value',
    'ffm_select' => 'Dropdown Option 1',
    'ffm_date' => '10/09/2016 13:04',
    'ffm_radio' => 'Radio Option 1',
    'ffm_checkbox' => 'Option 1| Option 2',
    'ffm_email' => 'ffm_test@example.com',
    'ffm_phone' => '(555) 555-5555',
    'ffm_url' => 'http://example.com',
    'ffm_multiselect' => 'Option 1| Option 2',
    'ffm_repeat' => array(
        0 => 'foo| bar',
        1 => 'baz| qux',
    ),
)

Next Steps

In the case of First Name, Last Name, Email, and Address, the data entered in the form could be totally different from that of the user ID associated with the payment. We need to figure out how to handle that scenario while keeping an eye on backwards compatibility for existing sites that may be relying on the structure shown above.

@DevinWalker

This comment has been minimized.

Copy link
Member

commented Oct 18, 2016

@ravinderk can you review this when you get a chance and provide your insight.

@DevinWalker DevinWalker added this to the 1.8 milestone Oct 18, 2016

@ravinderk

This comment has been minimized.

Copy link
Collaborator

commented Oct 19, 2016

@DevinWalker Alright

@ravinderk

This comment has been minimized.

Copy link
Collaborator

commented Oct 20, 2016

@DevinWalker @kevinwhoffman Here are my thoughts on this:

1. Form email is stored twice

Yes, I think we can remove ['email'] with backward compatibility.

2. First and Last Name reflect name used on form, not user name

3. Address is billing address, not user address

user_info in payment meta is confusing term between user and donor, because it is donor data for specific donation expect containing user id instead of donor id. I think we can replace ['user_info']['id'] value with donor id and create new array key for user id. I am also agree to rename address to billing_address and we can also rename user_info to donor_info then whole array data will become less confusing.

Array
(
    [key] => f2358650851aa52a053604a561dbb28c
    [form_title] => form testing
    [email] => ravinder+576-1@gmail.com
    [date] => 2016-10-20 17:25:38
    [user_info] => Array
        (
            [first_name] => Ravinder
            [last_name] => Kumar
            [id] => 22
            [email] => ravinder+576-1@gmail.com
            [address] => Array
                (
                    [line1] => street 1, Ujina
                    [line2] => street 2
                    [city] => gurgaon
                    [state] => HR
                    [country] => IN
                    [zip] => 122001
                )

        )

    [form_id] => 2120
    [price_id] => 
    [fees] => Array
        (
        )

    [currency] => USD
)

Note [Bug]: when we view donation details then it always show primary email instead of new email (secondary)
screen shot 2016-10-20 at 9 12 24 pm
screen shot 2016-10-20 at 9 03 00 pm

@kevinwhoffman

This comment has been minimized.

Copy link
Member Author

commented Oct 20, 2016

@ravinderk Thanks for the feedback. I agree that the source of confusion comes down to the relationship between WordPress user and Give donor. Sometimes what appears to be donor information is actually user information and vice-versa. The confusion exists both in the structure of the _give_payment_meta array and in the interface as you noted in the bug above. I'll think on this some more and come back with a recommendation.

@drzraf

This comment has been minimized.

Copy link

commented Dec 7, 2016

Let me mention here issue #1296

  • we do need donor meta (donor's address, birthday, ...) which, unlike payment meta, is unique (= deduplicated) for all the payments this person does
    I bet in most cases address only apply to donor rather than payment; in many countries the "address" is spontaneously seen as a personal information rather than as a financial information. Another question is whether one user can have multiple set of addresses (most online shop softwares allow such a cardinality but should it be the case here?), user's billing address is one specific case of a user having more than 1 address.

  • IHMO neither user_info nor email have their place in payment/payment meta. User info = User meta. Payment entity should just contains payment information + references (to the ID of a set of payment meta (custom fields), to the ID a Give customer, ...).

  • Like previously said, Give donor and WordPress user add to confusion. In #1296, it was said to be an additional feature to ease the life of the site owner. In such case, the "link" between a payment and a WordPress user (_give_payment_user_id) could be simply dropped since wp_give_customers' user_id already has the same purpose
    (could even be implemented as a distinct Give plugin)
    Creating as many WP user as there are donors isn't somewhat a niche-case? (Few people will really want to use WP as a CRM replacement)

@kevinwhoffman

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2016

@drzraf Thanks for your feedback here. The points you bring up are exactly why we are looking at the structure of payment meta and user meta, where they overlap, and how to handle that overlap. We will be bringing more clarity to this situation as we work towards the 1.8 release.

@DevinWalker DevinWalker modified the milestones: 1.9, 1.8 Dec 21, 2016

@drzraf

This comment has been minimized.

Copy link

commented Jan 26, 2017

Any progress on this ?
I'd a look at the (changelog.txt-less) 1.9 branch, but couldn't figure anything related to this.
Is there W.I.P out there.

Personally I trying to fight the issue in our WP/Give instance.
I've setup a converter to the new scheme but having new donations respecting such a data structure using Give's hooks is (very) far from easy.
I'd be really happy to hear from you about the state of this moot issue.

@DevinWalker

This comment has been minimized.

Copy link
Member

commented Jan 26, 2017

I understand it's difficult and we're shifting focus onto this once 1.8 is released. It's definitely an important priority.

@drzraf

This comment has been minimized.

Copy link

commented Feb 1, 2017

Currently, if Give is set to not create WordPress users, customers gets no "address" meta (only the payment does).
As a consequence, customer metas are not editable and even if customer "action" would do customer->update_meta("_give_user_address"); that would make the Give customer edit-form to work since it only relies upon address from the WP User object's meta.

Given that I need editable customers now (not WordPress User) and that I'm in the process of converting metadata, my genuine question: I'd like to be futur-proof and intend to migrate payment's address meta into customers' "_give_user_address" meta (the same meta' name used for users).
Have you yet thought about using the very same name? Another one? Another name/mechanism?

sorry to be appear like pressuring about this, but I need to move on while avoiding reprocessing my database uselessly if 1.8 comes with something different.

@DevinWalker DevinWalker modified the milestones: 2.0, 1.9 Feb 13, 2017

@ravinderk ravinderk referenced this issue May 30, 2017

Closed

Refactor payment meta #1746

3 of 3 tasks complete

@ravinderk ravinderk added the has pr label Jun 28, 2017

@DevinWalker DevinWalker removed their assignment Jul 6, 2017

DevinWalker added a commit that referenced this issue Aug 18, 2017

Merge pull request #1794 from /issues/1132
Refactor payment meta
@DevinWalker

This comment has been minimized.

Copy link
Member

commented Aug 18, 2017

Thanks @ravinderk on this excellent PR: #1794

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.