Просмотр исходного кода

Merge branch 'feat/77515-display-search-result-with-snippet' into feat/77515-79012-fix-bug-caused-by-search-refactor

* feat/77515-display-search-result-with-snippet: (29 commits)
  79625 undo diff of state var name
  79625 undo diffs made to names of methods nad state vars
  78577 fix
  79625 remove comments & change state var names
  no message
  78577 add await to catch exception
  78577 refactoring back
  78577 refactoring front
  79625 fix the text back to original
  79625 change button position and names of methods and state vars
  79625 change translation text
  78577 if condition changed
  78577 refactor
  78577 refactor
  added comment
  78577 adding es to pages in service
  organize diffs
  cleaned up diffs
  added contentWithNoKeyword to page
  78577 removed searchResult
  ...
Mao 4 лет назад
Родитель
Сommit
f63ef00069

+ 1 - 1
packages/app/resource/locales/ja_JP/translation.json

@@ -64,7 +64,7 @@
   "Include Attachment File": "添付ファイルも含める",
   "Include Comment": "コメントも含める",
   "Include Subordinated Page": "配下ページも含める",
-  "Include Subordinated Target Page": "{{target}}下を含む",
+  "Include Subordinated Target Page": "{{target}} 下を含む",
   "All Subordinated Page": "全ての配下ページ",
   "Specify Hierarchy": "階層の深さを指定",
   "Submitted the request to create the archive": "アーカイブ作成のリクエストを正常に送信しました",

+ 0 - 5
packages/app/src/components/SearchPage.jsx

