Added loading and disabled states for async user actions across the app.#484
Merged
roshankumar0036singh merged 1 commit intoMay 31, 2026
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds per-action loading flags and early-return guards across components and screens, disables relevant UI during async work, shows inline ActivityIndicators and spinner+label loading content, and ensures flags are cleared in finally blocks. ChangesAsync Operation Guards and Loading UI Patterns
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/src/components/EventCard.jsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. app/src/components/FeedbackModal.jsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. app/src/components/PremiumInput.jsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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 |
Owner
|
fix the issue that is flagged by sonar |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/components/PremiumInput.js (1)
53-64:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake
disabledapply to the password toggle as well.
editable={false}stops typing, but password inputs still expose a live eye button below, so the control is only partially disabled.Suggested fix
{secureTextEntry && ( <TouchableOpacity onPress={() => setIsPasswordVisible(!isPasswordVisible)} style={styles.eyeIcon} + disabled={disabled} accessibilityRole="button" accessibilityLabel={isPasswordVisible ? 'Hide password' : 'Show password'} accessibilityHint="Toggles password visibility" - accessibilityState={{ checked: isPasswordVisible }} + accessibilityState={{ checked: isPasswordVisible, disabled }} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/PremiumInput.js` around lines 53 - 64, The password visibility toggle in PremiumInput.js is still interactive when the field is disabled; update the password-toggle control (the eye button handler and its Touchable/Pressable) to respect the disabled prop by either not rendering the toggle or rendering it in a non-interactive state when disabled is true, and ensure its accessibilityState includes disabled and its onPress is no-op (or disabled={true}) so users cannot toggle visibility; update any references to secureTextEntry/isPasswordVisible handling accordingly so the toggle only functions when disabled is false.app/src/screens/EventDetail.js (1)
904-915:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the certificate action locked until the web print call actually executes.
On web,
printWindow.print()is deferred viasetTimeout, butcertificateDownloadLoadingis cleared infinallyright after scheduling (lines 940-941), so the FAB can re-enable before the print dialog opens and allow duplicate popup/print attempts.Suggested fix
if (Platform.OS === 'web') { // Open separate window for reliable printing on mobile web const printWindow = window.open('', '_blank'); if (printWindow) { printWindow.document.write(html); printWindow.document.close(); // Allow styles and fonts to load - setTimeout(() => { - printWindow.focus(); - printWindow.print(); - }, 500); + await new Promise(resolve => { + setTimeout(() => { + printWindow.focus(); + printWindow.print(); + resolve(); + }, 500); + }); } else { Alert.alert('Blocked', 'Please allow pop-ups to download the certificate.'); }Also, the “Send Certificates” tap guard (
handleSendCertificates) relies on state (sendingCertificates), while the in-flight flag is set insidesendCertificates; rapid double-taps before re-render could still trigger duplicate sends—set the in-flight flag in the tap handler (or use a ref) before callingsendCertificates().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/screens/EventDetail.js` around lines 904 - 915, The web print flow clears certificateDownloadLoading too early: keep the FAB locked until the print actually runs by moving the certificateDownloadLoading state clear into the printWindow.print() callback path — i.e., only call setCertificateDownloadLoading(false) after printWindow.print() completes (or after focus/print promise resolution in the setTimeout), and clear it on both success and error paths (and when printWindow is null). Also prevent double-send in the “Send Certificates” flow by setting the in-flight flag before invoking sendCertificates: set sendingCertificates (or use a ref) in handleSendCertificates immediately on tap and return early if already set, then call sendCertificates(); ensure sendCertificates clears the flag on completion/errors. Reference symbols: certificateDownloadLoading, printWindow.print(), setTimeout callback, handleSendCertificates, sendingCertificates, sendCertificates.
🧹 Nitpick comments (4)
app/src/screens/EventRegistrationFormScreen.js (1)
404-409: 💤 Low valueConsider standardizing loading content gap.
This
submitLoadingContentstyle usesgap: 10, whileFeedbackModal.jsusesgap: 8for the same layout pattern. Both work, but standardizing the value across the app would improve consistency.This is cosmetic and can be deferred.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/screens/EventRegistrationFormScreen.js` around lines 404 - 409, The submitLoadingContent style in EventRegistrationFormScreen.js uses gap: 10 which differs from the gap: 8 used for the same loading layout in FeedbackModal.js; update submitLoadingContent to use gap: 8 for consistent spacing (or alternatively adjust both to a single chosen value) so the layout across components like submitLoadingContent and the FeedbackModal loading style is standardized.app/src/components/FeedbackModal.js (2)
217-220: 💤 Low valueCallback guard is redundant with
readonlyprop.Both rating inputs combine
readonly={loading}with anif (!loading)guard insideonFinishRating. Thereadonlyprop already prevents user interaction, making the callback guard defensive redundancy.While not harmful, you can simplify by relying on
readonlyalone or keeping only the callback guard. The current implementation works correctly.Also applies to: 246-249
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/FeedbackModal.js` around lines 217 - 220, The onFinishRating callbacks in FeedbackModal are redundantly checking loading inside the handler while also passing readonly={loading} to the rating inputs; remove the internal guard and rely on readonly instead (i.e., update the onFinishRating handlers for the event rating and speaker rating—functions referencing setEventRating and setSpeakerRating—to simply call setEventRating(rating) / setSpeakerRating(rating) without the if (!loading) check), or conversely remove readonly and keep the guard—pick one approach and apply consistently to both occurrences.
82-89: ⚡ Quick winConsider providing visual feedback that the modal is locked during submission.
The
onRequestClosehandler now blocks modal dismissal whileloadingis true, which prevents accidental cancellation. However, users attempting to close the modal via hardware back button (Android) or escape key will receive no feedback explaining why the action is blocked.Consider adding a brief toast or alert when users attempt to close during loading, or show a persistent "Submitting..." indicator in the modal header.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/FeedbackModal.js` around lines 82 - 89, The onRequestClose handler currently swallows close attempts when loading is true without feedback; update the Modal behavior so that when loading is true the handler both prevents closing and gives immediate user feedback—e.g., call a platform toast/alert (ToastAndroid.show or Alert.alert / a cross-platform Toast component) inside the onRequestClose arrow function when loading is true, and additionally ensure the FeedbackModal UI (component FeedbackModal) shows a persistent "Submitting..." indicator in the modal header while loading; reference the Modal prop onRequestClose, the loading boolean, and the onClose callback to implement this behavior.app/src/screens/CreateEvent.js (1)
100-102: ⚖️ Poor tradeoffDocument the
allowWhileSubmittingbypass and consider concurrent auth risks.The
allowWhileSubmittingoption allows Meet link generation during form submission, but this creates two possible flows:
- User clicks "Auto-Generate" → blocked if loading
- Submit auto-generates → proceeds even if loading
While the intent is clear (prevent user-initiated concurrency but allow submit-driven generation), this could create confusing states if the Google auth flow (
promptAsync) is invoked concurrently or if multiple token requests overlap.Consider adding a comment explaining why this bypass exists and verify that the Google Calendar API handles concurrent or rapid-fire requests safely.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/screens/CreateEvent.js` around lines 100 - 102, The handleGenerateMeetLink function's allowWhileSubmitting bypass permits meet-link generation during form submission and risks concurrent Google auth/token requests (e.g., promptAsync) causing confusing states; add an inline comment above handleGenerateMeetLink explaining the intended bypass behavior and rationale (prevent user-initiated generation while allowing submit-driven generation), and then add a lightweight safety guard: ensure promptAsync/token requests are serialized or deduplicated (e.g., short-lived in-memory "authInProgress" flag) so concurrent calls from submit and user actions cannot overlap; also add a TODO note to verify Google Calendar API idempotency/safety for rapid requests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/screens/AttendanceDashboard.js`:
- Around line 192-197: Consolidate the two separate try/finally blocks into a
single try/catch/finally around the offline sync work so errors from
getOfflineCheckInCount are caught and do not propagate; in the catch log or
handle the error and in the finally always call setSyncingOffline(false). Also
guard state updates (setPendingOfflineCount and setSyncingOffline) with a
mounted check (e.g., an isMounted ref or AbortController) so the flag is cleared
only when the component is still mounted; update the code paths around
getOfflineCheckInCount, setPendingOfflineCount, and setSyncingOffline
accordingly.
In `@app/src/screens/ClubProfileScreen.js`:
- Line 205: The optimistic update flips isFollowing directly then attempts to
revert by flipping again, which mis-restores state; capture the original value
before the optimistic change (e.g., const prevIsFollowing = isFollowing) in the
handler that performs the optimistic update (where setIsFollowing(!isFollowing)
is called), use setIsFollowing(!prevIsFollowing) for the optimistic change or
simply setIsFollowing(!prevIsFollowing) then, in the catch/error path where
setIsFollowing(!isFollowing) is currently used, call
setIsFollowing(prevIsFollowing) to reliably restore the original state; update
the code around the setIsFollowing and error handler references so the error
branch uses the captured prevIsFollowing rather than the stale isFollowing
closure.
In `@app/src/screens/EventDetail.js`:
- Around line 971-975: The handler handleSendCertificates has a race where it
reads sendingCertificates but setSendingCertificates(true) happens inside
sendCertificates, allowing rapid double-taps to start multiple flows; change the
latch to be acquired synchronously in the handler (either
setSendingCertificates(true) immediately before dispatching or use a ref like
sendingCertificatesRef.current = true in handleSendCertificates) and release it
in a finally block after awaiting sendCertificates (or ensure sendCertificates'
finally clears it). Update references to sendingCertificates,
setSendingCertificates, handleSendCertificates and sendCertificates so the
in-flight guard is set synchronously in the handler and cleared reliably to
prevent double-dispatch of sendBulkCertificates.
In `@app/src/screens/MyEventsScreen.js`:
- Around line 76-114: The current guard in handleDelete prevents any deletion
while any deletion is in progress, but the UI only disables the specific event's
delete button (renderItem uses deletingEventId === item.id), causing clicks on
other items to do nothing; change the guard in handleDelete from if
(deletingEventId) return; to if (deletingEventId === eventId) return; so only
duplicate deletes for the same event are blocked and the UI and logic remain
consistent (symbols: handleDelete, deletingEventId, eventId, renderItem,
item.id).
---
Outside diff comments:
In `@app/src/components/PremiumInput.js`:
- Around line 53-64: The password visibility toggle in PremiumInput.js is still
interactive when the field is disabled; update the password-toggle control (the
eye button handler and its Touchable/Pressable) to respect the disabled prop by
either not rendering the toggle or rendering it in a non-interactive state when
disabled is true, and ensure its accessibilityState includes disabled and its
onPress is no-op (or disabled={true}) so users cannot toggle visibility; update
any references to secureTextEntry/isPasswordVisible handling accordingly so the
toggle only functions when disabled is false.
In `@app/src/screens/EventDetail.js`:
- Around line 904-915: The web print flow clears certificateDownloadLoading too
early: keep the FAB locked until the print actually runs by moving the
certificateDownloadLoading state clear into the printWindow.print() callback
path — i.e., only call setCertificateDownloadLoading(false) after
printWindow.print() completes (or after focus/print promise resolution in the
setTimeout), and clear it on both success and error paths (and when printWindow
is null). Also prevent double-send in the “Send Certificates” flow by setting
the in-flight flag before invoking sendCertificates: set sendingCertificates (or
use a ref) in handleSendCertificates immediately on tap and return early if
already set, then call sendCertificates(); ensure sendCertificates clears the
flag on completion/errors. Reference symbols: certificateDownloadLoading,
printWindow.print(), setTimeout callback, handleSendCertificates,
sendingCertificates, sendCertificates.
---
Nitpick comments:
In `@app/src/components/FeedbackModal.js`:
- Around line 217-220: The onFinishRating callbacks in FeedbackModal are
redundantly checking loading inside the handler while also passing
readonly={loading} to the rating inputs; remove the internal guard and rely on
readonly instead (i.e., update the onFinishRating handlers for the event rating
and speaker rating—functions referencing setEventRating and setSpeakerRating—to
simply call setEventRating(rating) / setSpeakerRating(rating) without the if
(!loading) check), or conversely remove readonly and keep the guard—pick one
approach and apply consistently to both occurrences.
- Around line 82-89: The onRequestClose handler currently swallows close
attempts when loading is true without feedback; update the Modal behavior so
that when loading is true the handler both prevents closing and gives immediate
user feedback—e.g., call a platform toast/alert (ToastAndroid.show or
Alert.alert / a cross-platform Toast component) inside the onRequestClose arrow
function when loading is true, and additionally ensure the FeedbackModal UI
(component FeedbackModal) shows a persistent "Submitting..." indicator in the
modal header while loading; reference the Modal prop onRequestClose, the loading
boolean, and the onClose callback to implement this behavior.
In `@app/src/screens/CreateEvent.js`:
- Around line 100-102: The handleGenerateMeetLink function's
allowWhileSubmitting bypass permits meet-link generation during form submission
and risks concurrent Google auth/token requests (e.g., promptAsync) causing
confusing states; add an inline comment above handleGenerateMeetLink explaining
the intended bypass behavior and rationale (prevent user-initiated generation
while allowing submit-driven generation), and then add a lightweight safety
guard: ensure promptAsync/token requests are serialized or deduplicated (e.g.,
short-lived in-memory "authInProgress" flag) so concurrent calls from submit and
user actions cannot overlap; also add a TODO note to verify Google Calendar API
idempotency/safety for rapid requests.
In `@app/src/screens/EventRegistrationFormScreen.js`:
- Around line 404-409: The submitLoadingContent style in
EventRegistrationFormScreen.js uses gap: 10 which differs from the gap: 8 used
for the same loading layout in FeedbackModal.js; update submitLoadingContent to
use gap: 8 for consistent spacing (or alternatively adjust both to a single
chosen value) so the layout across components like submitLoadingContent and the
FeedbackModal loading style is standardized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eaf06a2a-9b22-471e-8fe8-fba54c0c80c3
📒 Files selected for processing (9)
app/src/components/EventCard.jsapp/src/components/FeedbackModal.jsapp/src/components/PremiumInput.jsapp/src/screens/AttendanceDashboard.jsapp/src/screens/ClubProfileScreen.jsapp/src/screens/CreateEvent.jsapp/src/screens/EventDetail.jsapp/src/screens/EventRegistrationFormScreen.jsapp/src/screens/MyEventsScreen.js
Richa-2005
force-pushed
the
feature/loading-states-async-operations
branch
from
dd0675d to
8d6a670
Compare
Owner
|
check the coderabbit sugestions |
Richa-2005
force-pushed
the
feature/loading-states-async-operations
branch
2 times, most recently
from
533a34c to
569efe9
Compare
Owner
|
@Richa-2005 resolev the conflcit carefullly for this one |
Contributor
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)
app/src/screens/CreateEvent.js (1)
697-708:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDisable marker drag interaction while loading, not only the state write.
Line 706 blocks
setCoordinates, but the marker is still draggable, which gives a false interactive affordance during submit. Setdraggable={!loading}(and optionally disable map gestures) so UI behavior matches the loading lock.Proposed fix
<Marker - draggable + draggable={!loading} coordinate={ coordinates || { latitude: 28.7041, longitude: 77.1025, } } onDragEnd={e => !loading && setCoordinates(e.nativeEvent.coordinate) } />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/screens/CreateEvent.js` around lines 697 - 708, The Marker currently remains draggable even when submission is loading, causing a misleading affordance; update the Marker props to set draggable={!loading} (instead of hardcoded draggable) and ensure onDragEnd still guards setCoordinates with the loading flag; also consider disabling map gestures on the surrounding MapView (e.g., set scrollEnabled/zoomEnabled/rotateEnabled/pitchEnabled to !loading) so the map and Marker both reflect the loading lock.
🧹 Nitpick comments (1)
app/src/screens/ClubProfileScreen.js (1)
177-242: Error-revert logic is now correct; consider an atomic write for backend consistency.The optimistic update and rollback via
previousFollowingcorrectly resolve the earlier double-flip bug, andfinallyreliably clears the loading flag.One residual concern: the follow/unfollow path issues multiple sequential
awaits (the two membership docs plus the countsetDoc). If a later write fails, the UI reverts cleanly but Firestore can be left in an inconsistent state (e.g.,followingdoc deleted while thefollowersdoc andfollowersCountare stale). AwriteBatchwould make all three writes atomic and remove the partial-failure window.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/screens/ClubProfileScreen.js` around lines 177 - 242, In toggleFollow replace the sequential await calls that touch myFollowingRef, clubFollowerRef and clubRef with a single Firestore writeBatch so the three mutations are atomic: build a writeBatch(), call batch.delete(myFollowingRef) / batch.set(myFollowingRef, ...) and batch.set(clubFollowerRef, ...) as appropriate, and update the club document via batch.set or batch.update using FieldValue.increment(1) / increment(-1) for followersCount instead of reading followersCount directly, then await batch.commit(); keep the optimistic UI and rollback logic unchanged but remove the partial-update risk by committing all three operations inside the batch before resolving success.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/screens/ClubProfileScreen.js`:
- Around line 35-36: The avatar URL currently injects club?.displayName raw
which breaks URLs and can produce name=undefined; update getClubAvatarUrl to
URL-encode the display name and avoid "undefined" by using a safe fallback (e.g.
const name = encodeURIComponent(club?.displayName ?? '') or similar) before
interpolating into the ui-avatars query string, while keeping the existing
club?.photoURL precedence; reference getClubAvatarUrl and club?.displayName when
making the change.
---
Outside diff comments:
In `@app/src/screens/CreateEvent.js`:
- Around line 697-708: The Marker currently remains draggable even when
submission is loading, causing a misleading affordance; update the Marker props
to set draggable={!loading} (instead of hardcoded draggable) and ensure
onDragEnd still guards setCoordinates with the loading flag; also consider
disabling map gestures on the surrounding MapView (e.g., set
scrollEnabled/zoomEnabled/rotateEnabled/pitchEnabled to !loading) so the map and
Marker both reflect the loading lock.
---
Nitpick comments:
In `@app/src/screens/ClubProfileScreen.js`:
- Around line 177-242: In toggleFollow replace the sequential await calls that
touch myFollowingRef, clubFollowerRef and clubRef with a single Firestore
writeBatch so the three mutations are atomic: build a writeBatch(), call
batch.delete(myFollowingRef) / batch.set(myFollowingRef, ...) and
batch.set(clubFollowerRef, ...) as appropriate, and update the club document via
batch.set or batch.update using FieldValue.increment(1) / increment(-1) for
followersCount instead of reading followersCount directly, then await
batch.commit(); keep the optimistic UI and rollback logic unchanged but remove
the partial-update risk by committing all three operations inside the batch
before resolving success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aeb9779f-fed8-4b17-afaa-5083e7e2a3fe
📒 Files selected for processing (9)
app/src/components/EventCard.jsapp/src/components/FeedbackModal.jsapp/src/components/PremiumInput.jsapp/src/screens/AttendanceDashboard.jsapp/src/screens/ClubProfileScreen.jsapp/src/screens/CreateEvent.jsapp/src/screens/EventDetail.jsapp/src/screens/EventRegistrationFormScreen.jsapp/src/screens/MyEventsScreen.js
🚧 Files skipped from review as they are similar to previous changes (5)
- app/src/screens/EventRegistrationFormScreen.js
- app/src/screens/AttendanceDashboard.js
- app/src/components/EventCard.js
- app/src/screens/MyEventsScreen.js
- app/src/components/FeedbackModal.js
Richa-2005
force-pushed
the
feature/loading-states-async-operations
branch
from
569efe9 to
7ce23b4
Compare
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/components/EventCard.js (2)
93-121:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate
lookingForBuddylocally to avoid stale switch state during async toggle.Line 322 uses a controlled
Switch, but Lines 93-121 only persist to Firestore and wait foronSnapshotto reflect the new value. Under latency, the toggle can visually revert/lag even though the action is in progress.Proposed fix
const handleToggleBuddy = async value => { if (buddyLoading) return; if (!user || !event?.id) return; + const previousValue = lookingForBuddy; try { setBuddyLoading(true); + setLookingForBuddy(value); // optimistic UI const participantRef = doc(db, 'events', event.id, 'participants', user.uid); await updateDoc(participantRef, { lookingForBuddy: value, }); @@ } catch (error) { console.error('Error updating buddy preference:', error); + setLookingForBuddy(previousValue); // rollback on failure Alert.alert('Error', 'Failed to update buddy preference.'); } finally { setBuddyLoading(false); } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/EventCard.js` around lines 93 - 121, The Switch visually lags because handleToggleBuddy only persists the change to Firestore; update the local state that the Switch is controlled by immediately (optimistic update) before calling updateDoc so the UI reflects the new value, then call setBuddyLoading(true), await updateDoc(participantRef, { lookingForBuddy: value }), and in the catch block revert that local state if the update fails; reference handleToggleBuddy, the controlled Switch state (e.g., localLookingForBuddy / setLocalLookingForBuddy), participantRef/updateDoc, and ensure setBuddyLoading(false) remains in finally.
159-170:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlways evict
profileRequestCacheafter promise resolution, even when user doc is missing.At Line 163, cache eviction only happens when
snap.exists()is true. For missing docs, the resolved promise stays inprofileRequestCacheforever, which can accumulate stale misses and prevent future re-fetch if the profile is later created.Proposed fix
profileRequestCache .get(event.ownerId) .then(snap => { + profileRequestCache.delete(event.ownerId); if (snap.exists()) { const data = snap.data(); profileCache.set(event.ownerId, { data, cachedAt: Date.now() }); - profileRequestCache.delete(event.ownerId); if (!cancelled) { setHostName(data.displayName || event.organization || 'Club Name'); } } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/EventCard.js` around lines 159 - 170, The promise resolution path in EventCard.js only deletes profileRequestCache for event.ownerId when snap.exists() is true, leaving misses cached forever; update the .then handler (the block that handles snap => { ... }) so that profileRequestCache.delete(event.ownerId) is always called regardless of whether snap.exists() is true (or move the eviction into a finally-like path), and ensure you still only call setHostName(data.displayName || event.organization || 'Club Name') when !cancelled and data exists; keep existing updates to profileCache and the catch block behavior intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/src/components/EventCard.js`:
- Around line 93-121: The Switch visually lags because handleToggleBuddy only
persists the change to Firestore; update the local state that the Switch is
controlled by immediately (optimistic update) before calling updateDoc so the UI
reflects the new value, then call setBuddyLoading(true), await
updateDoc(participantRef, { lookingForBuddy: value }), and in the catch block
revert that local state if the update fails; reference handleToggleBuddy, the
controlled Switch state (e.g., localLookingForBuddy / setLocalLookingForBuddy),
participantRef/updateDoc, and ensure setBuddyLoading(false) remains in finally.
- Around line 159-170: The promise resolution path in EventCard.js only deletes
profileRequestCache for event.ownerId when snap.exists() is true, leaving misses
cached forever; update the .then handler (the block that handles snap => { ...
}) so that profileRequestCache.delete(event.ownerId) is always called regardless
of whether snap.exists() is true (or move the eviction into a finally-like
path), and ensure you still only call setHostName(data.displayName ||
event.organization || 'Club Name') when !cancelled and data exists; keep
existing updates to profileCache and the catch block behavior intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c8c6254-9f23-4e6f-96b2-09d63dc3e30c
📒 Files selected for processing (9)
app/src/components/EventCard.jsapp/src/components/FeedbackModal.jsapp/src/components/PremiumInput.jsapp/src/screens/AttendanceDashboard.jsapp/src/screens/ClubProfileScreen.jsapp/src/screens/CreateEvent.jsapp/src/screens/EventDetail.jsapp/src/screens/EventRegistrationFormScreen.jsapp/src/screens/MyEventsScreen.js
🚧 Files skipped from review as they are similar to previous changes (8)
- app/src/screens/EventRegistrationFormScreen.js
- app/src/screens/MyEventsScreen.js
- app/src/screens/ClubProfileScreen.js
- app/src/components/PremiumInput.js
- app/src/screens/CreateEvent.js
- app/src/components/FeedbackModal.js
- app/src/screens/AttendanceDashboard.js
- app/src/screens/EventDetail.js
roshankumar0036singh
approved these changes
May 31, 2026
roshankumar0036singh
merged commit 3edd753
into
roshankumar0036singh:main
6 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes Issue #321
Description
Adds loading states and duplicate-submit protection for async operations across key app flows.
Changes
Testing
npm run lintUserFeed.jswarnings.npm test -- --ci --passWithNoTestsSummary by CodeRabbit
New Features
Bug Fixes