Fix replacing site url in DB after pulling site - #992
Conversation
86c76a5 to
a7f5443
Compare
| return; | ||
| } | ||
|
|
||
| console.groupCollapsed( 'Sync Pull' ); |
There was a problem hiding this comment.
If you want to clean up the logging, could you consider opening a separate PR for that?
| if ( userChoice === 1 ) { | ||
| console.log( 'User cancelled pull operation' ); | ||
| const isCanceledByUser = userChoice === 1; | ||
| if ( isCanceledByUser ) { |
There was a problem hiding this comment.
Do we need this variable if it's used only once?
There was a problem hiding this comment.
I just wanted to avoid "magic" number 1, since at a glance I thought that 1 means kinda true, as a confirmation to continue pulling, but actually it's canceling.
So to keep readability, we need either to add comment about 1 or create readable and simple variable. And the good variable name is much better than any comment.
| const downloadUrl = hasBackupCompleted ? response.download_url : null; | ||
|
|
||
| if ( hasBackupCompleted && downloadUrl ) { | ||
| if ( downloadUrl ) { |
There was a problem hiding this comment.
What's the context of this removal?
There was a problem hiding this comment.
The above line has const downloadUrl = hasBackupCompleted ? response.download_url : null; - downloadUrl is available only if hasBackupCompleted is true. So it's duplication of the condition.
| const urlWithoutProtocol = oldUrl.replace( /^https?:\/\//, '' ); | ||
|
|
||
| const oldUrlVariants = [ `http://${ urlWithoutProtocol }`, `https://${ urlWithoutProtocol }` ]; | ||
| const oldUrlVariants = [ |
|
@nightnei, it worked as expected for me - IMG had a local URL after the pull. However, I couldn't make the video load from wp-admin, and it always had the videopress URL. How could I test this part? |
@wojtekn hm, maybe it's the same case as my assumption here? p1741020425670679/1740755830.254999-slack-C07Q6R5HW3T Could you please check and ensure that you are adding a newly uploaded image as a poster, instead of some image in your library. If it's not the case, then I will think more to try to understand why it works for you differently. Or, as an option, could you please share that website with me (nei.css@gmail.com) with Admin access and I will take a look. |
a7f5443 to
adc57cb
Compare
|
@wojtekn I revert changes for console.logs etc, I will create separate PR for them, but anyway could you please take a look at my answers regarding your feedback. |
Could you show a screenshot that illustrates what is a poster in Elementor? |
|
@nightnei I've reproduced the case with the video's poster URL not being replaced and confirmed that PR fixes the issue. |
2 participants
Related issues
Proposed Changes
When user pull or puh a site to/from Studio - all links should reflect the actual domain, all of them should be
localhost:<port>if it was pulling to Studio or actual domain on dotcom if it was pushing.This PR handles improving pulling process. We had it before, but I found not covered cases, so this PR fixes them
Regarding pushing - we don't handle it at all. I was thinking about it end ended up that it's better to do it in VaultPress side (details - p1740755830254999-slack-C07Q6R5HW3T).
Examples of issues covered in this PR:
Testing Instructions