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

Issues/1190 #1202

Merged
merged 4 commits into from
Nov 7, 2016
Merged

Issues/1190 #1202

merged 4 commits into from
Nov 7, 2016

Conversation

ravinderk
Copy link
Collaborator

Description

This pr will fix #1190

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

@ravinderk
Copy link
Collaborator Author

@DevinWalker Please review this commit a17acc7.

I changed [give_forms](https://github.com/ravinderk/Give/blob/a17acc7ea40273ab04f7f350dea31d9d5f899df6/includes/payments/class-payments-query.php#L515#L527) functions. Previously it was getting donation IDs from logs and now I am adding meta query when user query for specific form related donations.

Suggestions

I think logs are not static and we should provide a tool:

  1. To flush them automatically
  2. Give the option to enable/disable logs

Question

Currently, user can fetch a list of donors for s specific form of all completed donations.
Do we have to create extra params to donors subcommand which filter a list of donors on basis of donation status?

What do you think?

@DevinWalker
Copy link
Member

@ravinderk can you resolve the conflict here?

@ravinderk
Copy link
Collaborator Author

@DevinWalker conflict resolved.

@DevinWalker
Copy link
Member

@ravinderk thanks!

@@ -512,43 +512,19 @@ public function give_forms() {
return;
}

global $give_logs;

$args = array(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you're cleaning up the code here, which is nice. Have you tested this throughout the ensure it works as expected?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DevinWalker I did not find the use of this code inside core but it is working fine with give-cli.

}

$sales = $give_logs->get_connected_logs( $args );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of counting logs you're counting the payments attached to each form?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

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.

None yet

2 participants