Skip to content

refine time range picker styles and user store#433

Merged
iceljc merged 1 commit intoSciSharp:mainfrom
iceljc:main
Mar 11, 2026
Merged

refine time range picker styles and user store#433
iceljc merged 1 commit intoSciSharp:mainfrom
iceljc:main

Conversation

@iceljc
Copy link
Collaborator

@iceljc iceljc commented Mar 11, 2026

No description provided.

@qodo-code-review
Copy link

Review Summary by Qodo

Refine time range picker styles and user store logic

✨ Enhancement 🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Improved time range picker button styling with ellipsis text overflow handling
• Increased HTTP retry queue timeout from 20ms to 200ms for reliability
• Refactored user store retrieval to check store value before session storage
• Added get import from svelte/store for proper store value access
Diagram
flowchart LR
  A["TimeRangePicker Button"] -- "Add clickable class" --> B["Enhanced Styling"]
  A -- "Add text overflow handling" --> C["Ellipsis on Long Text"]
  D["HTTP Retry Queue"] -- "Increase timeout 20→200ms" --> E["Better Reliability"]
  F["User Store"] -- "Check store value first" --> G["Improved Logic"]
  F -- "Add get import" --> G
Loading

Grey Divider

File Changes

1. src/lib/common/shared/TimeRangePicker.svelte ✨ Enhancement +4/-3

Add text overflow handling and clickable styling

• Added clickable CSS class to the button element for consistent styling
• Removed inline cursor: pointer style in favor of class-based approach
• Wrapped span text with overflow handling styles (ellipsis, text-overflow, white-space)
• Prevents long time range text from breaking button layout

src/lib/common/shared/TimeRangePicker.svelte


2. src/lib/helpers/http.js 🐞 Bug fix +1/-1

Increase HTTP retry queue timeout value

• Increased retry queue timeout from 20ms to 200ms
• Provides more time for token refresh operations to complete

src/lib/helpers/http.js


3. src/lib/helpers/store.js ✨ Enhancement +8/-7

Refactor user store retrieval with proper value access

• Added get import from svelte/store alongside writable
• Refactored getUserStore() to check store value first before session storage
• Simplified logic by removing redundant else branches
• Returns get(userStore) as fallback instead of raw store object

src/lib/helpers/store.js


Grey Divider

Qodo Logo

@qodo-code-review
Copy link

qodo-code-review bot commented Mar 11, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Logout token not cleared 🐞 Bug ⛨ Security
Description
getUserStore() now returns the in-memory userStore value when it has a token, but logout only clears
sessionStorage via resetStorage(true) and never clears userStore, so the SPA can continue sending
the old Authorization token after logout. This breaks logout guarantees and can keep a user
effectively authenticated until a full reload.
Code

src/lib/helpers/store.js[R45-56]

+        const storeValue = get(userStore);
+        if (storeValue?.token) {
+            return storeValue;
+        }
+
        let json = sessionStorage.getItem(userKey);
        if (json) {
            return JSON.parse(json);
-        } else {
-            return userStore;
        }
-    } else {
-        // Return a default value for SSR
-        return userStore;
    }
+
+    return get(userStore);
Evidence
Logout clears sessionStorage (userKey) but not the in-memory Svelte store; after this PR,
getUserStore() prefers the in-memory store value when it has a token, and axios attaches
Authorization from getUserStore().token on every request. Therefore, after logout, if userStore
still contains a token, requests will continue to be authenticated.

src/lib/helpers/store.js[43-57]
src/lib/helpers/store.js[296-307]
src/lib/common/dropdowns/ProfileDropdown.svelte[16-28]
src/lib/helpers/http.js[105-125]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Logout calls `resetStorage(true)` which clears `sessionStorage`, but does not clear the in-memory `userStore`. After the PR change, `getUserStore()` prefers `get(userStore)` when it has a token, so the app can keep sending an Authorization header even after logout.

### Issue Context
`axios.interceptors.request.use` reads `getUserStore().token` for every request; therefore logout must clear both persisted and in-memory auth state.

