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.

Sign up for GitHub

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

Jump to bottom

Remove deprecated deposits_status account key from Account Status (changed WCPay Server API response) #4710

Closed
brucealdridge opened this issue Sep 7, 2022 · 4 comments · Fixed by #8376 or #8755
Closed

Remove deprecated deposits_status account key from Account Status (changed WCPay Server API response) #4710

brucealdridge opened this issue Sep 7, 2022 · 4 comments · Fixed by #8376 or #8755
Assignees
@shendy-a8c
Labels
component: deposits Issues related to Deposits focus: deposits priority: low The issue/PR is low priority—not many people are affected or there’s a workaround, etc. type: technical debt This issue/PR represents/solves the technical debt of the project.
Milestone
Helix WooPayments...

Comments

@brucealdridge
Copy link
Contributor

brucealdridge commented Sep 7, 2022 •
edited by haszari

Descrption

deposits_status key in has been deprecated and should no longer be used. It has been replaced by deposits.status.
This is alongside a change to the WCPay server response also grouping deposit-based information under a deposits key.

woocommerce-payments/includes/class-wc-payments-account.php

Lines 195 to 196 in 6e76075

'deposits' => $account['deposits'] ?? [],
'depositsStatus' => $account['deposits']['status'] ?? $account['deposits_status'] ?? '',

@brucealdridge brucealdridge added status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. component: deposits Issues related to Deposits type: technical debt This issue/PR represents/solves the technical debt of the project. labels Sep 7, 2022
@RadoslavGeorgiev RadoslavGeorgiev added the category: projects For any issues which are part of any project, including bugs, enhancements, etc. label Sep 9, 2022
@brucealdridge brucealdridge removed the status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. label Sep 14, 2022
@haszari
Copy link
Contributor

haszari commented Feb 14, 2023

NB: This issue is currently blocked due to downstream dependencies

@brucealdridge is it still blocked? If so can you add details of the blocker inline so this issue is self-contained.

I'm assuming there's no blocker and we can handle this as maintenance (and we should prioritise) – let me know your thoughts 🙌

@haszari haszari removed the category: projects For any issues which are part of any project, including bugs, enhancements, etc. label Feb 14, 2023
@haszari haszari changed the title Remove deprecated deposits_status account key from Account Status data Remove deprecated deposits_status account key from Account Status api response Feb 20, 2023
@haszari haszari changed the title Remove deprecated deposits_status account key from Account Status api response Remove deprecated deposits_status account key from Account Status (changed WCPay Server API response) Feb 20, 2023
@brucealdridge
Copy link
Contributor Author

@brucealdridge is it still blocked? If so can you add details of the blocker inline so this issue is self-contained.

Pre-deploy checks originally caught this issue and these are somewhat hidden (private) from the normal flows and checks (unit tests, E2E.) It was just one of these tests that had hardcoded expectations around server responses. These have now been updated.

So the blocker has been removed and we should be 👍 to proceed on this.

@haszari
Copy link
Contributor

haszari commented Feb 20, 2023

Pre-deploy checks originally caught this issue and these are somewhat hidden (private) from the normal flows and checks (unit tests, E2E.)

Gotcha, thanks for clarifying – this was blocked by a pre-deploy integration test (tumblr something?) that is not in PR checks, and that breakage has been fixed.

@haszari haszari added the priority: medium The issue/PR is medium priority—non-critical functionality loss, minimal effect on usability label Dec 17, 2023
@souravdebnath1986 souravdebnath1986 added priority: low The issue/PR is low priority—not many people are affected or there’s a workaround, etc. and removed priority: medium The issue/PR is medium priority—non-critical functionality loss, minimal effect on usability labels Feb 15, 2024
@naman03malhotra naman03malhotra self-assigned this Mar 13, 2024
@naman03malhotra naman03malhotra mentioned this issue Mar 13, 2024
Merged
6 tasks
@shendy-a8c shendy-a8c mentioned this issue Mar 21, 2024
Merged
6 tasks
@shendy-a8c shendy-a8c reopened this Mar 21, 2024
@shendy-a8c
Copy link
Contributor

shendy-a8c commented Mar 21, 2024 •
edited

Because this issue's PR, #8376, broke an integration test (read context here p1710976497541019-slack-C02BW3Z8SHK), I created a revert PR #8432.

We need to re-apply the removal of depositsStatus key once the test is fixed, so I reopened this issue.

@shendy-a8c shendy-a8c mentioned this issue May 2, 2024
Merged
6 tasks
@shendy-a8c shendy-a8c self-assigned this May 2, 2024
@Jinksi Jinksi added this to the Helix WooPayments 7.7 milestone May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shendy-a8c shendy-a8c

Labels
component: deposits Issues related to Deposits focus: deposits priority: low The issue/PR is low priority—not many people are affected or there’s a workaround, etc. type: technical debt This issue/PR represents/solves the technical debt of the project.
Projects
None yet
Milestone
Helix WooPayments 7.7
7 participants
@brucealdridge @Jinksi @haszari @RadoslavGeorgiev @naman03malhotra @shendy-a8c @souravdebnath1986