Skip to content
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

issue-108: Add saved search feature #110

Closed
wants to merge 1 commit into from
Closed

issue-108: Add saved search feature #110

wants to merge 1 commit into from

Conversation

nikhilj13
Copy link
Contributor

Save searches

Save search feature allows you to save your search (including any filters that are applied) which can later be revisited by clicking on the saved search title. This feature is disabled by default.

How to enable save search

You can turn on the save search option by passing in an additional prop to SearchBar component, this prop is called allowSavedSearch. For example:

<SearchBar allowSavedSearch />

This would show a heart icon with a drop down next to the Search Bar, which means the UI component for saved search is enabled.

Note: In order to make this feature work, we need to create a new zone to store the saved searches. This feature will NOT work if you haven't configured and specified a new zone for saved searches.

Configuring a new zone to store the saved searches

Follow these steps to configure a new zone for storing the saved searches:

  1. Zones are configured via the Index Feature, in [project-dir]\conf\features\core\Index.index.xml. It should look similar to:

    <?xml version="1.0" encoding="UTF-8"?>
    
    <ff:features xmlns:ff="http://www.attivio.com/configuration/config" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:fbase="http://www.attivio.com/configuration/features/base" xmlns:f="http://www.attivio.com/configuration/features/core" xsi:schemaLocation="http://www.attivio.com/configuration/config http://www.attivio.com/configuration/config.xsd http://www.attivio.com/configuration/features/base http://www.attivio.com/configuration/features/baseFeatures.xsd http://www.attivio.com/configuration/features/core http://www.attivio.com/configuration/features/coreFeatures.xsd">
     <f:index enabled="true" name="index" profile="true">
       <f:partitionSet size="1"/>
       <f:writer logCommits="true" nodeset="ingestion" search="true"/>
     </f:index>
    </ff:features>
    
  2. Edit this file to add in a new index zone. For example, we're creating a new index zone called testZone in the example below:

    <?xml version="1.0" encoding="UTF-8"?>
    
    <ff:features xmlns:ff="http://www.attivio.com/configuration/config" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:fbase="http://www.attivio.com/configuration/features/base" xmlns:f="http://www.attivio.com/configuration/features/core" xsi:schemaLocation="http://www.attivio.com/configuration/config http://www.attivio.com/configuration/config.xsd http://www.attivio.com/configuration/features/base http://www.attivio.com/configuration/features/baseFeatures.xsd http://www.attivio.com/configuration/features/core http://www.attivio.com/configuration/features/coreFeatures.xsd">
       <f:index enabled="true" name="index" profile="true">
       	<f:writer defaultZoneName="default" logCommits="true" nodeset="ingestion" search="true">
       		<f:zone name="default" />
       		<f:zone name="testZone" />
       	</f:writer>
       </f:index>
    </ff:features>
    

If you don't configure any zones, there is always one zone named default. However, if you configure zones, you must explicitly configure the default zone in addition to the other zone. Additionally, you can also set up separate search workflows for these zones so you don't get the results from your testZone as a part of the search results.

  1. Configure the save search in SearchUI's configuration.properties file. You can specify the name of the new index zone for saved searches, the table name for all the documents for saved searches and the search workflow to be used for querying the mentioned zone. Below is an example with default values:
    SearchBar: {
    	// configures the search workflow used for saved searches
    	savedSearchWorkflow: 'searchSavedSearches',
    	// configures the table name for saved searches
    	savedSearchTable: 'savedSearches',
    	// specifies the index zone for saved searches
    	savedSearchZone: 'savedSearches',
    }
    

How to use saved searches

After you enable and configure saved searches, you should have this component right next to the search bar:
SavedSearch

Saving a search

You can save searches by clicking on the first option in the drop-down menu 'SAVE SEARCH'. It will open up a modal that asks for the name for this search.
Save a search

If you don't enter a name, the search term is used as the name.

Click on save search button to save the search.

Revisiting a search

You can revisit a search by clicking on the search title in the drop-down for saved search.
Revisit a search

Deleting a saved search

You can also delete a saved search, there's a x next to every saved search in the drop-down. Simply click on the x to delete a saved search.
Delete a search

Note that the drop down only shows the 10 most recent saved searches.

@klk1010 klk1010 added the merge conflicts This PR has a merge conflict that needs to be resolved. label Jul 25, 2019
@klk1010
Copy link
Contributor

