Update deposits REST API docs with decoupled deposit changes #7848
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: 0 B Total Size: 1.27 MB ℹ️ View Unchanged
|
introduced in #7828
Thanks @shendy-a8c – updated 7.0.0 → 7.1.0 in dc9227b. Changes visible on docs preview here. |
|
To update this PR's progress: I'll be:
|
Jinksi
added
pr: needs review
and removed
status: blocked
|
I've validated the docs against the production server deployment of decouple deposits API changes. I've removed the deprecated fields and values as per the discussion above. See preview here. Ready for re-review. |
| @@ -481,7 +413,7 @@ Fetches a deposit by ID. | |||
|
|
|||
| If a deposit is found for the provided ID, the response will return a [**Deposit**](#deposit-object) object. | |||
|
|
|||
| If no deposit is found for the provided ID, the response will be an empty array. | |||
| If no deposit is found for the provided ID, the response will return a `500` status code. | |||
There was a problem hiding this comment.
NB: The HTTP response code was changed to 404 on the server, the client still responds with 500.
There was a problem hiding this comment.
Interesting … I like the idea that we work towards no 500 errors from any of our APIs.
There was a problem hiding this comment.
Yeh this came up in #4342 which implemented the server returning 404, only for client to swap it to a 500.
For now, the docs should reflect the current state of the REST API but we can open a new issue to ensure 404's are passed through to client REST API responses.
There was a problem hiding this comment.
Nice, yea I'm thinking long term here. Also might be a cool way into more monitoring of our APIs – e.g. monitoring can tell us when we return 500, we can work toward no 500s (maybe in API paths that we own).
brucealdridge
approved these changes
Jan 25, 2024
Jinksi
dismissed
shendy-a8c’s stale review
Suggestions have been applied – relevant fields have been removed from the docs.
Jinksi
deleted the
fix/7847-update-deposits-api-docs-estimated-deposits-rm
branch
|
Docs deployed to https://automattic.github.io/woocommerce-payments/#deposits, workflow https://github.com/Automattic/woocommerce-payments/actions/runs/7661809270 was automatically triggered. |
Jinksi
added a commit
that referenced
this pull request
Jan 30, 2024
Co-authored-by: Shendy <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #7847
Changes proposed in this Pull Request
This PR updates the deposits REST API documentation to reflect the "decoupled deposits" changes deployed on Jan 23 2024 in server PR 4500.
Note
I've uploaded a preview of this PR's version of the docs here.
GET deposits/overview-alldeposit.next_scheduledis no longer returned.balance.pending[].deposits_countis no longer returned.balance.instant[].transaction_idsis no longer returned.GET deposits/overviewnext_depositis no longer returned.balance.pending.deposits_countis no longer returned.instant_balance.transaction_idsis no longer returned.GET depositsestimateddeposits are no longer returned.status_isandstatus_not_isfilter params no longer filter byestimated.GET deposits/summaryestimateddeposits are no longer returned.status_isandstatus_not_isfilter params no longer filter byestimated.GET deposits/(?P<deposit_id>\w+)estimateddeposits are no longer returned, will return status404in this case.POST depositstransaction_idsproperty is deprecated.currencyproperty.POST deposits/downloadestimateddeposits are no longer included in CSV export.status_isandstatus_not_isfilter params no longer filter byestimated.See project thread: paJDYF-aY6-p2
See discussion on removing estimated deposits: paJDYF-bD7-p2
Testing instructions
cd docs/rest-api./build.shto build the docs via a Docker container.npx serve buildto serve the built HTML atlocalhost:3000npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge