-
Notifications
You must be signed in to change notification settings - Fork 406
RI-7614 Filter Query Results based on the type of the page #5043
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
Conversation
- makse sure to follow the guidelines defined by the Workbecnh page, so do save query results in IndexDB only when the respective feature flag allows it ire #RI-7602
- filter out the history loaded from the IndexDB, based on the type of the commands, so we can show it accordingly to the Search and Workbench pages re #RI-7602
Code Coverage - Frontend unit tests
Test suite run success5170 tests passing in 677 suites. Report generated by 🧪jest coverage report action from e92d15d |
- extend the IndexDB indexes to allow easier deletion of records by their type (search/workbench) - apply the new deletion logic on the search and workbench pages re #RI-7614
|
||
// Step 3: Persist results locally so Vector Search history (CommandsView) shows it | ||
if (Array.isArray(data) && data.length) { | ||
if (envDependentFlag === false && Array.isArray(data) && data.length) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to preserve the commands inside IndexDB always, it should depend on this feature flag, as it's handled in the Workbench.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be done on this layer. Ideally we must always call addCommands and then have a storage strategy based on envDependent flag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, addCommands
is tightly related to wbHistoryStorage
, it is not meant to be general solution for command history storage. Internally it uses WorkbenchStorage
class, that is writing to IndexedDb.
Same or similar thing can be applied to search history storage. Since it is new implementation, I agree with Artem, that better approach would be an implementation, that takes a useBackendPersistanse = false
flag and internally decides how to store the commands, but using addCommands
will ALWAYS add these commands to workbench history, which is apparently a problem
}) | ||
|
||
await addCommands(reverse(data)) | ||
if (envDependentFlag === false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to preserve the commands inside IndexDB always, it should depend on this feature flag, as it's handled in the Workbench.
}) | ||
objectStore.createIndex( | ||
CommandExecutionIndex.DatabaseIdType, | ||
['databaseId', 'type'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should only remove the records related to the page (Search/Workbench) when clicking the “Clear Results” button. Instead of adding an additional check in the cursor (which iterates over the records while deleting them), I’m introducing a new index that can easily target the correct type.
However, if you find this approach to be incorrect, I can revert to the iterative deletion method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thinks this is wrong approach. Do not change WorkbenchStorage class to differentiate between storage types, just create vectorSearchHistoryStorage
export const vectorSearchHistoryStorage = new WorkbenchStorage(
riConfig.app.vectorSearchName,
2,
)
And if you want to generalize addCommands
, getLocalWbHistory
etc, modify them to accept the storage instance
const idbIndex = objectStore?.index('dbId') | ||
const indexReq = idbIndex?.openCursor(dbId) | ||
const idbIndex = objectStore?.index(indexName) | ||
const indexReq = idbIndex?.openCursor(indexSelector) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, we simply pass the database instance and the type we target (search or workbench), and it will do it.
const idbIndex = objectStore?.index('dbId_type')
const indexReq = idbIndex?.openCursor(['databaseId', 'SEARCH'])
Of course, we still support the old behavior, so you can still target a specific database instance and delete all records related to it.
const idbIndex = objectStore?.index('dbId')
const indexReq = idbIndex?.openCursor('databaseId')
Otherwise, we should delete the records by their type inline, inside the cursor below.
} | ||
|
||
return history || [] | ||
return history.filter((item) => item.type === commandExecutionType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we do the magic to apply some filtering, based on the type (Search/Workbench), so we don't show the full history on both pages.
- rework the hooks responsible to keep the commands history for the searach page (on desktop) to use the redux store, like the workbench re #RI-7614
|
||
const resultsMode = ResultsMode.Default | ||
const activeRunQueryMode = RunQueryMode.ASCII | ||
const executionType = CommandExecutionType.Search |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, we need to rely on the current implementation for the history manipulation (and automatically switch between SQLite and IndexDB based on the feature flag), so we should go for the existing helpers instead of building them again.
const envDependentFlag = useSelector( | ||
(state: RootState) => | ||
state?.app?.features?.featureFlags?.features?.envDependent?.flag, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we must return envDependent: { flag: boolean}
here and use it below like envdepenent.flag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we have appFeatureFlagsFeaturesSelector
Redux selector, so we can rely on it, or create a new one for this envDependant
flag as well. It should be applied to a few more places in the app as well.
|
||
// Step 3: Persist results locally so Vector Search history (CommandsView) shows it | ||
if (Array.isArray(data) && data.length) { | ||
if (envDependentFlag === false && Array.isArray(data) && data.length) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be done on this layer. Ideally we must always call addCommands and then have a storage strategy based on envDependent flag
|
||
// Step 3: Persist results locally so Vector Search history (CommandsView) shows it | ||
if (Array.isArray(data) && data.length) { | ||
if (envDependentFlag === false && Array.isArray(data) && data.length) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, addCommands
is tightly related to wbHistoryStorage
, it is not meant to be general solution for command history storage. Internally it uses WorkbenchStorage
class, that is writing to IndexedDb.
Same or similar thing can be applied to search history storage. Since it is new implementation, I agree with Artem, that better approach would be an implementation, that takes a useBackendPersistanse = false
flag and internally decides how to store the commands, but using addCommands
will ALWAYS add these commands to workbench history, which is apparently a problem
}) | ||
objectStore.createIndex( | ||
CommandExecutionIndex.DatabaseIdType, | ||
['databaseId', 'type'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thinks this is wrong approach. Do not change WorkbenchStorage class to differentiate between storage types, just create vectorSearchHistoryStorage
export const vectorSearchHistoryStorage = new WorkbenchStorage(
riConfig.app.vectorSearchName,
2,
)
And if you want to generalize addCommands
, getLocalWbHistory
etc, modify them to accept the storage instance
Description
Show the correct command results on the Search and Workbench pages, by filtering out the queries based on the page from which they were executed.
Screen.Recording.2025-10-07.at.16.36.08.mov
How it was tested
We have two available ways to store the history of the executed commands - SQLite and IndexDB inside the browser itself.
Using Index DB
Create
redisinsight/ui/.env
and make sure to putRI_FEATURES_ENV_DEPENDENT_DEFAULT_FLAG=false
inside it.Using SQLite
It's the default mode, but in case you added
RI_FEATURES_ENV_DEPENDENT_DEFAULT_FLAG
make sure to flip it totrue
Reproduction Steps
SEARCH COMMAND
)WORKBENCH COMMAND
)Switch between the pages and make sure you see the correct commands in the list with the results.