Reweight trending toward recency, RO topic→categorie, delete-account modal - #58
Merged
Merged
Conversation
…delete-account modal Trending: computeTrendingScore now weights recency 6x higher (1 pt per 10 min) and caps the coverage bonus at 288 (~48h), so heavily-covered but stale events no longer hold top trending slots past ~3-4 days. Score stays built from an absolute timestamp (indexed, never recomputed by cron) so ordering can't rot. Adds events.rescorePublicPreviews: a one-shot, idempotent migration that re-syncs every existing preview so stored old-formula scores get recomputed, then rebuilds the anonymous feed snapshot. i18n (RO): replace the anglicism "topic/topicuri" with "categorie/categorii" across the feed filter and admin diagnostics (keys and category names unchanged); ban the term in the enforcement test. Profile: replace the two-tap "arm" delete pattern (which crammed the full warning sentence onto the button label) with a proper confirmation Dialog. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
WalkthroughThe PR replaces account deletion’s two-step confirmation with a dialog, adjusts public event preview trending scores and adds paginated rescoring, and updates Romanian localization terminology with regression coverage. ChangesAccount deletion confirmation
Public preview ranking
Romanian category terminology
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant RescoreMutation
participant PublicEventPreviews
participant PublicFeedSnapshots
RescoreMutation->>PublicEventPreviews: paginate and resync previews
PublicEventPreviews-->>RescoreMutation: return page progress
RescoreMutation->>PublicFeedSnapshots: rebuild after final page
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/profile/AuthenticatedProfile.tsx (1)
107-123: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrevent false "deletion failed" toasts from sign-out errors.
The inner
try/finallyblock forauthClient.signOut({})does not catch errors. If the sign-out request fails (e.g., due to a brief network interruption, or because the session was just wiped out bydeleteMyAccount), the error propagates to the outercatchblock.When this happens, the
finallyblock schedules a page redirect, but the outercatchexecutes immediately, showing a falseprofile.deleteFailederror toast despite the user's account having been successfully deleted. Adding an innercatchswallows this benign error and ensures the success state remains intact before the page unloads. As per path instructions, focus on web performance, proper error handling, and runtime errors.💡 Proposed fix
const handleDeleteAccount = async () => { setIsDeleting(true); try { await deleteMyAccount({}); toast.success(t("profile.deleteDone")); try { await authClient.signOut({}); + } catch (signOutError) { + console.error("Sign-out after deletion failed:", signOutError); } finally { location.href = "/"; } } catch (error) { console.error("Account deletion failed:", error); toast.error(t("profile.deleteFailed")); setIsDeleting(false); setDeleteDialogOpen(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 `@apps/web/src/components/profile/AuthenticatedProfile.tsx` around lines 107 - 123, Update handleDeleteAccount so errors from authClient.signOut({}) are caught and swallowed within the inner sign-out flow, while retaining the finally redirect to "/". Ensure sign-out failures do not reach the outer catch or trigger profile.deleteFailed after deleteMyAccount succeeds.Source: Path instructions
🤖 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 `@apps/web/src/components/profile/AuthenticatedProfile.tsx`:
- Around line 315-360: Update the DialogContent in the delete-account flow
around handleDeleteAccount so it cannot close while isDeleting is true: hide the
default close button with showCloseButton and prevent both outside-pointer and
Escape-key dismissal during the mutation via the appropriate event handlers.
Preserve normal dialog dismissal when isDeleting is false.
---
Outside diff comments:
In `@apps/web/src/components/profile/AuthenticatedProfile.tsx`:
- Around line 107-123: Update handleDeleteAccount so errors from
authClient.signOut({}) are caught and swallowed within the inner sign-out flow,
while retaining the finally redirect to "/". Ensure sign-out failures do not
reach the outer catch or trigger profile.deleteFailed after deleteMyAccount
succeeds.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e25fbae2-5517-4bb8-99d5-cac4a4c2f894
📒 Files selected for processing (6)
apps/web/src/components/profile/AuthenticatedProfile.tsxapps/web/src/lib/i18n/strings.test.tspackages/backend/convex/events.tspackages/backend/convex/lib/publicEventPreviews.tspackages/backend/convex/publicEventPreviews.test.tspackages/i18n/src/strings.ts
Comment on lines
+315
to
+360
| <Dialog | ||
| open={deleteDialogOpen} | ||
| onOpenChange={setDeleteDialogOpen} | ||
| > | ||
| <span> | ||
| {deleteArmed | ||
| ? t("profile.deleteConfirm") | ||
| : t("profile.deleteAccount")} | ||
| </span> | ||
| <Trash2 className="size-4" /> | ||
| </Button> | ||
| <DialogTrigger asChild> | ||
| <Button | ||
| type="button" | ||
| variant="outline" | ||
| className="w-full justify-between border-destructive/40 text-destructive hover:bg-destructive/10 hover:text-destructive" | ||
| disabled={isDeleting} | ||
| > | ||
| <span>{t("profile.deleteAccount")}</span> | ||
| <Trash2 className="size-4" /> | ||
| </Button> | ||
| </DialogTrigger> | ||
| <DialogContent> | ||
| <DialogHeader> | ||
| <DialogTitle> | ||
| {t("profile.deleteDialogTitle")} | ||
| </DialogTitle> | ||
| <DialogDescription> | ||
| {t("profile.deleteConfirm")} | ||
| </DialogDescription> | ||
| </DialogHeader> | ||
| <DialogFooter> | ||
| <DialogClose asChild> | ||
| <Button | ||
| type="button" | ||
| variant="outline" | ||
| disabled={isDeleting} | ||
| > | ||
| {t("profile.deleteCancel")} | ||
| </Button> | ||
| </DialogClose> | ||
| <Button | ||
| type="button" | ||
| variant="destructive" | ||
| disabled={isDeleting} | ||
| onClick={() => void handleDeleteAccount()} | ||
| > | ||
| <Trash2 className="size-4" /> | ||
| <span>{t("profile.deleteAccount")}</span> | ||
| </Button> | ||
| </DialogFooter> | ||
| </DialogContent> | ||
| </Dialog> |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Prevent dialog closure while deletion is in progress.
While isDeleting is true, the buttons inside the dialog are disabled, but the user can still close the modal by clicking the backdrop, pressing the Escape key, or clicking the default top-right close button. This causes the UI to abruptly disappear while the background deletion request is still pending.
Consider passing the appropriate event handlers and showCloseButton prop to DialogContent to lock the modal open during the mutation. This gives the user clear, continuous visual feedback until the redirect occurs.
✨ Proposed optional refactor
- <DialogContent>
+ <DialogContent
+ onInteractOutside={(e) => {
+ if (isDeleting) e.preventDefault();
+ }}
+ onEscapeKeyDown={(e) => {
+ if (isDeleting) e.preventDefault();
+ }}
+ showCloseButton={!isDeleting}
+ >
<DialogHeader>
<DialogTitle>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Dialog | |
| open={deleteDialogOpen} | |
| onOpenChange={setDeleteDialogOpen} | |
| > | |
| <span> | |
| {deleteArmed | |
| ? t("profile.deleteConfirm") | |
| : t("profile.deleteAccount")} | |
| </span> | |
| <Trash2 className="size-4" /> | |
| </Button> | |
| <DialogTrigger asChild> | |
| <Button | |
| type="button" | |
| variant="outline" | |
| className="w-full justify-between border-destructive/40 text-destructive hover:bg-destructive/10 hover:text-destructive" | |
| disabled={isDeleting} | |
| > | |
| <span>{t("profile.deleteAccount")}</span> | |
| <Trash2 className="size-4" /> | |
| </Button> | |
| </DialogTrigger> | |
| <DialogContent> | |
| <DialogHeader> | |
| <DialogTitle> | |
| {t("profile.deleteDialogTitle")} | |
| </DialogTitle> | |
| <DialogDescription> | |
| {t("profile.deleteConfirm")} | |
| </DialogDescription> | |
| </DialogHeader> | |
| <DialogFooter> | |
| <DialogClose asChild> | |
| <Button | |
| type="button" | |
| variant="outline" | |
| disabled={isDeleting} | |
| > | |
| {t("profile.deleteCancel")} | |
| </Button> | |
| </DialogClose> | |
| <Button | |
| type="button" | |
| variant="destructive" | |
| disabled={isDeleting} | |
| onClick={() => void handleDeleteAccount()} | |
| > | |
| <Trash2 className="size-4" /> | |
| <span>{t("profile.deleteAccount")}</span> | |
| </Button> | |
| </DialogFooter> | |
| </DialogContent> | |
| </Dialog> | |
| <Dialog | |
| open={deleteDialogOpen} | |
| onOpenChange={setDeleteDialogOpen} | |
| > | |
| <DialogTrigger asChild> | |
| <Button | |
| type="button" | |
| variant="outline" | |
| className="w-full justify-between border-destructive/40 text-destructive hover:bg-destructive/10 hover:text-destructive" | |
| disabled={isDeleting} | |
| > | |
| <span>{t("profile.deleteAccount")}</span> | |
| <Trash2 className="size-4" /> | |
| </Button> | |
| </DialogTrigger> | |
| <DialogContent | |
| onInteractOutside={(e) => { | |
| if (isDeleting) e.preventDefault(); | |
| }} | |
| onEscapeKeyDown={(e) => { | |
| if (isDeleting) e.preventDefault(); | |
| }} | |
| showCloseButton={!isDeleting} | |
| > | |
| <DialogHeader> | |
| <DialogTitle> | |
| {t("profile.deleteDialogTitle")} | |
| </DialogTitle> | |
| <DialogDescription> | |
| {t("profile.deleteConfirm")} | |
| </DialogDescription> | |
| </DialogHeader> | |
| <DialogFooter> | |
| <DialogClose asChild> | |
| <Button | |
| type="button" | |
| variant="outline" | |
| disabled={isDeleting} | |
| > | |
| {t("profile.deleteCancel")} | |
| </Button> | |
| </DialogClose> | |
| <Button | |
| type="button" | |
| variant="destructive" | |
| disabled={isDeleting} | |
| onClick={() => void handleDeleteAccount()} | |
| > | |
| <Trash2 className="size-4" /> | |
| <span>{t("profile.deleteAccount")}</span> | |
| </Button> | |
| </DialogFooter> | |
| </DialogContent> | |
| </Dialog> |
🤖 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 `@apps/web/src/components/profile/AuthenticatedProfile.tsx` around lines 315 -
360, Update the DialogContent in the delete-account flow around
handleDeleteAccount so it cannot close while isDeleting is true: hide the
default close button with showCloseButton and prevent both outside-pointer and
Escape-key dismissal during the mutation via the appropriate event handlers.
Preserve normal dialog dismissal when isDeleting is false.
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
1 participant
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.
Summary
Three changes:
1. Trending ranking — weight recency more, drop stale events
computeTrendingScore(packages/backend/convex/lib/publicEventPreviews.ts):source×10 + article×3) capped at 288 (~48h of recency), so a heavily-covered but stale event floats at most ~2 days above fresher ones. Nothing older than ~3–4 days holds a top slot; actively-updated stories keep a freshlastUpdatedAtand are unaffected.by_trending_score, never recomputed by a cron) so the stored ordering can't silently rot.events.rescorePublicPreviewsre-runs the normal idempotent preview write path for every preview (recomputing the score) and rebuilds the anonymous snapshot on the final page. Must be run in prod after deploy (paginatecontinueCursoruntilisDone).2. Romanian i18n:
topic→categorieReplaced the anglicism topic/topicuri with categorie/categorii (feminine agreement) across the feed filter (
feed.topic.*,feed.preferredTopics) and admin diagnostics. Keys and category display names (Politică, Economie…) unchanged; the legitimate word subiect left alone. Added a/\btopicur/iban + pinned-value test to the enforcement suite.3. Account deletion → confirmation modal
Replaced the two-tap "arm" pattern (which swapped the entire warning sentence onto the button label) with a proper
Dialog: trigger button + modal with title, permanence warning, Cancel, and destructive Delete. Rewordedprofile.deleteConfirm; addedprofile.deleteDialogTitle/profile.deleteCancelin RO + EN.Testing
vitest277 pass / 4 skipped;tsccleanvitest84 pass;tscclean🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Localization
Bug Fixes