Optimistic updates via pending sats in item context #1229
Conversation
ekzyis
changed the title
ekzyis
force-pushed
the
zap-pending-sats-context
branch
5 times, most recently
from
99c7f50 to
e29070e
Compare
We already handle undoing pending sats by wrapping the payment+mutation with try/finally.
If a comment was root and it was zapped, the pending sats contributed to the sats shown in <CommentsHeader>. This was caused by <CommentsHeader> accessing the root item context for all comments, even for the root comment. So even if the root comment was zapped, the pending sats contributed to the sats for the comment section. This wasn't the case for posts since their item context was above the context used by <CommentsHeader>. This was fixed by moving <ItemProviderContext> down into <Comments> and <Item> instead of declaring it at <ItemFull> which wraps the root item and all comments.
ekzyis
force-pushed
the
zap-pending-sats-context
branch
from
8c23df3 to
ee29401
Compare
|
As I review, and I'm just curious because it would've been my first approach, did you try using |
Mhh, I think it was one of my first approaches when I started implementing optimistic UX (so not in this PR but like weeks ago) but I must have had some problems with (undoing) it |
|
This works well. The only thing I'm noticing in QA is that for a brief moment, it appears as if I zapped double. e.g. If I zap 100 sats, it'll briefly show 200 sats, then 100. I'll see if it's easy to fix, but it isn't a huge deal. I still need to QA attached wallets too. |
|
Ah, I think this happens because |
Ah yes, the optimistic context is undone in the This would change the semantics of the item context though but we will remove this code anyway1 so ... Footnotes
|
|
I committed my changes to remove the doubling. Please QA it again yourself. Then I can deploy tomorrow if that works for you. |
|
LGTM. Regarding c4b52dd: I created a PR where one can disable |
* wip backend optimism * another inch * make action state transitions only happen once * another inch * almost ready for testing * use interactive txs * another inch * ready for basic testing * lint fix * inches * wip item update * get item update to work * donate and downzap * inchy inch * fix territory paid actions * wip usePaidMutation * usePaidMutation error handling * PENDING_HELD and HELD transitions, gql paidAction return types * mostly working pessimism * make sure invoice field is present in optimisticResponse * inches * show optimistic values to current me * first pass at notifications and payment status reporting * fix migration to have withdrawal hash * reverse optimism on payment failure * Revert "Optimistic updates via pending sats in item context (#1229)" This reverts commit 93713b3. * add onCompleted to usePaidMutation * onPaid and onPayError for new comments * use 'IS DISTINCT FROM' for NULL invoiceActionState columns * make usePaidMutation easier to read * enhance invoice qr * prevent actions on unpaid items * allow navigation to action's invoice * retry create item * start edit window after item is paid for * fix ux of retries from notifications * refine retries * fix optimistic downzaps * remember item updates can't be retried * store reference to action item in invoice * remove invoice modal layout shift * fix destructuring * fix zap undos * make sure ItemAct is paid in aggregate queries * dont toast on long press zap undo * fix delete and remindme bots * optimistic poll votes with retries * fix retry notifications and invoice item context * fix pessimisitic typo * item mentions and mention notifications * dont show payment retry on item popover * make bios work * refactor paidAction transitions * remove stray console.log * restore docker compose nwc settings * add new todos * persist qr modal on post submission + unify item form submission * fix post edit threshold * make bounty payments work * make job posting work * remove more store procedure usage ... document serialization concerns * dont use dynamic imports for paid action modules * inline comment denormalization * create item starts with median votes * fix potential of serialization anomalies in zaps * dont trigger notification indicator on successful paid action invoices * ignore invoiceId on territory actions and add optimistic concurrency control * begin docs for paid actions * better error toasts and fix apollo cache warnings * small documentation enhancements * improve paid action docs * optimistic concurrency control for territory updates * use satsToMsats and msatsToSats helpers * explictly type raw query template parameters * improve consistency of nested relation names * complete paid action docs * useEffect for canEdit on payment * make sure invoiceId is provided when required * don't return null when expecting array * remove buy credits * move verifyPayment to paidAction * fix comments invoicePaidAt time zone * close nwc connections once * grouped logs for paid actions * stop invoiceWaitUntilPaid if not attempting to pay * allow actionState to transition directly from HELD to PAID * make paid mutation wait until pessimistic are fully paid * change button text when form submits/pays * pulsing form submit button * ignore me in notification indicator for territory subscription * filter unpaid items from more queries * fix donation stike timing * fix pending poll vote * fix recent item notifcation padding * no default form submitting button text * don't show paying on submit button on free edits * fix territory autorenew with fee credits * reorg readme * allow jobs to be editted forever * fix image uploads * more filter fixes for aggregate views * finalize paid action invoice expirations * remove unnecessary async * keep clientside cache normal/consistent * add more detail to paid action doc * improve paid action table * remove actionType guard * fix top territories * typo api/paidAction/README.md Co-authored-by: ekzyis <[email protected]> * typo components/use-paid-mutation.js Co-authored-by: ekzyis <[email protected]> * Apply suggestions from code review Co-authored-by: ekzyis <[email protected]> * encorporate ek feeback * more ek suggestions * fix 'cost to post' hover on items * Apply suggestions from code review Co-authored-by: ekzyis <[email protected]> --------- Co-authored-by: ekzyis <[email protected]>
Description
Since we removed optimistic updates in #1220 because the implementation broke replies, there is no immediate visual feedback if you zap from a non-custodial wallet. This is bad UX since it looks like zaps don't work since nothing happens. However, the payment is just pending.
This PR fixes this by implementing optimistic updates via a new context
ItemContext(maybe not the best name). This context keeps track of pending sats. Since it uses the Context API, this is completely separate of the Apollo cache and thus does not interfere with any mutation. All this does is to add pending sats to the item context, remove them after the mutation (if it failed or not) and hook into the context in the components that need to be aware of pending sats.To update
Item.commentSatsfor all ancestors on a zap,setPendingSatsalso callssetPendingCommentSatsof the parentItemContextcontext. This is done recursively up to the root item.This is just meant as a temporary fix until #1195 is released.
TODO:
Item.commentSatsoptimistic update for bounty paymentsabandoned because would require too many changes for a temporary fixChecklist
Are your changes backwards compatible? Please answer below:
Yes
Did you QA this? Could we deploy this straight to production? Please answer below:
Yes
For frontend changes: Tested on mobile? Please answer below:
Did you introduce any new environment variables? If so, call them out explicitly here: