Remove Gdoc Preview autoupdating, fix lightning publishing #2766
Conversation
adminSiteClient/gdocsDeploy.ts
Outdated
| const prevOmitted = omit( | ||
| { ...prevGdoc, tags: nextGdoc.tags?.map((tag) => JSON.stringify(tag)) }, | ||
| keysToOmit | ||
| ) | ||
| const nextOmitted = omit( | ||
| { ...nextGdoc, tags: prevGdoc.tags?.map((tag) => JSON.stringify(tag)) }, | ||
| keysToOmit | ||
| ) |
There was a problem hiding this comment.
prevOmitted should use prevGdoc.tags (and vice-versa for nextOmitted)
This swap ends up correcting another swap in another part of the codebase, so this is a case of (-) x (-) = (+), but we should still address the inital swap, as it creates other issues.
Here is the scenario that leads to it:
- load preview
- update gdoc body
- refresh preview ("⚡️ Republishing" button appears)
- change tags
- hit "⚡️ Republishing"
The first surface issue is that a standard publishing triggered. The front-end doesn't know of the change of tags, and still sees this change as a lightning update. This leads to a discrepancy between the visual promise of a ⚡️ update and the effective non-⚡️ update that follows.
The second issue is that tag changes are not persisted if they happen during a preview session. Going back to the previous timeline:
- load preview -> e.g. loads tags A and B
- update gdoc body
- refresh preview ("⚡️ Republishing" button appears)
- change tags -> e.g. removes tag B.
- hit "⚡️ Republishing"
The change carried out in 4 has not been propagated to the front-end, so the call to apiRouter.put("/gdocs/:id") is made with a mix of nextGdoc's latest properties and the tags that were set on page load. Upon hitting "⚡️ Republishing", the previous set of tags (A and B) is then reapplied. Meanwhile, when prevGdoc is set in that same API call, it is set to the version currently in the DB (conceptually, nextGdoc's tags), hence the swap mentioned earlier.
There was a problem hiding this comment.
Nice catch! Those variables being swapped was just a goof, definitely not intentional. But I'm glad it prompted you pointing out this potential footgun in either case.
I've updated the GdocsStore.update method from the preview page to omit the tags property in the JSON it posts. This means that the tags in the DB won't get changed in any way from publishing a gdoc.
I think this means that technically we should trigger a bake whenever a Gdoc's tags are updated, but we don't do that for charts either, and I think we could try it out this way first (with manual+incidental bake triggers) and see if that works well enough.
There was a problem hiding this comment.
yes, I agree.
Tested and both issues are gone, nice!
ikesau
force-pushed
the
fix-unpublishing
branch
from
243ee13 to
c7f4f26
Compare
Autoupdating was causing lots of headaches when multiple clients were previewing the same document. The effort in fixing that class of issue seems to outweigh the marginal benefit of autoupdating, so I'm removing the autoupdating functionality altogether.
I also noticed that lightning updates hadn't been working properly for ages, so I've updated that code to fix the thing that was breaking it (Date comparisons) and made the declaration of light props more explicit and tied to our type validation so that it'll be harder to miss in the future.