Publicize: set connection `done` state optimistically.

[retrofox]

Branched off from ~#21208~ and #21231

This PR populates updates the post metadata to mark those connections used to share the post as done once the post is published, without waiting for the final result of the async process. This is an optimistic approach to the final result of the post-sharing action. Take a look at the issue description to get the idea :-D

Fixes https://github.com/Automattic/jetpack/issues/21233

https://user-images.githubusercontent.com/77539/136374335-ac548222-7b20-4aa3-83a7-63bbee58bc6a.mov

Changes proposed in this Pull Request:

• Publicize: set connection done state optimistically.

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

• Create a new post
• Add some connections
• Add content to the post
• Publish
• Confirm the done state is updated once the post publishes
• Confirm that it shows the same state after a hard refresh

• Take a look at the store to see how the done and toggleable properties are set as true in the post meta.

https://user-images.githubusercontent.com/77539/135283248-7c2b2342-8250-44a3-be3e-99d46c48d3db.mov

[github-actions[bot]]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• :white_check_mark: Include a description of your PR changes.
  
• :warning: All commits were linted before commit.
  
• :white_check_mark: Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• :white_check_mark: Add testing instructions.
  
• :white_check_mark: Specify whether this PR includes any changes to data or privacy.
  
• :white_check_mark: Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation :robot:

---

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

---

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped. Then, add the "[Status] Needs Team review" label and ask someone from your team review the code. Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

---

Jetpack plugin:

• Next scheduled release: November 2, 2021.
• Scheduled code freeze: October 25, 2021.

[pablinos]

I think the change we're making to done makes the post dirty, so if I publish a post and then click to leave the editor I get the warning about unsaved changes:

[retrofox]

I think the change we're making to done makes the post dirty, so if I publish a post and then click to leave the editor I get the warning about unsaved changes:

In theory, that shouldn't be generated by this approach, because although we do change the post metadata, which introduces changes to save, it happens just before publishing the post. That's the goal of the usePostJustBeforePublish() hook.

The experience that you share here is something that happens constantly to me, too, even in production.

[retrofox]

If I reload the editor after publishing the post, then the networks become disabled. That's sort of good, as it can't be shared, but I'm not sure if it's intended.

This is not introduced by this PR if I'm not wrong. You can confirm it just by testing with master branch. I dove on it and I think it's because $all_done is true for Jetpack sites, here, , once the post shares independently of how many connections were used.

[pablinos]

If I reload the editor after publishing the post, then the networks become disabled. That's sort of good, as it can't be shared, but I'm not sure if it's intended.

This is not introduced by this PR if I'm not wrong. You can confirm it just by testing with master branch. I dove on it and I think it's because $all_done is true for Jetpack sites, here, , once the post shares independently of how many connections were used.

That definitely sounds like something that would be good to fix, but not in this PR of course! As we introduce Republicize, I think the concept of 'done' isn't needed, or not in this binary way.

[pablinos]

In theory, that shouldn't be generated by this approach, because although we do change the post metadata, which introduces changes to save, it happens just before publishing the post. That's the goal of the usePostJustBeforePublish() hook.

Yes, I was thinking about that, but we react as the point that isPublishing changes to true. At that point the AJAX request has been initiated, and so any changes we make will not be included.

The experience that you share here is something that happens constantly to me, too, even in production.

That's true, but it would be good not to add to the problem if we can 😄

[retrofox]

In theory, that shouldn't be generated by this approach, because although we do change the post metadata, which introduces changes to save, it happens just before publishing the post. That's the goal of the usePostJustBeforePublish() hook.

Yes, I was thinking about that, but we react as the point that isPublishing changes to true. At that point the AJAX request has been initiated, and so any changes we make will not be included.

Then it means the hook doesn't work as expected

The experience that you share here is something that happens constantly to me, too, even in production.

That's true, but it would be good not to add to the problem if we can 😄

I'm not saying we should ignore a possible issue that we may be adding but the issue is being generated by other stuff.

[jeherve]

Closing this for now because of the lack of activity on this. We can always reopen in the future if needed.