-
Notifications
You must be signed in to change notification settings - Fork 198
Feature: Donation details overview backend #7976
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
Feature: Donation details overview backend #7976
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alaca Nice work! I left a few comments with some suggestions.
Also, should we implement the update_item
endpoint on this PR as well? I'm asking because I'm not sure if we need it for now.
'methods' => WP_REST_Server::READABLE, | ||
'callback' => [$this, 'get_items'], | ||
'permission_callback' => [$this, 'permissionsCheck'], | ||
'args' => $this->get_collection_params(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not super imortant, but since the includeSensitiveData
and anonymousDonations
params are the same for get_item
and get_items
, we can create a getSharedParams to not repeat our selves so we can merge it and both args entries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, nice idea. I would do that if we had more parameters, but since there are only 2, I think it is much cleaner this way.
…re/donation-details-overview-backend-GIVE-2602
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alaca Nice work! I just left a comment about a minor thing.
About the failing unit tests, maybe it can help:
givewp/tests/Unit/API/REST/V3/Routes/Donors/DonorRouteGetCollectionTest.php
Lines 60 to 61 in 1c175ed
// Remove additional property add by the prepare_response_for_collection() method | |
unset($data[0]['_links']); |
Another thing to keep in mind is this limitation related to the $response->get_data()
method:
givewp/tests/Unit/API/REST/V3/Routes/Donors/DonorRouteGetTest.php
Lines 144 to 145 in 1c175ed
//The $response->get_data() method do not include _embedded data | |
$data = $this->responseToData($response, true); |
src/API/REST/V3/Routes/Donations/DonationStatisticsController.php
Outdated
Show resolved
Hide resolved
@glaubersilva, your help and insight on the WP REST API helped me greatly. Thanks, man. Appreciate that. |
Description
This PR adds donation details overview backend.
Pre-review Checklist
@unreleased
tags included in DocBlocks