@@ -115,11 +115,6 @@ class SearchPage extends React.Component {
       .then((res) => {
         this.changeURL(keyword);
         if (res.data.length > 0) {
-          // TODO: remove creating dummy snippet lines when the data with snippet is abole to be retrieved
-          res.data.forEach((page) => {
-            page.snippet = `dummy snippet dummpy snippet dummpy snippet dummpy snippet dummpy snippet
-            dummpy snippet dummpy snippet dummpy snippet dummpy snippet`;
-          });
           this.setState({
             searchedKeyword: keyword,
             searchedPages: res.data,

+ 8 - 12
packages/app/src/components/SearchPage/SearchControl.tsx

@@ -40,29 +40,25 @@ const SearchControl: FC <Props> = (props: Props) => {
       </div>
       {/* TODO: replace the following elements deleteAll button , relevance button and include specificPath button component */}
       <div className="d-flex my-4">
-        <div className="form-check border-gray">
+        <div className="d-flex align-items-center border rounded border-gray px-2 py-1 mr-2 ml-auto">
+          <label className="my-0 mr-2" htmlFor="flexCheckDefault">
+            {t('Include Subordinated Target Page', { target: '/user' })}
+          </label>
           <input
-            className="form-check-input"
             type="checkbox"
-            value=""
             id="flexCheckDefault"
             onClick={() => onExcludeUsersHome()}
           />
-          <label className="form-check-label" htmlFor="flexCheckDefault">
-            {t('Include Subordinated Target Page', { target: '/user' })}
-          </label>
         </div>
-        <div className="form-check">
+        <div className="d-flex align-items-center border rounded border-gray px-2 mr-3">
+          <label className="my-0 mr-2" htmlFor="flexCheckChecked">
+            {t('Include Subordinated Target Page', { target: '/trash' })}
+          </label>
           <input
-            className="form-check-input"
             type="checkbox"
-            value=""
             id="flexCheckChecked"
             onClick={() => onExcludeTrash()}
           />
-          <label className="form-check-label" htmlFor="flexCheckChecked">
-            {t('Include Subordinated Target Page', { target: '/trash' })}
-          </label>
         </div>
       </div>
     </div>

+ 0 - 356
packages/app/src/components/SearchPage/SearchResult.jsx

@@ -1,356 +0,0 @@
-import React from 'react';
-import PropTypes from 'prop-types';
-import * as toastr from 'toastr';
-
-import { withTranslation } from 'react-i18next';
-
-import Page from '../PageList/Page';
-import SearchResultList from './SearchResultList';
-import DeletePageListModal from './DeletePageListModal';
-import AppContainer from '~/client/services/AppContainer';
-import { withUnstatedContainers } from '../UnstatedUtils';
-
-// NOTE : this file will be deleted in the future. Merge conflict happend in this file, so temporaly kept this left here.
-// Task 77833 deleted this file ;
-class SearchResult extends React.Component {
-
-  constructor(props) {
-    super(props);
-    this.state = {
-      deletionMode: false,
-      selectedPages: new Set(),
-      isDeleteCompletely: undefined,
-      isDeleteConfirmModalShown: false,
-      errorMessageForDeleting: undefined,
-    };
-    this.toggleDeleteCompletely = this.toggleDeleteCompletely.bind(this);
-    this.deleteSelectedPages = this.deleteSelectedPages.bind(this);
-    this.closeDeleteConfirmModal = this.closeDeleteConfirmModal.bind(this);
-  }
-
-  isNotSearchedYet() {
-    return !this.props.searchResultMeta.took;
-  }
-
-  isNotFound() {
-    return this.props.searchingKeyword !== '' && this.props.pages.length === 0;
-  }
-
-  isError() {
-    if (this.props.searchError !== null) {
-      return true;
-    }
-    return false;
-  }
-
-  /**
-   * move the page
-   */
-  visitPageButtonHandler(e) {
-    window.location.href = e.currentTarget.value;
-  }
-
-  /**
-   * toggle checkbox and add (or delete from) selected pages list
-   *
-   * @param {any} page
-   * @memberof SearchResult
-   */
-  toggleCheckbox(page) {
-    if (this.state.selectedPages.has(page)) {
-      this.state.selectedPages.delete(page);
-    }
-    else {
-      this.state.selectedPages.add(page);
-    }
-    this.setState({ isDeleteConfirmModalShown: false });
-    this.setState({ selectedPages: this.state.selectedPages });
-  }
-
-  /**
-   * check and return is all pages selected for delete?
-   *
-   * @returns all pages selected (or not)
-   * @memberof SearchResult
-   */
-  isAllSelected() {
-    return this.state.selectedPages.size === this.props.pages.length;
-  }
-
-  /**
-   * handle checkbox clicking that all pages select for delete
-   *
-   * @memberof SearchResult
-   */
-  handleAllSelect() {
-    if (this.isAllSelected()) {
-      this.state.selectedPages.clear();
-    }
-    else {
-      this.state.selectedPages.clear();
-      this.props.pages.map((page) => {
-        this.state.selectedPages.add(page);
-        return;
-      });
-    }
-    this.setState({ selectedPages: this.state.selectedPages });
-  }
-
-  /**
-   * change deletion mode
-   *
-   * @memberof SearchResult
-   */
-  handleDeletionModeChange() {
-    this.state.selectedPages.clear();
-    this.setState({ deletionMode: !this.state.deletionMode });
-  }
-
-  /**
-   * toggle check delete completely
-   *
-   * @memberof SearchResult
-   */
-  toggleDeleteCompletely() {
-    // request で completely が undefined でないと指定アリと見なされるため
-    this.setState({ isDeleteCompletely: this.state.isDeleteCompletely ? undefined : true });
-  }
-
-  /**
-   * delete selected pages
-   *
-   * @memberof SearchResult
-   */
-  deleteSelectedPages() {
-    const deleteCompletely = this.state.isDeleteCompletely;
-    Promise.all(Array.from(this.state.selectedPages).map((page) => {
-      return new Promise((resolve, reject) => {
-        const pageId = page._id;
-        const revisionId = page.revision._id;
-
-        this.props.appContainer.apiPost('/pages.remove', { page_id: pageId, revision_id: revisionId, completely: deleteCompletely })
-          .then((res) => {
-            if (res.ok) {
-              this.state.selectedPages.delete(page);
-              return resolve();
-            }
-
-            return reject();
-
-          })
-          .catch((err) => {
-            console.log(err.message); // eslint-disable-line no-console
-            this.setState({ errorMessageForDeleting: err.message });
-            return reject();
-          });
-      });
-    }))
-      .then(() => {
-        window.location.reload();
-      })
-      .catch((err) => {
-        toastr.error(err, 'Error occured', {
-          closeButton: true,
-          progressBar: true,
-          newestOnTop: false,
-          showDuration: '100',
-          hideDuration: '100',
-          timeOut: '3000',
-        });
-      });
-  }
-
-  /**
-   * open confirm modal for page selection delete
-   *
-   * @memberof SearchResult
-   */
-  showDeleteConfirmModal() {
-    this.setState({ isDeleteConfirmModalShown: true });
-  }
-
-  /**
-   * close confirm modal for page selection delete
-   *
-   * @memberof SearchResult
-   */
-  closeDeleteConfirmModal() {
-    this.setState({
-      isDeleteConfirmModalShown: false,
-      errorMessageForDeleting: undefined,
-    });
-  }
-
-  renderListView(pages) {
-    return pages.map((page) => {
-      // Add prefix 'id_' in pageId, because scrollspy of bootstrap doesn't work when the first letter of id attr of target component is numeral.
-      const pageId = `#id_${page._id}`;
-      return (
-        <li key={page._id} className="nav-item page-list-li w-100 m-0 border-bottom">
-          <a className="nav-link page-list-link d-flex align-items-baseline" href={pageId}>
-            <div className="form-check my-auto">
-              <input className="form-check-input my-auto" type="checkbox" value="" id="flexCheckDefault" />
-            </div>
-            <Page page={page} noLink />
-            <div className="ml-auto d-flex">
-              { this.state.deletionMode
-                && (
-                  <div className="custom-control custom-checkbox custom-checkbox-danger">
-                    <input
-                      type="checkbox"
-                      id={`page-delete-check-${page._id}`}
-                      className="custom-control-input search-result-list-delete-checkbox"
-                      value={pageId}
-                      checked={this.state.selectedPages.has(page)}
-                      onChange={() => { return this.toggleCheckbox(page) }}
-                    />
-                    <label className="custom-control-label" htmlFor={`page-delete-check-${page._id}`}></label>
-                  </div>
-                )
-              }
-              <div className="page-list-option">
-                <button type="button" className="btn btn-link p-0" value={page.path} onClick={this.visitPageButtonHandler}><i className="icon-login" /></button>
-              </div>
-            </div>
-          </a>
-          <div>{page.highlight['body.en']?.map(text => <p dangerouslySetInnerHTML={{ __html: text }} />)}</div>
-        </li>
-      );
-    });
-  }
-
-  render() {
-    const { t } = this.props;
-
-    if (this.isError()) {
-      return (
-        <div className="content-main">
-          <i className="searcing fa fa-warning"></i> Error on searching.
-        </div>
-      );
-    }
-
-    if (this.isNotSearchedYet()) {
-      return <div />;
-    }
-
-    if (this.isNotFound()) {
-      let under = '';
-      if (this.props.tree != null) {
-        under = ` under "${this.props.tree}"`;
-      }
-      return (
-        <div className="content-main">
-          <i className="icon-fw icon-info" /> No page found with &quot;{this.props.searchingKeyword}&quot;{under}
-        </div>
-      );
-
-    }
-
-    let deletionModeButtons = '';
-    let allSelectCheck = '';
-
-    if (this.state.deletionMode) {
-      deletionModeButtons = (
-        <div className="btn-group">
-          <button type="button" className="btn btn-outline-secondary btn-sm rounded-pill-weak" onClick={() => { return this.handleDeletionModeChange() }}>
-            <i className="icon-ban" /> {t('search_result.cancel')}
-          </button>
-          <button
-            type="button"
-            className="btn btn-danger btn-sm rounded-pill-weak"
-            onClick={() => { return this.showDeleteConfirmModal() }}
-            disabled={this.state.selectedPages.size === 0}
-          >
-            <i className="icon-trash" /> {t('search_result.delete')}
-          </button>
-        </div>
-      );
-      allSelectCheck = (
-        <div className="custom-control custom-checkbox custom-checkbox-danger">
-          <input
-            id="all-select-check"
-            className="custom-control-input"
-            type="checkbox"
-            onChange={() => { return this.handleAllSelect() }}
-            checked={this.isAllSelected()}
-          />
-          <label className="custom-control-label" htmlFor="all-select-check">&nbsp;{t('search_result.check_all')}</label>
-        </div>
-      );
-    }
-    else {
-      deletionModeButtons = (
-        <div className="btn-group">
-          <button type="button" className="btn btn-outline-secondary rounded-pill btn-sm" onClick={() => { return this.handleDeletionModeChange() }}>
-            <i className="ti-check-box" /> {t('search_result.deletion_mode_btn_lavel')}
-          </button>
-        </div>
-      );
-    }
-
-    const listView = this.renderListView(this.props.pages);
-
-    /*
-    UI あとで考える
-    <span className="search-result-meta">Found: {this.props.searchResultMeta.total} pages with "{this.props.searchingKeyword}"</span>
-    */
-    return (
-      <div className="content-main">
-        <div className="search-result row" id="search-result">
-          <div className="col-lg-6 d-none d-lg-block page-list search-result-list pr-0" id="search-result-list">
-            <nav>
-              <div className="d-flex align-items-start justify-content-between mt-1">
-                <div className="search-result-meta">
-                  <i className="icon-magnifier" /> Found {this.props.searchResultMeta.total} pages with &quot;{this.props.searchingKeyword}&quot;
-                </div>
-                <div className="text-nowrap">
-                  {deletionModeButtons}
-                  {allSelectCheck}
-                </div>
-              </div>
-
-              <div className="page-list">
-                <ul className="page-list-ul page-list-ul-flat nav nav-pills">{listView}</ul>
-              </div>
-            </nav>
-          </div>
-          <div className="col-lg-6 search-result-content" id="search-result-content">
-            <SearchResultList pages={this.props.pages} searchingKeyword={this.props.searchingKeyword} />
-          </div>
-        </div>
-        <DeletePageListModal
-          isShown={this.state.isDeleteConfirmModalShown}
-          pages={Array.from(this.state.selectedPages)}
-          errorMessage={this.state.errorMessageForDeleting}
-          cancel={this.closeDeleteConfirmModal}
-          confirmedToDelete={this.deleteSelectedPages}
-          isDeleteCompletely={this.state.isDeleteCompletely}
-          toggleDeleteCompletely={this.toggleDeleteCompletely}
-        />
-      </div> // content-main
-    );
-  }
-
-}
-
-/**
- * Wrapper component for using unstated
- */
-const SearchResultWrapper = withUnstatedContainers(SearchResult, [AppContainer]);
-
-SearchResult.propTypes = {
-  appContainer: PropTypes.instanceOf(AppContainer).isRequired,
-  t: PropTypes.func.isRequired, // i18next
-
-  pages: PropTypes.array.isRequired,
-  searchingKeyword: PropTypes.string.isRequired,
-  searchResultMeta: PropTypes.object.isRequired,
-  searchError: PropTypes.object,
-  tree: PropTypes.string,
-};
-SearchResult.defaultProps = {
-  searchError: null,
-};
-
-export default withTranslation()(SearchResultWrapper);

+ 3 - 0
packages/app/src/components/SearchPage/SearchResultList.jsx

@@ -6,6 +6,9 @@ class SearchResultList extends React.Component {
 
   render() {
     return this.props.pages.map((page) => {
+      // TODO : send cetain  length of body (revisionBody) from elastisearch by adding some settings to the query and
+      //         when keyword is not in page content, display revisionBody.
+      // TASK : https://estoc.weseek.co.jp/redmine/issues/79606
       return (
         <SearchResultListItem
           page={page}

+ 9 - 2
packages/app/src/components/SearchPage/SearchResultListItem.tsx

@@ -10,10 +10,12 @@ const logger = loggerFactory('growi:searchResultList');
 type Props ={
   page: {
     _id: string,
-    snippet: string,
     path: string,
     noLink: boolean,
     lastUpdateUser: any
+    elasticSearchResult: {
+      snippet: string,
+    }
   },
   onClickInvoked: (data: string) => void,
 }
@@ -28,6 +30,10 @@ const SearchResultListItem: FC<Props> = (props:Props) => {
   const dPagePath = new DevidedPagePath(page.path, false, true);
   const pagePathElem = <PagePathLabel page={page} isFormerOnly />;
 
+  // TODO : send cetain  length of body (revisionBody) from elastisearch by adding some settings to the query and
+  //         when keyword is not in page content, display revisionBody.
+  // TASK : https://estoc.weseek.co.jp/redmine/issues/79606
+
   return (
     <li key={page._id} className="page-list-li w-100 border-bottom pr-4">
       <a
@@ -82,7 +88,8 @@ const SearchResultListItem: FC<Props> = (props:Props) => {
               </button> */}
 
             </div>
-            <div className="mt-1">{page.snippet}</div>
+            {/* eslint-disable-next-line react/no-danger */}
+            <div className="mt-1" dangerouslySetInnerHTML={{ __html: page.elasticSearchResult.snippet }}></div>
           </div>
         </div>
       </a>

+ 8 - 5
packages/app/src/server/routes/search.js

@@ -139,7 +139,9 @@ module.exports = function(crowi, app) {
 
     const result = {};
     try {
-      const esResult = await searchService.searchKeyword(keyword, user, userGroups, searchOpts);
+      const esResult = searchService.formatResult(
+        await searchService.searchKeyword(keyword, user, userGroups, searchOpts),
+      );
 
       // create score map for sorting
       // key: id , value: score
@@ -150,12 +152,13 @@ module.exports = function(crowi, app) {
 
       const ids = esResult.data.map((page) => { return page._id });
       const findResult = await Page.findListByPageIds(ids);
-
-      // add tags and snippet data to result pages
+      // add tags and elasticSearch data to page
       findResult.pages.map((page) => {
-        const data = esResult.data.find((data) => { return page.id === data._id });
+        const data = esResult.data.find((data) => {
+          return page.id === data._id;
+        });
         page._doc.tags = data._source.tag_names;
-        page._doc.snippet = data._highlight;
+        page._doc.elasticSearchResult = data.elasticSearchResult;
         return page;
       });
 

+ 2 - 0
packages/app/src/server/service/search-delegator/elasticsearch.js

@@ -869,6 +869,8 @@ class ElasticsearchDelegator {
         '*': {
           fragment_size: 40,
           fragmenter: 'simple',
+          pre_tags: ["<em class='highlighted-keyword'>"],
+          post_tags: ['</em>'],
         },
       },
     };

+ 28 - 0
packages/app/src/server/service/search.js

@@ -2,6 +2,16 @@ import loggerFactory from '~/utils/logger';
 
 // eslint-disable-next-line no-unused-vars
 const logger = loggerFactory('growi:service:search');
+const xss = require('xss');
+
+// options for filtering xss
+const filterXssOptions = {
+  whiteList: {
+    em: ['class'],
+  },
+};
+
+const filterXss = new xss.FilterXSS(filterXssOptions);
 
 class SearchService {
 
@@ -149,6 +159,24 @@ class SearchService {
     }
   }
 
+  /**
+   * formatting result
+   */
+  formatResult(esResult) {
+    esResult.data.forEach((data) => {
+      const highlightData = data._highlight;
+      const snippet = highlightData['body.en'] || highlightData['body.ja'] || '';
+      const pathMatch = highlightData['path.en'] || highlightData['path.ja'] || '';
+
+      data.elasticSearchResult = {
+        snippet: filterXss.process(snippet),
+        // todo: use filter xss.process() for matchedPath;
+        matchedPath: pathMatch,
+      };
+    });
+    return esResult;
+  }
+
 }
 
 module.exports = SearchService;

+ 3 - 0
packages/app/src/styles/theme/_apply-colors.scss

@@ -591,6 +591,9 @@ body.pathname-sidebar {
 .search-result {
   .search-result-list {
     .page-list {
+      .highlighted-keyword {
+        background-color: $bgcolor-keyword-highlighted;
+      }
       .page-list-ul {
         > li.nav-item > a.nav-link {
           color: inherit;