Skip to content

fix: Historical user conversation data has been cleared #2843

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ const props = withDefaults(
showUserInput?: boolean
sendMessage: (question: string, other_params_data?: any, chat?: chatType) => void
openChatId: () => Promise<string>
validate: () => Promise<boolean | string>
validate: () => Promise<any>
}>(),
{
applicationDetails: () => ({}),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code snippet has one primary issue:

  1. Type Inconsistency in validate Function: The original code specifies that validate returns Promise<boolean | string>, which means it can either resolve to a Boolean value or a String indicating an error.

  2. Updated Type for validate:

    validate: () => Promise<any>,

    This change allows validate to return any type of object or data, which is not appropriate according to the previous definition. You should ideally specify string as the return type if you plan on returning errors, or otherwise define a common type for both success outcomes and errors.

To maintain correctness, here is how you could update the function declaration:

validate(): Promise<ResponseData>,


// Example response object structure:
interface ResponseData {
  success: boolean,
  errorMessage?: string // If used, optional
}

This update maintains consistency with the expected behavior specified in the initial withDefaults setup when calling validate().

Expand Down
4 changes: 2 additions & 2 deletions ui/src/stores/modules/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ const useApplicationStore = defineStore({
applicationApi
.postAppAuthentication(token, loading, authentication_value)
.then((res) => {
localStorage.setItem(`${token}accessToken`, res.data)
sessionStorage.setItem(`${token}accessToken`, res.data)
localStorage.setItem(`${token}-accessToken`, res.data)
sessionStorage.setItem(`${token}-accessToken`, res.data)
resolve(res)
})
.catch((error) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code snippet has a couple of small optimizations you might consider:

  1. Consistent Item Name Format: It seems that using different storage methods (localStorage and sessionStorage) with the same item name (${token}accessToken) can lead to unexpected behavior if keys overlap due to differences in case sensitivity cross-browsers.

    // Recommended change: Use different item names for each store type
    localStorage.setItem(`${token}-accessToken`, res.data);
    sessionStorage.setItem(`${token.toUpperCase()}-accessToken`, res.data);  // Ensure consistency in casing

2. **Handling Error Cases More Explicitly**: The current error handling does not explicitly catch or log errors. This makes it harder to debug issues related to API responses or server-side problems.

3. **TypeScript Annotations**: If this is TypeScript code, adding appropriate types to arguments and return values could improve readability and maintainability.

```typescript
// Type annotations
const useApplicationStore = defineStore({
  state() {
    return {
      applicationApi: null as any,
      token: '' as string,
      loading: false as boolean,
    };
  },
  actions: {
    setToken(newToken: string): void {
      this.token = newToken;
    },
    postAppAuthentication(
      token: string,
      loading: boolean,
      authentication_value: string | null
    ): Promise<any> { // Add more specific typing as needed
      this.loading = true;  // You might want to start loading here too

      return this.applicationApi.postAppAuthentication(token, loading, authentication_value).then((res) => {
        localStorage.setItem(`${token.toLocaleUpperCase()}-accessToken`, res.data)  // Adjusted format for uniformity
        this.setToken(token) // Assuming you need to update your main context with the new token
        return res; // Return the response promise to allow chaining
      }).catch((error: any) => {  // Catch and handle errors appropriately
        console.error('Error during authentication:', error.message || error.toString());
        reject(error);
      });
    },
  },
});

By implementing these changes, the code becomes cleaner, more robust, and adheres better to best practices for both syntax and functionality.

Expand Down
4 changes: 2 additions & 2 deletions ui/src/stores/modules/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@ const useUserStore = defineStore({
return this.userType === 1 ? localStorage.getItem('token') : this.getAccessToken()
},
getAccessToken() {
const token = sessionStorage.getItem(`${this.userAccessToken}accessToken`)
const token = sessionStorage.getItem(`${this.userAccessToken}-accessToken`)
if (token) {
return token
}
const local_token = localStorage.getItem(`${token}accessToken`)
const local_token = localStorage.getItem(`${token}-accessToken`)
if (local_token) {
return local_token
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code snippet has a small issue that needs to be addressed:

const useUserStore = defineStore({
  state: () => ({
    userType: null,
    userAccessToken: '',
  }),
  actions: {
    getTokenFromStorage(type) {
      let token;
      if (type === 'session') {
        token = sessionStorage.getItem(`${this.userAccessToken}-accessToken`);
      } else { // Assuming 'local' means using localStorage
        token = localStorage.getItem(`${this.userAccessToken}-accessToken`);
      }
      return token || '';
    },
    getAccessToken(token_type) {
      const storedToken = this.getTokenFromStorage(token_type);
      const isSessionToken = token_type === 'session';
      
      const access_token = isSessionToken ? storedToken : this.getAccessToken();
      
      return access_token;
    }
  }
});

Explanation of the Improvement:

  • Variable Naming Consistency: Changed token variable name within the function getTokenFromStorage() to avoid confusion with the existing token property in the store's state.
  • Consistent Logic: The logic inside the getTokensFromStorage() function remains consistent and uses similar pattern checks.
  • Return Value Handling: Ensured an empty string is returned if no token is found instead of undefined, which makes it safer from usage bugs.

This change improves readability and maintainability while maintaining the same functionality.

Expand Down