diff --git a/app/components/CollectionDescription.tsx b/app/components/CollectionDescription.tsx index 68818c5d4..278fdb62a 100644 --- a/app/components/CollectionDescription.tsx +++ b/app/components/CollectionDescription.tsx @@ -1,3 +1,4 @@ +import debounce from "lodash/debounce"; import { observer } from "mobx-react"; import { transparentize } from "polished"; import * as React from "react"; @@ -9,7 +10,6 @@ import ButtonLink from "~/components/ButtonLink"; import Editor from "~/components/Editor"; import LoadingIndicator from "~/components/LoadingIndicator"; import NudeButton from "~/components/NudeButton"; -import useDebouncedCallback from "~/hooks/useDebouncedCallback"; import usePolicy from "~/hooks/usePolicy"; import useStores from "~/hooks/useStores"; import useToasts from "~/hooks/useToasts"; @@ -49,21 +49,25 @@ function CollectionDescription({ collection }: Props) { [isExpanded] ); - const handleSave = useDebouncedCallback(async (getValue) => { - try { - await collection.save({ - description: getValue(), - }); - setDirty(false); - } catch (err) { - showToast( - t("Sorry, an error occurred saving the collection", { - type: "error", - }) - ); - throw err; - } - }, 1000); + const handleSave = React.useMemo( + () => + debounce(async (getValue) => { + try { + await collection.save({ + description: getValue(), + }); + setDirty(false); + } catch (err) { + showToast( + t("Sorry, an error occurred saving the collection", { + type: "error", + }) + ); + throw err; + } + }, 1000), + [] + ); const handleChange = React.useCallback( (getValue) => { diff --git a/app/components/Highlight.tsx b/app/components/Highlight.tsx index 40761b17b..612c605bc 100644 --- a/app/components/Highlight.tsx +++ b/app/components/Highlight.tsx @@ -41,10 +41,10 @@ function Highlight({ ); } -const Mark = styled.mark` +export const Mark = styled.mark` background: ${(props) => props.theme.searchHighlight}; border-radius: 2px; - padding: 0 4px; + padding: 0 2px; `; export default Highlight; diff --git a/app/components/Input.tsx b/app/components/Input.tsx index 8d8257d08..c73c8b133 100644 --- a/app/components/Input.tsx +++ b/app/components/Input.tsx @@ -119,6 +119,7 @@ export type Props = React.HTMLAttributes & { onChange?: ( ev: React.ChangeEvent ) => unknown; + innerRef?: React.RefObject; onKeyDown?: (ev: React.KeyboardEvent) => unknown; onFocus?: (ev: React.SyntheticEvent) => unknown; onBlur?: (ev: React.SyntheticEvent) => unknown; @@ -126,7 +127,7 @@ export type Props = React.HTMLAttributes & { @observer class Input extends React.Component { - input = React.createRef(); + input = this.props.innerRef; @observable focused = false; @@ -147,10 +148,6 @@ class Input extends React.Component { } }; - focus() { - this.input.current?.focus(); - } - render() { const { type = "text", diff --git a/app/components/InputSearch.tsx b/app/components/InputSearch.tsx index 1a1fbdca7..a49c7f648 100644 --- a/app/components/InputSearch.tsx +++ b/app/components/InputSearch.tsx @@ -2,7 +2,7 @@ import { SearchIcon } from "outline-icons"; import * as React from "react"; import { useTranslation } from "react-i18next"; import { useTheme } from "styled-components"; -import Input, { Props as InputProps } from "./Input"; +import Input, { Props as InputProps } from "~/components/Input"; type Props = InputProps & { placeholder?: string; @@ -11,7 +11,10 @@ type Props = InputProps & { onKeyDown?: (event: React.KeyboardEvent) => unknown; }; -export default function InputSearch(props: Props) { +function InputSearch( + props: Props, + ref: React.RefObject +) { const { t } = useTranslation(); const theme = useTheme(); const [isFocused, setIsFocused] = React.useState(false); @@ -39,7 +42,10 @@ export default function InputSearch(props: Props) { onBlur={handleBlur} margin={0} labelHidden + innerRef={ref} {...rest} /> ); } + +export default React.forwardRef(InputSearch); diff --git a/app/components/InputSearchPage.tsx b/app/components/InputSearchPage.tsx index 6fec91daf..347dec37f 100644 --- a/app/components/InputSearchPage.tsx +++ b/app/components/InputSearchPage.tsx @@ -8,7 +8,7 @@ import useBoolean from "~/hooks/useBoolean"; import useKeyDown from "~/hooks/useKeyDown"; import { isModKey } from "~/utils/keyboard"; import { searchPath } from "~/utils/routeHelpers"; -import Input from "./Input"; +import Input, { Outline } from "./Input"; type Props = { source: string; @@ -30,7 +30,7 @@ function InputSearchPage({ collectionId, source, }: Props) { - const inputRef = React.useRef(null); + const inputRef = React.useRef(null); const theme = useTheme(); const history = useHistory(); const { t } = useTranslation(); @@ -67,7 +67,7 @@ function InputSearchPage({ return ( { +const ListPlaceHolder = ({ count, className, header, body }: Props) => { return ( {times(count || 2, (index) => ( - - - + + + ))} diff --git a/app/components/PaginatedList.tsx b/app/components/PaginatedList.tsx index da0132f0e..554f8f560 100644 --- a/app/components/PaginatedList.tsx +++ b/app/components/PaginatedList.tsx @@ -15,32 +15,33 @@ import { dateToHeading } from "~/utils/dates"; type Props = WithTranslation & RootStore & { - fetch?: (options: Record | null | undefined) => Promise; + fetch?: ( + options: Record | null | undefined + ) => Promise | undefined; options?: Record; heading?: React.ReactNode; empty?: React.ReactNode; - items: any[]; + loading?: React.ReactElement; + items?: any[]; renderItem: ( item: any, index: number, - composite: CompositeStateReturn + compositeProps: CompositeStateReturn ) => React.ReactNode; renderHeading?: (name: React.ReactElement | string) => React.ReactNode; + onEscape?: (ev: React.KeyboardEvent) => void; }; @observer class PaginatedList extends React.Component { - isInitiallyLoaded = this.props.items.length > 0; - - @observable - isLoaded = false; - @observable isFetchingMore = false; @observable isFetching = false; + fetchCounter = 0; + @observable renderCount: number = DEFAULT_PAGINATION_LIMIT; @@ -70,7 +71,6 @@ class PaginatedList extends React.Component { this.renderCount = DEFAULT_PAGINATION_LIMIT; this.isFetching = false; this.isFetchingMore = false; - this.isLoaded = false; }; fetchResults = async () => { @@ -78,7 +78,9 @@ class PaginatedList extends React.Component { return; } this.isFetching = true; + const counter = ++this.fetchCounter; const limit = DEFAULT_PAGINATION_LIMIT; + const results = await this.props.fetch({ limit, offset: this.offset, @@ -92,9 +94,12 @@ class PaginatedList extends React.Component { } this.renderCount += limit; - this.isLoaded = true; - this.isFetching = false; - this.isFetchingMore = false; + + // only the most recent fetch should end the loading state + if (counter >= this.fetchCounter) { + this.isFetching = false; + this.isFetchingMore = false; + } }; @action @@ -105,7 +110,7 @@ class PaginatedList extends React.Component { } // If there are already cached results that we haven't yet rendered because // of lazy rendering then show another page. - const leftToRender = this.props.items.length - this.renderCount; + const leftToRender = (this.props.items?.length ?? 0) - this.renderCount; if (leftToRender > 1) { this.renderCount += DEFAULT_PAGINATION_LIMIT; @@ -120,20 +125,24 @@ class PaginatedList extends React.Component { }; render() { - const { items, heading, auth, empty, renderHeading } = this.props; + const { items, heading, auth, empty, renderHeading, onEscape } = this.props; let previousHeading = ""; + + const showList = !!items?.length; + const showEmpty = items?.length === 0; const showLoading = - this.isFetching && !this.isFetchingMore && !this.isInitiallyLoaded; - const showEmpty = !items.length && !showLoading; - const showList = - (this.isLoaded || this.isInitiallyLoaded) && !showLoading && !showEmpty; + this.isFetching && !this.isFetchingMore && !showList && !showEmpty; + return ( <> {showEmpty && empty} {showList && ( <> {heading} - + {(composite: CompositeStateReturn) => items.slice(0, this.renderCount).map((item, index) => { const children = this.props.renderItem( @@ -180,11 +189,12 @@ class PaginatedList extends React.Component { )} )} - {showLoading && ( - - - - )} + {showLoading && + (this.props.loading || ( + + + + ))} ); } diff --git a/app/components/PlaceholderText.tsx b/app/components/PlaceholderText.tsx index bf4d6201f..b977e12f7 100644 --- a/app/components/PlaceholderText.tsx +++ b/app/components/PlaceholderText.tsx @@ -4,7 +4,7 @@ import { randomInteger } from "@shared/random"; import Flex from "~/components/Flex"; import { pulsate } from "~/styles/animations"; -type Props = { +export type Props = { header?: boolean; height?: number; minWidth?: number; diff --git a/app/components/Popover.tsx b/app/components/Popover.tsx index 731ad0d1a..2399ab418 100644 --- a/app/components/Popover.tsx +++ b/app/components/Popover.tsx @@ -1,41 +1,50 @@ import * as React from "react"; import { Dialog } from "reakit/Dialog"; -import { Popover as ReakitPopover } from "reakit/Popover"; +import { Popover as ReakitPopover, PopoverProps } from "reakit/Popover"; import styled from "styled-components"; import breakpoint from "styled-components-breakpoint"; import { depths } from "@shared/styles"; import useMobile from "~/hooks/useMobile"; import { fadeAndScaleIn } from "~/styles/animations"; -type Props = { - tabIndex?: number; +type Props = PopoverProps & { + children: React.ReactNode; width?: number; + shrink?: boolean; + tabIndex?: number; }; -const Popover: React.FC = ({ children, width = 380, ...rest }) => { +const Popover: React.FC = ({ + children, + shrink, + width = 380, + ...rest +}) => { const isMobile = useMobile(); if (isMobile) { return ( - {children} + {children} ); } return ( - {children} + + {children} + ); }; -const Contents = styled.div<{ $width?: number }>` +const Contents = styled.div<{ $shrink?: boolean; $width?: number }>` animation: ${fadeAndScaleIn} 200ms ease; transform-origin: 75% 0; background: ${(props) => props.theme.menuBackground}; border-radius: 6px; - padding: 12px 24px; + padding: ${(props) => (props.$shrink ? "6px 0" : "12px 24px")}; max-height: 50vh; overflow-y: scroll; box-shadow: ${(props) => props.theme.menuShadow}; diff --git a/app/components/SearchListItem.tsx b/app/components/SearchListItem.tsx new file mode 100644 index 000000000..fad5500e3 --- /dev/null +++ b/app/components/SearchListItem.tsx @@ -0,0 +1,148 @@ +import { observer } from "mobx-react"; +import * as React from "react"; +import { Link } from "react-router-dom"; +import { CompositeItem } from "reakit/Composite"; +import styled, { css } from "styled-components"; +import breakpoint from "styled-components-breakpoint"; +import Document from "~/models/Document"; +import Highlight, { Mark } from "~/components/Highlight"; +import { hover } from "~/styles"; + +type Props = { + document: Document; + highlight: string; + context: string | undefined; + showParentDocuments?: boolean; + showCollection?: boolean; + showPublished?: boolean; + shareId?: string; + onClick?: React.MouseEventHandler; +}; +const SEARCH_RESULT_REGEX = /]*>(.*?)<\/b>/gi; + +function replaceResultMarks(tag: string) { + // don't use SEARCH_RESULT_REGEX here as it causes + // an infinite loop to trigger a regex inside it's own callback + return tag.replace(/]*>(.*?)<\/b>/gi, "$1"); +} + +function DocumentListItem( + props: Props, + ref: React.RefObject +) { + const { document, highlight, context, shareId, ...rest } = props; + + return ( + + + + + </Heading> + + { + <ResultContext + text={context} + highlight={highlight ? SEARCH_RESULT_REGEX : undefined} + processResult={replaceResultMarks} + /> + } + </Content> + </CompositeItem> + ); +} + +const Content = styled.div` + flex-grow: 1; + flex-shrink: 1; + min-width: 0; +`; + +const DocumentLink = styled(Link)<{ + $isStarred?: boolean; + $menuOpen?: boolean; +}>` + display: flex; + align-items: center; + padding: 6px 12px; + max-height: 50vh; + + &:not(:last-child) { + margin-bottom: 4px; + } + + &:focus-visible { + outline: none; + } + + ${breakpoint("tablet")` + width: auto; + `}; + + &:${hover}, + &:active, + &:focus, + &:focus-within { + background: ${(props) => props.theme.listItemHoverBackground}; + } + + ${(props) => + props.$menuOpen && + css` + background: ${(props) => props.theme.listItemHoverBackground}; + `} +`; + +const Heading = styled.h4<{ rtl?: boolean }>` + display: flex; + justify-content: ${(props) => (props.rtl ? "flex-end" : "flex-start")}; + align-items: center; + height: 18px; + margin-top: 0; + margin-bottom: 0.25em; + overflow: hidden; + white-space: nowrap; + color: ${(props) => props.theme.text}; +`; + +const Title = styled(Highlight)` + max-width: 90%; + overflow: hidden; + text-overflow: ellipsis; + + ${Mark} { + padding: 0; + } +`; + +const ResultContext = styled(Highlight)` + display: block; + color: ${(props) => props.theme.textTertiary}; + font-size: 14px; + margin-top: -0.25em; + margin-bottom: 0.25em; + + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + + ${Mark} { + padding: 0; + } +`; + +export default observer(React.forwardRef(DocumentListItem)); diff --git a/app/components/SearchPopover.tsx b/app/components/SearchPopover.tsx new file mode 100644 index 000000000..86e4a0e8e --- /dev/null +++ b/app/components/SearchPopover.tsx @@ -0,0 +1,197 @@ +import { debounce } from "lodash"; +import { observer } from "mobx-react"; +import * as React from "react"; +import { useTranslation } from "react-i18next"; +import { usePopoverState, PopoverDisclosure } from "reakit/Popover"; +import styled from "styled-components"; +import { depths } from "@shared/styles"; +import Empty from "~/components/Empty"; +import { Outline } from "~/components/Input"; +import InputSearch from "~/components/InputSearch"; +import Placeholder from "~/components/List/Placeholder"; +import PaginatedList from "~/components/PaginatedList"; +import Popover from "~/components/Popover"; +import useStores from "~/hooks/useStores"; +import SearchListItem from "./SearchListItem"; + +type Props = { shareId: string }; + +function SearchPopover({ shareId }: Props) { + const { t } = useTranslation(); + const { documents } = useStores(); + + const popover = usePopoverState({ + placement: "bottom-start", + unstable_offset: [-24, 0], + modal: true, + }); + + const [query, setQuery] = React.useState(""); + const searchResults = documents.searchResults(query); + + const [cachedQuery, setCachedQuery] = React.useState(query); + const [cachedSearchResults, setCachedSearchResults] = React.useState< + Record<string, any>[] | undefined + >(searchResults); + + React.useEffect(() => { + if (searchResults) { + setCachedQuery(query); + setCachedSearchResults(searchResults); + popover.show(); + } + }, [searchResults, query, popover.show]); + + const performSearch = React.useCallback( + async ({ query, ...options }: Record<string, any>) => { + if (query?.length > 0) { + return await documents.search(query, { shareId, ...options }); + } + return undefined; + }, + [documents, shareId] + ); + + const handleSearch = React.useMemo( + () => + debounce(async (event: React.ChangeEvent<HTMLInputElement>) => { + const { value } = event.target; + setQuery(value.trim()); + + // covers edge case: user manually dismisses popover then + // quickly edits input resulting in no change in query + // the useEffect that normally shows the popover will miss it + if (value === cachedQuery) { + popover.show(); + } + + if (!value.length) { + popover.hide(); + } + }, 300), + [popover, cachedQuery] + ); + + const searchInputRef = popover.unstable_referenceRef; + const firstSearchItem = React.useRef<HTMLAnchorElement>(null); + + const handleEscapeList = React.useCallback( + () => searchInputRef?.current?.focus(), + [searchInputRef] + ); + + const handleKeyDown = React.useCallback( + (ev: React.KeyboardEvent<HTMLInputElement>) => { + if (ev.key === "Enter") { + if (searchResults) { + popover.show(); + } + } + + if (ev.key === "ArrowDown" && !ev.shiftKey) { + if (ev.currentTarget.value.length) { + if ( + ev.currentTarget.value.length === ev.currentTarget.selectionStart + ) { + popover.show(); + } + firstSearchItem.current?.focus(); + } + } + + if (ev.key === "ArrowUp") { + if (popover.visible) { + popover.hide(); + if (!ev.shiftKey) { + ev.preventDefault(); + } + } + + if (ev.currentTarget.value) { + if (ev.currentTarget.selectionEnd === 0) { + ev.currentTarget.selectionStart = 0; + ev.currentTarget.selectionEnd = ev.currentTarget.value.length; + ev.preventDefault(); + } + } + } + + if (ev.key === "Escape") { + if (popover.visible) { + popover.hide(); + ev.preventDefault(); + } + } + }, + [popover, searchResults] + ); + + return ( + <> + <PopoverDisclosure {...popover}> + {(props) => { + // props assumes the disclosure is a button, but we want a type-ahead + // so we take the aria props, and ref and ignore the event handlers + return ( + <StyledInputSearch + aria-controls={props["aria-controls"]} + aria-expanded={props["aria-expanded"]} + aria-haspopup={props["aria-haspopup"]} + ref={props.ref} + onChange={handleSearch} + onKeyDown={handleKeyDown} + /> + ); + }} + </PopoverDisclosure> + + <Popover + {...popover} + aria-label={t("Results")} + unstable_autoFocusOnShow={false} + style={{ zIndex: depths.sidebar + 1 }} + shrink + > + <PaginatedList + options={{ query, snippetMinWords: 10, snippetMaxWords: 11 }} + items={cachedSearchResults} + fetch={performSearch} + onEscape={handleEscapeList} + empty={ + <NoResults>{t("No results for {{query}}", { query })}</NoResults> + } + loading={<PlaceholderList count={3} header={{ height: 20 }} />} + renderItem={(item, index, compositeProps) => ( + <SearchListItem + key={item.document.id} + shareId={shareId} + ref={index === 0 ? firstSearchItem : undefined} + document={item.document} + context={item.context} + highlight={cachedQuery} + onClick={popover.hide} + {...compositeProps} + /> + )} + /> + </Popover> + </> + ); +} + +const NoResults = styled(Empty)` + padding: 0 12px; + margin: 6px 0; +`; + +const PlaceholderList = styled(Placeholder)` + padding: 6px 12px; +`; + +const StyledInputSearch = styled(InputSearch)` + ${Outline} { + border-radius: 16px; + } +`; + +export default observer(SearchPopover); diff --git a/app/components/Sidebar/Shared.tsx b/app/components/Sidebar/Shared.tsx index adfeab23c..29800a437 100644 --- a/app/components/Sidebar/Shared.tsx +++ b/app/components/Sidebar/Shared.tsx @@ -2,6 +2,7 @@ import { observer } from "mobx-react"; import * as React from "react"; import styled from "styled-components"; import Scrollable from "~/components/Scrollable"; +import SearchPopover from "~/components/SearchPopover"; import useStores from "~/hooks/useStores"; import { NavigationNode } from "~/types"; import Sidebar from "./Sidebar"; @@ -19,6 +20,9 @@ function SharedSidebar({ rootNode, shareId }: Props) { return ( <Sidebar> <ScrollContainer flex> + <TopSection> + <SearchPopover shareId={shareId} /> + </TopSection> <Section> <DocumentLink index={0} @@ -38,4 +42,12 @@ const ScrollContainer = styled(Scrollable)` padding-bottom: 16px; `; +const TopSection = styled(Section)` + // this weird looking && increases the specificity of the style rule + && { + margin-top: 16px; + margin-bottom: 16px; + } +`; + export default observer(SharedSidebar); diff --git a/app/scenes/Search/Search.tsx b/app/scenes/Search/Search.tsx index e2fc5fa71..a874cc3e3 100644 --- a/app/scenes/Search/Search.tsx +++ b/app/scenes/Search/Search.tsx @@ -100,9 +100,31 @@ class Search extends React.Component<Props> { return this.goBack(); } - if (ev.key === "ArrowDown") { + if (ev.key === "ArrowUp") { + if (ev.currentTarget.value) { + const length = ev.currentTarget.value.length; + const selectionEnd = ev.currentTarget.selectionEnd || 0; + if (selectionEnd === 0) { + ev.currentTarget.selectionStart = 0; + ev.currentTarget.selectionEnd = length; + ev.preventDefault(); + } + } + } + + if (ev.key === "ArrowDown" && !ev.shiftKey) { ev.preventDefault(); + if (ev.currentTarget.value) { + const length = ev.currentTarget.value.length; + const selectionStart = ev.currentTarget.selectionStart || 0; + if (selectionStart < length) { + ev.currentTarget.selectionStart = length; + ev.currentTarget.selectionEnd = length; + return; + } + } + if (this.compositeRef) { const linkItems = this.compositeRef.querySelectorAll( "[href]" @@ -269,7 +291,7 @@ class Search extends React.Component<Props> { render() { const { documents, notFound, t } = this.props; const results = documents.searchResults(this.query); - const showEmpty = !this.isLoading && this.query && results.length === 0; + const showEmpty = !this.isLoading && this.query && results?.length === 0; return ( <Scene textTitle={this.title}> @@ -345,7 +367,7 @@ class Search extends React.Component<Props> { aria-label={t("Search Results")} > {(compositeProps) => - results.map((result) => { + results?.map((result) => { const document = documents.data.get(result.document.id); if (!document) { return null; diff --git a/app/stores/DocumentsStore.ts b/app/stores/DocumentsStore.ts index 43a7644be..53e291ad3 100644 --- a/app/stores/DocumentsStore.ts +++ b/app/stores/DocumentsStore.ts @@ -31,6 +31,7 @@ export type SearchParams = { includeDrafts?: boolean; collectionId?: string; userId?: string; + shareId?: string; }; type ImportOptions = { @@ -41,7 +42,7 @@ export default class DocumentsStore extends BaseStore<Document> { sharedTreeCache: Map<string, NavigationNode | undefined> = new Map(); @observable - searchCache: Map<string, SearchResult[]> = new Map(); + searchCache: Map<string, SearchResult[] | undefined> = new Map(); @observable backlinks: Map<string, string[]> = new Map(); @@ -170,8 +171,8 @@ export default class DocumentsStore extends BaseStore<Document> { return naturalSort(this.inCollection(collectionId), "title"); } - searchResults(query: string): SearchResult[] { - return this.searchCache.get(query) || []; + searchResults(query: string): SearchResult[] | undefined { + return this.searchCache.get(query); } @computed diff --git a/server/migrations/20220328215615-add-shareId-to-search-queries.js b/server/migrations/20220328215615-add-shareId-to-search-queries.js new file mode 100644 index 000000000..9e55adc51 --- /dev/null +++ b/server/migrations/20220328215615-add-shareId-to-search-queries.js @@ -0,0 +1,18 @@ +'use strict'; + +module.exports = { + up: async (queryInterface, Sequelize) => { + await queryInterface.addColumn("search_queries", "shareId", { + type: Sequelize.UUID, + defaultValue: null, + allowNull: true, + references: { + model: "shares", + key: "id" + }, + }); + }, + down: async (queryInterface) => { + await queryInterface.removeColumn("search_queries", "shareId"); + }, +}; diff --git a/server/models/Document.test.ts b/server/models/Document.test.ts index 715a85381..f06a37f27 100644 --- a/server/models/Document.test.ts +++ b/server/models/Document.test.ts @@ -4,6 +4,7 @@ import { buildCollection, buildTeam, buildUser, + buildShare, } from "@server/test/factories"; import { flushdb, seed } from "@server/test/support"; import slugify from "@server/utils/slugify"; @@ -26,7 +27,7 @@ paragraph 2`, const document = await buildDocument({ version: 0, text: `# Heading - + *paragraph*`, }); expect(document.getSummary()).toBe("paragraph"); @@ -174,7 +175,7 @@ describe("#searchForTeam", () => { expect(results[0].document?.id).toBe(document.id); }); - test("should not return search results from private collections", async () => { + test("should not return results from private collections without providing collectionId", async () => { const team = await buildTeam(); const collection = await buildCollection({ permission: null, @@ -189,6 +190,52 @@ describe("#searchForTeam", () => { expect(results.length).toBe(0); }); + test("should return results from private collections when collectionId is provided", async () => { + const team = await buildTeam(); + const collection = await buildCollection({ + permission: null, + teamId: team.id, + }); + await buildDocument({ + teamId: team.id, + collectionId: collection.id, + title: "test", + }); + const { results } = await Document.searchForTeam(team, "test", { + collectionId: collection.id, + }); + expect(results.length).toBe(1); + }); + + test("should return results from document tree of shared document", async () => { + const team = await buildTeam(); + const collection = await buildCollection({ + permission: null, + teamId: team.id, + }); + const document = await buildDocument({ + teamId: team.id, + collectionId: collection.id, + title: "test 1", + }); + await buildDocument({ + teamId: team.id, + collectionId: collection.id, + title: "test 2", + }); + + const share = await buildShare({ + documentId: document.id, + includeChildDocuments: true, + }); + + const { results } = await Document.searchForTeam(team, "test", { + collectionId: collection.id, + share, + }); + expect(results.length).toBe(1); + }); + test("should handle no collections", async () => { const team = await buildTeam(); const { results } = await Document.searchForTeam(team, "test"); diff --git a/server/models/Document.ts b/server/models/Document.ts index efea041dd..6e4838d2f 100644 --- a/server/models/Document.ts +++ b/server/models/Document.ts @@ -1,4 +1,5 @@ import removeMarkdown from "@tommoor/remove-markdown"; +import invariant from "invariant"; import { compact, find, map, uniq } from "lodash"; import randomstring from "randomstring"; import { @@ -38,6 +39,7 @@ import slugify from "@server/utils/slugify"; import Backlink from "./Backlink"; import Collection from "./Collection"; import Revision from "./Revision"; +import Share from "./Share"; import Star from "./Star"; import Team from "./Team"; import User from "./User"; @@ -45,7 +47,7 @@ import View from "./View"; import ParanoidModel from "./base/ParanoidModel"; import Fix from "./decorators/Fix"; -type SearchResponse = { +export type SearchResponse = { results: { ranking: number; context: string; @@ -58,10 +60,13 @@ type SearchOptions = { limit?: number; offset?: number; collectionId?: string; + share?: Share; dateFilter?: DateFilter; collaboratorIds?: string[]; includeArchived?: boolean; includeDrafts?: boolean; + snippetMinWords?: number; + snippetMaxWords?: number; }; const serializer = new MarkdownSerializer(); @@ -436,12 +441,24 @@ class Document extends ParanoidModel { query: string, options: SearchOptions = {} ): Promise<SearchResponse> { - const limit = options.limit || 15; - const offset = options.offset || 0; const wildcardQuery = `${escape(query)}:*`; - const collectionIds = await team.collectionIds(); + const { + snippetMinWords = 20, + snippetMaxWords = 30, + limit = 15, + offset = 0, + } = options; - // If the team has access no public collections then shortcircuit the rest of this + // restrict to specific collection if provided + // enables search in private collections if specified + let collectionIds; + if (options.collectionId) { + collectionIds = [options.collectionId]; + } else { + collectionIds = await team.collectionIds(); + } + + // short circuit if no relevant collections if (!collectionIds.length) { return { results: [], @@ -449,11 +466,25 @@ class Document extends ParanoidModel { }; } - // Build the SQL query to get documentIds, ranking, and search term context + // restrict to documents in the tree of a shared document when one is provided + let documentIds; + + if (options.share?.includeChildDocuments) { + const sharedDocument = await options.share.$get("document"); + invariant(sharedDocument, "Cannot find document for share"); + + const childDocumentIds = await sharedDocument.getChildDocumentIds(); + documentIds = [sharedDocument.id, ...childDocumentIds]; + } + + const documentClause = documentIds ? `"id" IN(:documentIds) AND` : ""; + + // Build the SQL query to get result documentIds, ranking, and search term context const whereClause = ` "searchVector" @@ to_tsquery('english', :query) AND "teamId" = :teamId AND "collectionId" IN(:collectionIds) AND + ${documentClause} "deletedAt" IS NULL AND "publishedAt" IS NOT NULL `; @@ -461,7 +492,7 @@ class Document extends ParanoidModel { SELECT id, ts_rank(documents."searchVector", to_tsquery('english', :query)) as "searchRanking", - ts_headline('english', "text", to_tsquery('english', :query), 'MaxFragments=1, MinWords=20, MaxWords=30') as "searchContext" + ts_headline('english', "text", to_tsquery('english', :query), 'MaxFragments=1, MinWords=:snippetMinWords, MaxWords=:snippetMaxWords') as "searchContext" FROM documents WHERE ${whereClause} ORDER BY @@ -479,6 +510,9 @@ class Document extends ParanoidModel { teamId: team.id, query: wildcardQuery, collectionIds, + documentIds, + snippetMinWords, + snippetMaxWords, }; const resultsQuery = this.sequelize!.query(selectSql, { type: QueryTypes.SELECT, @@ -526,8 +560,12 @@ class Document extends ParanoidModel { query: string, options: SearchOptions = {} ): Promise<SearchResponse> { - const limit = options.limit || 15; - const offset = options.offset || 0; + const { + snippetMinWords = 20, + snippetMaxWords = 30, + limit = 15, + offset = 0, + } = options; const wildcardQuery = `${escape(query)}:*`; // Ensure we're filtering by the users accessible collections. If @@ -580,7 +618,7 @@ class Document extends ParanoidModel { SELECT id, ts_rank(documents."searchVector", to_tsquery('english', :query)) as "searchRanking", - ts_headline('english', "text", to_tsquery('english', :query), 'MaxFragments=1, MinWords=20, MaxWords=30') as "searchContext" + ts_headline('english', "text", to_tsquery('english', :query), 'MaxFragments=1, MinWords=:snippetMinWords, MaxWords=:snippetMaxWords') as "searchContext" FROM documents WHERE ${whereClause} ORDER BY @@ -601,6 +639,8 @@ class Document extends ParanoidModel { query: wildcardQuery, collectionIds, dateFilter, + snippetMinWords, + snippetMaxWords, }; const resultsQuery = this.sequelize!.query(selectSql, { type: QueryTypes.SELECT, diff --git a/server/routes/api/__snapshots__/documents.test.ts.snap b/server/routes/api/__snapshots__/documents.test.ts.snap index 69d711f51..2ac18507a 100644 --- a/server/routes/api/__snapshots__/documents.test.ts.snap +++ b/server/routes/api/__snapshots__/documents.test.ts.snap @@ -38,7 +38,7 @@ Object { exports[`#documents.search should require authentication 1`] = ` Object { "error": "authentication_required", - "message": "Authentication required", + "message": "Authentication error", "ok": false, "status": 401, } diff --git a/server/routes/api/documents.test.ts b/server/routes/api/documents.test.ts index de4b2ab02..24afdde8d 100644 --- a/server/routes/api/documents.test.ts +++ b/server/routes/api/documents.test.ts @@ -433,7 +433,7 @@ describe("#documents.info", () => { id: document.id, }, }); - expect(res.status).toEqual(403); + expect(res.status).toEqual(401); }); it("should require authorization with incorrect token", async () => { @@ -633,7 +633,7 @@ describe("#documents.export", () => { id: document.id, }, }); - expect(res.status).toEqual(403); + expect(res.status).toEqual(401); }); it("should require authorization with incorrect token", async () => { @@ -944,6 +944,59 @@ describe("#documents.search", () => { expect(body.data[0].document.text).toEqual("# Much test support"); }); + it("should return results using shareId", async () => { + const findableDocument = await buildDocument({ + title: "search term", + text: "random text", + }); + + await buildDocument({ + title: "search term", + text: "should not be found", + userId: findableDocument.createdById, + teamId: findableDocument.teamId, + }); + + const share = await buildShare({ + includeChildDocuments: true, + documentId: findableDocument.id, + teamId: findableDocument.teamId, + }); + + const res = await server.post("/api/documents.search", { + body: { + query: "search term", + shareId: share.id, + }, + }); + + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data.length).toEqual(1); + expect(body.data[0].document.id).toEqual(share.documentId); + }); + + it("should not allow search if child documents are not included", async () => { + const findableDocument = await buildDocument({ + title: "search term", + text: "random text", + }); + + const share = await buildShare({ + includeChildDocuments: false, + document: findableDocument, + }); + + const res = await server.post("/api/documents.search", { + body: { + query: "search term", + shareId: share.id, + }, + }); + + expect(res.status).toEqual(400); + }); + it("should return results in ranked order", async () => { const { user } = await seed(); const firstResult = await buildDocument({ @@ -1287,7 +1340,11 @@ describe("#documents.search", () => { }); it("should require authentication", async () => { - const res = await server.post("/api/documents.search"); + const res = await server.post("/api/documents.search", { + body: { + query: "search term", + }, + }); const body = await res.json(); expect(res.status).toEqual(401); expect(body).toMatchSnapshot(); diff --git a/server/routes/api/documents.ts b/server/routes/api/documents.ts index 25d3e8b81..9285cd454 100644 --- a/server/routes/api/documents.ts +++ b/server/routes/api/documents.ts @@ -11,6 +11,7 @@ import { NotFoundError, InvalidRequestError, AuthorizationError, + AuthenticationError, } from "@server/errors"; import auth from "@server/middlewares/authentication"; import { @@ -386,7 +387,7 @@ async function loadDocument({ }: { id?: string; shareId?: string; - user: User; + user?: User; }): Promise<{ document: Document; share?: Share; @@ -396,6 +397,10 @@ async function loadDocument({ let collection; let share; + if (!shareId && !(id && user)) { + throw AuthenticationError(`Authentication or shareId required`); + } + if (shareId) { share = await Share.findOne({ where: { @@ -454,7 +459,7 @@ async function loadDocument({ // If the user has access to read the document, we can just update // the last access date and return the document without additional checks. - const canReadDocument = can(user, "read", document); + const canReadDocument = user && can(user, "read", document); if (canReadDocument) { await share.update({ @@ -519,9 +524,9 @@ async function loadDocument({ if (document.deletedAt) { // don't send data if user cannot restore deleted doc - authorize(user, "restore", document); + user && authorize(user, "restore", document); } else { - authorize(user, "read", document); + user && authorize(user, "read", document); } collection = document.collection; @@ -739,82 +744,133 @@ router.post("documents.search_titles", auth(), pagination(), async (ctx) => { }; }); -router.post("documents.search", auth(), pagination(), async (ctx) => { - const { - query, - includeArchived, - includeDrafts, - collectionId, - userId, - dateFilter, - } = ctx.body; - const { offset, limit } = ctx.state.pagination; - const { user } = ctx.state; - - assertNotEmpty(query, "query is required"); - - if (collectionId) { - assertUuid(collectionId, "collectionId must be a UUID"); - const collection = await Collection.scope({ - method: ["withMembership", user.id], - }).findByPk(collectionId); - authorize(user, "read", collection); - } - - let collaboratorIds = undefined; - - if (userId) { - assertUuid(userId, "userId must be a UUID"); - collaboratorIds = [userId]; - } - - if (dateFilter) { - assertIn( - dateFilter, - ["day", "week", "month", "year"], - "dateFilter must be one of day,week,month,year" - ); - } - - const { results, totalCount } = await Document.searchForUser(user, query, { - includeArchived: includeArchived === "true", - includeDrafts: includeDrafts === "true", - collaboratorIds, - collectionId, - dateFilter, - offset, - limit, - }); - - const documents = results.map((result) => result.document); - - const data = await Promise.all( - results.map(async (result) => { - const document = await presentDocument(result.document); - return { ...result, document }; - }) - ); - - // When requesting subsequent pages of search results we don't want to record - // duplicate search query records - if (offset === 0) { - SearchQuery.create({ - userId: user.id, - teamId: user.teamId, - source: ctx.state.authType, +router.post( + "documents.search", + auth({ + required: false, + }), + pagination(), + async (ctx) => { + const { query, - results: totalCount, - }); + includeArchived, + includeDrafts, + collectionId, + userId, + dateFilter, + shareId, + } = ctx.body; + assertNotEmpty(query, "query is required"); + + const { offset, limit } = ctx.state.pagination; + const snippetMinWords = parseInt(ctx.body.snippetMinWords || 20, 10); + const snippetMaxWords = parseInt(ctx.body.snippetMaxWords || 30, 10); + + // this typing is a bit ugly, would be better to use a type like ContextWithState + // but that doesn't adequately handle cases when auth is optional + const { user }: { user: User | undefined } = ctx.state; + + let teamId; + let response; + + if (shareId) { + const { share, document } = await loadDocument({ + shareId, + user, + }); + + if (!share?.includeChildDocuments) { + throw InvalidRequestError("Child documents cannot be searched"); + } + + teamId = share.teamId; + const team = await Team.findByPk(teamId); + invariant(team, "Share must belong to a team"); + + response = await Document.searchForTeam(team, query, { + includeArchived: includeArchived === "true", + includeDrafts: includeDrafts === "true", + collectionId: document.collectionId, + share, + dateFilter, + offset, + limit, + snippetMinWords, + snippetMaxWords, + }); + } else { + if (!user) { + throw AuthenticationError("Authentication error"); + } + + teamId = user.teamId; + + if (collectionId) { + assertUuid(collectionId, "collectionId must be a UUID"); + const collection = await Collection.scope({ + method: ["withMembership", user.id], + }).findByPk(collectionId); + authorize(user, "read", collection); + } + + let collaboratorIds = undefined; + + if (userId) { + assertUuid(userId, "userId must be a UUID"); + collaboratorIds = [userId]; + } + + if (dateFilter) { + assertIn( + dateFilter, + ["day", "week", "month", "year"], + "dateFilter must be one of day,week,month,year" + ); + } + + response = await Document.searchForUser(user, query, { + includeArchived: includeArchived === "true", + includeDrafts: includeDrafts === "true", + collaboratorIds, + collectionId, + dateFilter, + offset, + limit, + snippetMinWords, + snippetMaxWords, + }); + } + + const { results, totalCount } = response; + const documents = results.map((result) => result.document); + + const data = await Promise.all( + results.map(async (result) => { + const document = await presentDocument(result.document); + return { ...result, document }; + }) + ); + + // When requesting subsequent pages of search results we don't want to record + // duplicate search query records + if (offset === 0) { + SearchQuery.create({ + userId: user?.id, + teamId, + shareId, + source: ctx.state.authType || "app", // we'll consider anything that isn't "api" to be "app" + query, + results: totalCount, + }); + } + + ctx.body = { + pagination: ctx.state.pagination, + data, + policies: user ? presentPolicies(user, documents) : null, + }; } - - const policies = presentPolicies(user, documents); - - ctx.body = { - pagination: ctx.state.pagination, - data, - policies, - }; -}); +); // Deprecated – use stars.create instead router.post("documents.star", auth(), async (ctx) => { diff --git a/shared/i18n/locales/en_US/translation.json b/shared/i18n/locales/en_US/translation.json index 17dc4fe2f..fe195f941 100644 --- a/shared/i18n/locales/en_US/translation.json +++ b/shared/i18n/locales/en_US/translation.json @@ -136,6 +136,8 @@ "Dismiss": "Dismiss", "Back": "Back", "Documents": "Documents", + "Results": "Results", + "No results for {{query}}": "No results for {{query}}", "Logo": "Logo", "Document archived": "Document archived", "Move document": "Move document", diff --git a/shared/styles/theme.ts b/shared/styles/theme.ts index 659bfde4b..d325dcb11 100644 --- a/shared/styles/theme.ts +++ b/shared/styles/theme.ts @@ -14,6 +14,7 @@ const colors = { smokeLight: "#F9FBFC", smokeDark: "#E8EBED", white: "#FFF", + white05: "rgba(255, 255, 255, 0.05)", white10: "rgba(255, 255, 255, 0.1)", white50: "rgba(255, 255, 255, 0.5)", white75: "rgba(255, 255, 255, 0.75)", @@ -169,7 +170,7 @@ export const dark = { placeholder: colors.slateDark, sidebarBackground: colors.veryDarkBlue, sidebarActiveBackground: lighten(0.02, colors.almostBlack), - sidebarControlHoverBackground: "rgba(255,255,255,0.1)", + sidebarControlHoverBackground: colors.white10, sidebarDraftBorder: darken("0.35", colors.slate), sidebarText: colors.slate, backdrop: "rgba(255, 255, 255, 0.3)", @@ -188,7 +189,7 @@ export const dark = { titleBarDivider: darken(0.4, colors.slate), inputBorder: colors.slateDark, inputBorderFocused: colors.slate, - listItemHoverBackground: colors.black50, + listItemHoverBackground: colors.white10, toolbarHoverBackground: colors.slate, toolbarBackground: colors.white, toolbarInput: colors.black10,