Woo installer: use new transfer endpoints in Calypso #58709
Conversation
|
Link to Calypso live: https://calypso.live?image=registry.a8c.com/calypso/app:build-22180 |
|
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
lsl
force-pushed
the
add/atomic-transfer-with-plugin
branch
2 times, most recently
from
0dbff09
to
ceb6e71
Compare
| method: 'POST', | ||
| path: `/sites/${ action.siteId }/atomic/transfers/`, | ||
| body: { | ||
| software_set: action.softwareSet, |
There was a problem hiding this comment.
This parameter should be optional, we're going to need to add plugin, theme, plugin file, theme file, and later options along with it.
There was a problem hiding this comment.
Made this optional in the action so just needs to account for that here.
|
@allilevine had to rebase both branches to make the reset work. |
| blog_id: number; | ||
| software_set: Record< string, { path: string; state: string } >; | ||
| applied: boolean; | ||
| error?: string; |
There was a problem hiding this comment.
Don't really know how to do the error handling properly. Thoughts?
This is what ends up happening in the "front end" and I think its pretty brittle. Would just creating an error selector make sense? We probably want to fix up the object format anyway (fromApi or something like that does this I think?)
|
@allilevine @lsl Is there anything in client/state/atomic-transfer that can be reused or built on? |
|
That looks like v1 endpoint code so I don't think so, what did you have in mind? |
|
Happy path is stable now, testing using #58848, error handling might need an update but I think we should merge this as is to simplify / clarify whats going on in 58848. |
lsl
added
the
[Status] Needs Review
- Moves datalayer to atomic/* pathing to match endpoint routing - Mirrored the approach in state - Adds state selectors/reducers - todo: Replace transfer status with transfer as an object
allilevine
force-pushed
the
add/atomic-transfer-with-plugin
branch
from
06e644c
to
f74e34e
Compare
You're right, it is :( |
There was a problem hiding this comment.
I've tested the approach using the #58848 branch. It works as expected, keep in mind that there are still some patches to go.
Testing instruction is described in the PR.
Probably it deserves a couple of additional eyes, too.
github-actions
bot
removed
the
[Status] Needs Review
|
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/7039878 Thank you @allilevine for including a screenshot in the description! This is really helpful for our translators. |
|
Translation for this Pull Request has now been finished. |
Changes proposed in this Pull Request
Testing instructions
Error screenshot:
Related to #58384