### Fix Focus Areas
- src/lib/helpers/store.js[296-307]
- src/lib/helpers/store.js[37-57]
- src/lib/common/dropdowns/ProfileDropdown.svelte[16-28]
- src/lib/helpers/http.js[105-125]

### Suggested change
- In `resetStorage(resetUser=true)`, call `userStore.set({ id: '', full_name: '', expires: 0, token: null })` (and clear any other user fields used elsewhere, e.g. `renew_token_count`).
- Optionally, update the `userStore.subscribe` persistence logic to `removeItem(userKey)` when `value.token` is falsy to prevent stale resurrection in other flows.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Wrong getUserStore JSDoc 🐞 Bug ✓ Correctness
Description
getUserStore() returns a plain UserModel value (via get(userStore) or JSON.parse), but its JSDoc
claims it returns a Writable<UserModel>. This can mislead typed consumers and encourages incorrect
usage patterns.
Code

src/lib/helpers/store.js[R42-57]

 */
export function getUserStore() {
    if (browser) {
-        // Access localStorage only if in the browser context
+        const storeValue = get(userStore);
+        if (storeValue?.token) {
+            return storeValue;
+        }
+
        let json = sessionStorage.getItem(userKey);
        if (json) {
            return JSON.parse(json);
-        } else {
-            return userStore;
        }
-    } else {
-        // Return a default value for SSR
-        return userStore;
    }
+
+    return get(userStore);
};
Evidence
The function’s implementation only ever returns plain objects (store value or parsed JSON), never
the Svelte Writable itself, contradicting its declared return type.

src/lib/helpers/store.js[40-57]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`getUserStore()` returns a `UserModel` object, but its JSDoc says it returns `Writable&lt;UserModel&gt;`, which is incorrect.

### Issue Context
This function is used as a value getter throughout the codebase (e.g. accessing `.token`, `.expires`). Correct JSDoc improves IDE/TS inference and prevents misuse.

### Fix Focus Areas
- src/lib/helpers/store.js[40-57]

### Suggested change
- Change the JSDoc to `@returns {import(&#x27;$userTypes&#x27;).UserModel}` (or an explicit typedef) to match what is returned.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@iceljc iceljc merged commit b776f71 into SciSharp:main Mar 11, 2026
1 of 2 checks passed
Comment on lines +45 to +56
const storeValue = get(userStore);
if (storeValue?.token) {
return storeValue;
}

let json = sessionStorage.getItem(userKey);
if (json) {
return JSON.parse(json);
} else {
return userStore;
}
} else {
// Return a default value for SSR
return userStore;
}

return get(userStore);

Choose a reason for hiding this comment

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

Action required

1. Logout token not cleared 🐞 Bug ⛨ Security

getUserStore() now returns the in-memory userStore value when it has a token, but logout only clears
sessionStorage via resetStorage(true) and never clears userStore, so the SPA can continue sending
the old Authorization token after logout. This breaks logout guarantees and can keep a user
effectively authenticated until a full reload.
Agent Prompt
### Issue description
Logout calls `resetStorage(true)` which clears `sessionStorage`, but does not clear the in-memory `userStore`. After the PR change, `getUserStore()` prefers `get(userStore)` when it has a token, so the app can keep sending an Authorization header even after logout.

### Issue Context
`axios.interceptors.request.use` reads `getUserStore().token` for every request; therefore logout must clear both persisted and in-memory auth state.

### Fix Focus Areas
- src/lib/helpers/store.js[296-307]
- src/lib/helpers/store.js[37-57]
- src/lib/common/dropdowns/ProfileDropdown.svelte[16-28]
- src/lib/helpers/http.js[105-125]

### Suggested change
- In `resetStorage(resetUser=true)`, call `userStore.set({ id: '', full_name: '', expires: 0, token: null })` (and clear any other user fields used elsewhere, e.g. `renew_token_count`).
- Optionally, update the `userStore.subscribe` persistence logic to `removeItem(userKey)` when `value.token` is falsy to prevent stale resurrection in other flows.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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