Skip to content

Only call advisory after being logged in#4384

Open
ildyria wants to merge 1 commit into
masterfrom
fix-advisory-call
Open

Only call advisory after being logged in#4384
ildyria wants to merge 1 commit into
masterfrom
fix-advisory-call

Conversation

@ildyria
Copy link
Copy Markdown
Member

@ildyria ildyria commented May 30, 2026

Summary by CodeRabbit

Bug Fixes

  • Bug Fixes
    • Enhanced guest login flow with improved timing for advisory notifications during the authentication process.
    • Optimized data loading and synchronization to reduce unnecessary operations during login.

Review Change Stack

@ildyria ildyria requested a review from a team as a code owner May 30, 2026 08:38
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

📝 Walkthrough

Walkthrough

Timeline guest login flow refactored to separate post-login advisory checks from general refresh logic. A new onLoggedIn() handler awaits refresh() and then triggers advisoryCheck(), replacing direct refresh callbacks on login modals. refresh() now only handles timeline loading and concurrent store updates.

Changes

Guest login flow refactoring

Layer / File(s) Summary
Login flow separation with advisory check
resources/js/views/gallery-panels/Timeline.vue
Event handlers for LoginModal and WebauthnModal wired to new onLoggedIn() function. New onLoggedIn() awaits refresh() then calls advisoryCheck(). refresh() refactored to only handle timeline loading and concurrent store updates without advisory check.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A login dance, now split in two,
Post-check deferred until refresh is through,
Modal events flow to handlers anew,
Advisory checks when logins come true! ✨

🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
resources/js/views/gallery-panels/Timeline.vue (1)

314-317: 💤 Low value

Prefer .then() over async/await per project convention.

The handler logic is correct (advisory now fires only after a successful post-login refresh, matching the PR intent). However, as per coding guidelines, Vue3 code should chain promises with .then() rather than using await.

♻️ Proposed refactor
-async function onLoggedIn() {
-	await refresh();
-	advisoryCheck();
-}
+function onLoggedIn() {
+	refresh().then(() => advisoryCheck());
+}

As per coding guidelines: "Do not use await async calls in Vue3, use .then() instead". Note refresh() and onMounted in this file already use await, so you may prefer to defer this for consistency.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e0498bf4-16a2-4179-bd1c-c9b1291bead8

📥 Commits

Reviewing files that changed from the base of the PR and between 3fa2651 and 4aaedf0.

📒 Files selected for processing (1)
  • resources/js/views/gallery-panels/Timeline.vue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant