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(compat): prevent missing donation data with Restrict Content Pro #2710

Closed
12 tasks done
ravinderk opened this issue Jan 25, 2018 · 13 comments
Closed
12 tasks done

fix(compat): prevent missing donation data with Restrict Content Pro #2710

ravinderk opened this issue Jan 25, 2018 · 13 comments
Assignees

Comments

@ravinderk
Copy link
Collaborator

ravinderk commented Jan 25, 2018

Issue Overview

If you activate restrict content pro then you will see information missing on donation listing page in admin.

image

Related

Tasks

  • Implement solution
    • Renaming the meta type payment to donation
    • Renaming the table {*}_give_paymentmeta to {*}_give_donationmeta
    • Renaming the table and payment_id -> donation_id column
    • Rename $wpdb property paymentmeta -> donationmeta
    • Add DB upgrade
    • Add backward compatibility
  • Tests
    • Check DB upgrade
    • Test with multiple addon
  • Add blog post
  • Add issue for document update
@ravinderk ravinderk self-assigned this Jan 25, 2018
@ravinderk ravinderk added this to the 2.0.1 milestone Jan 25, 2018
@ravinderk
Copy link
Collaborator Author

@DevinWalker

Cause of issue:

I found the conflict between RCP and Give. We have same $wpdb->paymentmeta table name param, so restrict content pro overwriting over table name or vice versa 😞
For ref:
https://github.com/WordPress/WordPress/blob/master/wp-includes/meta.php#L931
https://github.com/restrictcontentpro/restrict-content-pro/blob/master/restrict-content-pro.php#L371

@ravinderk ravinderk modified the milestones: 2.0.1, 2.2 Jan 25, 2018
@DevinWalker DevinWalker removed this from the 2.2 milestone Mar 6, 2018
@kevinwhoffman kevinwhoffman changed the title Conflict with restrict content pro fix(compat): prevent missing donation data with Restrict Content Pro Mar 11, 2018
@samsmith89
Copy link

samsmith89 commented Mar 19, 2018

Having this issue currently with a customer. Was this fixed and reopened, or still waiting?
https://secure.helpscout.net/conversation/543868415/15603?folderId=1823994

@kevinwhoffman kevinwhoffman added this to the Sprint: 2018/03/28 - 2018/04/10 milestone Mar 30, 2018
@kevinwhoffman
Copy link
Contributor

kevinwhoffman commented Mar 30, 2018

@ravinderk Please complete the following tasks but do not move forward with implementation until we discuss with @DevinWalker.

  • Post a possible solution in this issue, including whether a database upgrade routine would be necessary.
  • Update the task list based on your proposed solution.
  • Update the story point estimate based on your proposed solution.
  • Notify @DevinWalker and @kevinwhoffman to discuss next steps.

@ravinderk
Copy link
Collaborator Author

ravinderk commented Apr 2, 2018

@kevinwhoffman I already explained the problem here: #2710 (comment)

paymentdata is not a general table name prefix that was logic behind keeping. But conflict with Restrict content pro prove that In future, we will have the possibility to conflict with a plugin which can process payment own there own.

Thanks to Restrict content pro team which took the first step to resolve conflict (release/3.0): https://github.com/restrictcontentpro/restrict-content-pro/issues/1656

To resolve this problem we can take following steps:

@DevinWalker @kevinwhoffman Let me know what do you think.

@kevinwhoffman
Copy link
Contributor

kevinwhoffman commented Apr 3, 2018

@ravinderk Thanks for the breakdown. RCP seems to have merged a fix on their end, but it is not slated for release until RCP 3.0, which does not look like it has a known release.

We should still adress it from our end, but this does not look like a minor fix, so I'd like to hear from @mathetos how many users this is affecting. Then we can discuss timeline.

@DevinWalker DevinWalker removed this from the Sprint: 2018/03/28 - 2018/04/10 milestone Apr 3, 2018
@mathetos
Copy link
Member

mathetos commented Apr 3, 2018

@kevinwhoffman Only two customers reporting as far as I've seen so far. But the urgency is related to the fact that Give 2.0+ is completely unusable for any user that is also running RCP. Additionally, I can imagine this problem would happen with other plugins that call paymentmeta using wpdb as well.

@Benunc
Copy link
Member

Benunc commented Apr 4, 2018

This issue also breaks functionality with Zapier and Give, and I have a user reporting that, and I replicated the problem on my test (live) install.

Findings

WHen RCP and Give are active on the same site, in addition to the above reference (donor missing) issue, Zapier is unable to connect to the api URL at all.

Additionally, the API returns incomplete and incorrect information:

Before activating RCP:


screen shot 2018-04-04 at 4 11 42 pm


After Activating RCP:


screen shot 2018-04-04 at 4 12 12 pm


So, in addition to removing all relevant data from the API, it also incorrectly attributes the donations to the POST "Thought 53" which isn't a form at all.

@ravinderk
Copy link
Collaborator Author

@WordImpress/support I think this issue fixed with latest https://github.com/restrictcontentpro/restrict-content-pro/releases/tag/2.9.12

I tested it. Can you confirm?

@samsmith89
Copy link

I will check on this today.

@mathetos
Copy link
Member

@ravinderk Two things:

  1. It looks like that PR on RCP was merged into version 3.0, but that's not their current release.
  2. Even if they fix it on their end, I believe we need a way to avoid this happening with other plugins as well. Is there any prefixing or namespacing we can do to avoid this problem in the future?

@ravinderk
Copy link
Collaborator Author

@mathetos yes, I will be fixing in release assign to this issue.

ravinderk added a commit to impress-org/give-database-healthcheck that referenced this issue May 22, 2018
@DevinWalker DevinWalker modified the milestones: Sprint: 2018/05/08 - 2018/05/22, Sprint: 2018/05/08 - 2018/06/05 May 23, 2018
DevinWalker pushed a commit that referenced this issue May 24, 2018
fix(compat): prevent missing donation data with Restrict Content Pro #2710
@mehul0810
Copy link
Contributor

@ravinderk I can see an issue on 2.2 Give core branch even after running DB upgrade.
image

@mehul0810 mehul0810 reopened this May 28, 2018
@ravinderk
Copy link
Collaborator Author

@mehul0810 Close this issue because it fixed.

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

No branches or pull requests

7 participants