klk1010 commented Jul 25, 2019

@nikhilj13, the notes on this PR are fantastic. Can you create an answer page and copy this info there? I'd hate to lose track of this functionality.


declare var webkitSpeechRecognition: any; // Prevent complaints about this not existing

type SearchBarProps = {
history: PropTypes.object.isRequired;
location: PropTypes.object.isRequired;
Copy link
Contributor

@klk1010 klk1010 Jul 25, 2019

Choose a reason for hiding this comment

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

If we make the location required this would become a breaking change. Can you refactor things a bit to make this optional?
Update: withRouter is already included in this component, so making use of an additional prop it provides is not a breaking change. This is fine.

/** Optional, specifies the workflow that should be used for querying the index which stores saved searches */
savedSearchWorkflow?: string;
/** Optional, specifies the table name assigned to the docs for saved searches, defaults to 'savedSearches' */
savedSearchTable: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Indicate this is optional in flow:
savedSearchTable?: string;

/** Optional, specifies the table name assigned to the docs for saved searches, defaults to 'savedSearches' */
savedSearchTable: string;
/** Specifies the zone name for the zone that stores saved searches, defaults to 'savedSearches' */
savedSearchZone: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this optional?
savedSearchZone?: string;


// populates the saved searches in the saved searches drop down
populateSavedSearches() {
const response = this.state.response;
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer desctructuring.
const { response } = this.state;

if (response) {
this.setState(
{
response,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you create a class for the response in the api directory and move the formatting you do later on into it to be used here?

@@ -54,6 +64,14 @@ type SearchBarProps = {
buttonLabel: string;
/** If set, this is the route to navigate to upon executing a search. By default, no navigation will occur when searching. */
route: string | null;
/** If set, the save search option is displayed next to the SearchBar, false by default */
allowSavedSearch: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

New props should be optional for backwards compatibility.
allowSavedSearch?: boolean;

savedSearch.title = htmlString.replace(/<[^>]+>/g, '');
savedSearch.query = doc.getFirstValue('query_s');
savedSearch.queryString = doc.getFirstValue('query_string_s');
let savedSearchTitle = savedSearch.title;
Copy link
Contributor

Choose a reason for hiding this comment

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

const savedSearchTitle = savedSearch.title > 15 ? `${savedSearch.title.substring(0, 15)}...` : savedSearch.title;

substring() does not modify the original string, it returns a brand new string, so it's safe to use the original string.

</MenuItem>,
);
});
this.setState({ savedSearchList: menuItemList });
Copy link
Contributor

Choose a reason for hiding this comment

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

Storing jsx as state is generally considered bad practice. It's usually an indicator that you need to refactor your component, break out a function, or create a building block component.
https://reactjs.org/docs/thinking-in-react.html#step-3-identify-the-minimal-but-complete-representation-of-ui-state

@@ -223,6 +357,44 @@ class SearchBar extends React.Component<SearchBarDefaultProps, SearchBarProps, S
}
}

// saves the current search from the url and feed it to a separate index zone
saveThisSearch(searcherState) {
const searcher = this.context.searcher;
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer destructuring:
const { searcher } = this.context;

const username = AuthUtils.getLoggedInUserId();
const loggedDateTime = new Date().toISOString();
const id = username.concat(loggedDateTime);
const title = this.state.savedSearchTitle;
Copy link
Contributor

Choose a reason for hiding this comment

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

Place this at the top of the function somewhere above line 364. Prefer destructuring:
const { savedSearchTitle: title } = this.state;

+ `[ "${title}" ], "query_s" : [ "${currentSearch}" ], "query_string_s" : [ "${queryString}" ], `
+ `"username_s" : [ "${username}" ], "date" : [ "${loggedDateTime}" ], "table" : [ "${savedSearchTable}" ] }, `
+ `"id" : "${id}", "zone" : "${savedSearchZone}" }`;
searcher.search.updateSavedSearches(JSON.parse(jsonDoc), this.getSavedSearches);
Copy link
Contributor

Choose a reason for hiding this comment

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

Response parsing should be done in a class.

</MenuItem>
);

const saveQuery = this.props.allowSavedSearch ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this to out's own render function. Same with the other saved variables.

Copy link
Contributor

@klk1010 klk1010 left a comment

Choose a reason for hiding this comment

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

Address comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge conflicts This PR has a merge conflict that needs to be resolved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants