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

Search in workspace folder #204

Merged
merged 10 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from 8 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
4 changes: 2 additions & 2 deletions backend/app/controllers/api/PagesController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class PagesController(val controllerComponents: AuthControllerComponents, manife
// Get language and highlight data for a given page
def getPageData(uri: Uri, pageNumber: Int, sq: Option[String], fq: Option[String]) = ApiAction.attempt { req =>
for {
response <- frontendPageFromQuery(uri, pageNumber, req.user.username, sq.map(Chips.parseQueryString), fq)
response <- frontendPageFromQuery(uri, pageNumber, req.user.username, sq.map(Chips.parseQueryString(_).query), fq)
} yield {
Ok(Json.toJson(response))
}
Expand Down Expand Up @@ -150,7 +150,7 @@ class PagesController(val controllerComponents: AuthControllerComponents, manife
// It behaves identically to the findInDocument endpoint, except that it expects its query to be in
// a JSON format that may contain chips, and it returns highlight ids with a different prefix.
def searchInDocument(uri: Uri, q: String) = ApiAction.attempt { req =>
getHighlights(uri, Chips.parseQueryString(q), req.user.username, isSearch = true).map(highlights =>
getHighlights(uri, Chips.parseQueryString(q).query, req.user.username, isSearch = true).map(highlights =>
Ok(Json.toJson(highlights))
)
}
Expand Down
10 changes: 5 additions & 5 deletions backend/app/controllers/api/Resource.scala
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ class Resource(val controllerComponents: AuthControllerComponents, manifest: Man
index: Index, pages: Pages, annotations: Annotations, previewStorage: ObjectStorage) extends AuthApiController {

def getResource(uri: Uri, basic: Boolean, q: Option[String]) = ApiAction.attempt { implicit req =>
val query = q.map(Chips.parseQueryString)
val parsedChips = q.map(Chips.parseQueryString)

val resourceFetchMode = (basic, query) match {
val resourceFetchMode = (basic, parsedChips) match {
case (true, _) => ResourceFetchMode.Basic
case (false, text) => ResourceFetchMode.WithData(text)
case (false, pt) => ResourceFetchMode.WithData(pt.map(_.query))
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe pc instead of pt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

parsed tchip

}

val decodedUri = Uri(UriEncoding.decodePath(uri.value, StandardCharsets.UTF_8))
Expand Down Expand Up @@ -75,12 +75,12 @@ class Resource(val controllerComponents: AuthControllerComponents, manifest: Man
}

def getTextPages(uri: Uri, top: Double, bottom: Double, q: Option[String], language: Option[Language]) = ApiAction.attempt { req =>
val query = q.map(Chips.parseQueryString)
val parsedChips = q.map(Chips.parseQueryString)

for {
// Check we have permission to see this file
_ <- GetResource(uri, ResourceFetchMode.Basic, req.user.username, manifest, index, annotations, controllerComponents.users).process()
response <- new GetPages(uri, top, bottom, query, language, pages, previewStorage).process()
response <- new GetPages(uri, top, bottom, parsedChips.map(_.query), language, pages, previewStorage).process()
} yield {
Ok(Json.toJson(response))
}
Expand Down
20 changes: 3 additions & 17 deletions backend/app/controllers/api/Search.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ class Search(override val controllerComponents: AuthControllerComponents, userMa

def search() = ApiAction.attempt { req: UserIdentityRequest[_] =>
val q = req.queryString.getOrElse("q", Seq("")).head
val maybeWorkspaceContextParams = Search.parseWorkspaceContextParams(req)
val proposedParams = Search.buildSearchParameters(q, req)

buildSearch(req.user, proposedParams, maybeWorkspaceContextParams).flatMap { case (verifiedParams, context) =>
buildSearch(req.user, proposedParams, proposedParams.workspaceContext).flatMap { case (verifiedParams, context) =>
val returnEmptyResult = Search.shouldReturnEmptyResult(proposedParams, verifiedParams, context)

if(returnEmptyResult) {
Expand Down Expand Up @@ -96,9 +95,9 @@ object Search extends Logging {
val createdAtFilters = req.queryString.getOrElse("createdAt[]", Seq()).toList
val (start, end) = parseCreatedAt(createdAtFilters)

val query = Chips.parseQueryString(q)
val parsedChips = Chips.parseQueryString(q)

SearchParameters(query, mimeFilters, ingestionFilters, workspaceFilters, start, end, page, pageSize, sortBy)
SearchParameters(parsedChips.query, mimeFilters, ingestionFilters, workspaceFilters, start, end, page, pageSize, sortBy, parsedChips.workspace)
}

def verifyParameters(user: User, params: SearchParameters, context: DefaultSearchContext): SearchParameters = {
Expand Down Expand Up @@ -183,19 +182,6 @@ object Search extends Logging {
}
}

def parseWorkspaceContextParams(req: RequestHeader): Option[WorkspaceSearchContextParams] = {
val maybeWorkspaceId = req.getQueryString("workspaceId")
val maybeWorkspaceFolderId = req.getQueryString("workspaceFolderId")

(maybeWorkspaceId, maybeWorkspaceFolderId) match {
case (Some(workspaceId), Some(workspaceFolderId)) =>
Some(WorkspaceSearchContextParams(workspaceId, workspaceFolderId))

case _ =>
None
}
}

private def epochTs(instant: LocalDateTime): Long = {
instant.toInstant(ZoneOffset.UTC).toEpochMilli
}
Expand Down
29 changes: 26 additions & 3 deletions backend/app/model/frontend/Chip.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package model.frontend

import play.api.libs.json._
import services.index.WorkspaceSearchContextParams

case class DropdownOption(label: String, value: String)

Expand All @@ -16,6 +17,12 @@ case class DateChip(name: String, template: String) extends Chip
case class ExclusiveDateChip(name: String, template: String) extends Chip
case class DropdownChip(name: String, options: List[DropdownOption], template: String) extends Chip

case class WorkspaceFolderChip(name: String, t: String, workspaceId: String, folderId: String)

object WorkspaceFolderChip {
implicit val workspaceFolderChip = Json.format[WorkspaceFolderChip]
}

object Chip {
private val plainChipFormat = Json.format[TextChip]
private val dateChipFormat = Json.format[DateChip]
Expand Down Expand Up @@ -46,14 +53,29 @@ object Chip {
}
}

case class ParsedChips (query: String, workspace: Option[WorkspaceSearchContextParams])

object Chips {
// TODO - when the index supports attempts we can uncomment these lines to make this attempty
//def parseQueryString(q: String): Attempt[String] = {
def parseQueryString(q: String): String = {
def parseQueryString(q: String): ParsedChips = {
//Attempt.catchNonFatal {
val parsedQ = Json.parse(q)
// find WorkspaceSearchContextParams
Copy link
Contributor

Choose a reason for hiding this comment

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

it might be worth adding a slightly more detailed comment here explaning what's happening - i.e extracting the workspace parameters, then ignoring them in the next bit

val workspaceFolder = parsedQ match {
case JsArray(value) => value.collectFirst {
case JsObject(o) if o.get("t").map(_.validate[String].get).get == "workspace_folder" =>
WorkspaceSearchContextParams(o.get("workspaceId").map(_.validate[String].get).get, o.get("folderId").map(_.validate[String].get).get)
}
case _ => None
}

Json.parse(q) match {
case JsArray(v) => v.toList.map {
val query = parsedQ match {
case JsArray(v) => v.toList.filter {
// remove workspace_folder chips
case JsObject(o) if o.get("t").map(_.validate[String].get).get == "workspace_folder" => false
case _ => true
}.map {
// When typing a new chip, we end up with a dangling + which is illegal in the ES query syntax.
// This doesn't matter if you start a chip before an existing term or in between two existing.
// In that case it will be parsed as the boolean operator attached to the subsequent term.
Expand All @@ -78,6 +100,7 @@ object Chips {
}.mkString(" ")
case _ => throw new UnsupportedOperationException("Outer json type must be an array")
}
ParsedChips(query, workspaceFolder)
// } {
// case s: Exception => ClientFailure(s"Invalid query: ${s.getMessage}")
//}
Expand Down
6 changes: 5 additions & 1 deletion backend/app/model/index/SearchParameters.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package model.index

import services.index.WorkspaceSearchContextParams

trait SortBy
case object Relevance extends SortBy
case object SizeAsc extends SortBy
Expand All @@ -15,7 +17,9 @@ case class SearchParameters(q: String,
end: Option[Long],
page: Int,
pageSize: Int,
sortBy: SortBy) {
sortBy: SortBy,
workspaceContext: Option[WorkspaceSearchContextParams] = None
) {
def from: Int = (page - 1) * pageSize
def size: Int = pageSize
}
22 changes: 15 additions & 7 deletions backend/test/controllers/api/WorkspaceSearchITest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import test.integration.{ElasticsearchTestService, ItemIds, Neo4jTestService}
import scala.concurrent.ExecutionContext
import scala.concurrent.duration._
import org.scalatest.funsuite.AnyFunSuite
import model.frontend.WorkspaceFolderChip._
import model.frontend.WorkspaceFolderChip
import play.api.libs.json._

class WorkspaceSearchITest extends AnyFunSuite with Neo4jTestService with ElasticsearchTestService with BeforeAndAfterEach {
final implicit override def patience: PatienceConfig = PatienceConfig(Span(30, Seconds), Span(250, Millis))
Expand Down Expand Up @@ -65,7 +68,8 @@ class WorkspaceSearchITest extends AnyFunSuite with Neo4jTestService with Elasti
test("Barry can't search inside a folder in Paul's workspace") {
asUser("barry") { controllers =>
val folderId = itemIds.f
val request = FakeRequest("GET", s"""/query?q=["*"]&workspaceId=${paulWorkspace.id}&workspaceFolderId=${folderId}""")
val q = Json.toJson(WorkspaceFolderChip("workspace", "workspace_folder", paulWorkspace.id, folderId))
val request = FakeRequest("GET", s"""/query?q=[${q},"*"]""")

val results = contentAsJson(controllers.search.search().apply(request)).as[SearchResults]
// TODO: This is only 0 because Barry cannot see anything at all (he has no workspaces or collections of his own).
Expand All @@ -78,7 +82,8 @@ class WorkspaceSearchITest extends AnyFunSuite with Neo4jTestService with Elasti
test("Paul can search in a folder in his workspace") {
asUser("paul") { controllers =>
val folderId = itemIds.`f/h`
val request = FakeRequest("GET", s"""/query?q=["*"]&workspaceId=${paulWorkspace.id}&workspaceFolderId=${folderId}""")
val q = Json.toJson(WorkspaceFolderChip("workspace", "workspace_folder", paulWorkspace.id, folderId))
val request = FakeRequest("GET", s"""/query?q=[${q},"*"]""")

val results = contentAsJson(controllers.search.search().apply(request)).as[SearchResults].results
results should have length(1)
Expand All @@ -90,7 +95,8 @@ class WorkspaceSearchITest extends AnyFunSuite with Neo4jTestService with Elasti
test("Paul can see results from nested folders in his workspace") {
asUser("paul") { controllers =>
val folderId = itemIds.`f`
val request = FakeRequest("GET", s"""/query?q=["*"]&workspaceId=${paulWorkspace.id}&workspaceFolderId=${folderId}""")
val q = Json.toJson(WorkspaceFolderChip("workspace", "workspace_folder", paulWorkspace.id, folderId))
val request = FakeRequest("GET", s"""/query?q=[${q},"*"]""")

val results = contentAsJson(controllers.search.search().apply(request)).as[SearchResults].results

Expand All @@ -105,8 +111,8 @@ class WorkspaceSearchITest extends AnyFunSuite with Neo4jTestService with Elasti

asUser("paul") { controllers =>
val folderId = itemIds.`f/g`
val request = FakeRequest("GET", s"""/query?q=["*"]&workspaceId=${paulWorkspace.id}&workspaceFolderId=${folderId}""")

val q = Json.toJson(WorkspaceFolderChip("workspace", "workspace_folder", paulWorkspace.id, folderId))
val request = FakeRequest("GET", s"""/query?q=[${q},"*"]""")
val results = contentAsJson(controllers.search.search().apply(request)).as[SearchResults].results

results.map(_.uri) should contain only(
Expand All @@ -125,7 +131,8 @@ class WorkspaceSearchITest extends AnyFunSuite with Neo4jTestService with Elasti
itemId = itemIds.`f/h/1.txt`.nodeId
)

val request = FakeRequest("GET", s"""/query?q=["*"]&workspaceId=${paulWorkspace.id}&workspaceFolderId=${folderId}""")
val q = Json.toJson(WorkspaceFolderChip("workspace", "workspace_folder", paulWorkspace.id, folderId))
val request = FakeRequest("GET", s"""/query?q=[${q},"*"]""")
val results = contentAsJson(controllers.search.search().apply(request)).as[SearchResults]

results.hits should be(0)
Expand All @@ -139,7 +146,8 @@ class WorkspaceSearchITest extends AnyFunSuite with Neo4jTestService with Elasti

asUser("barry") { implicit controllers =>
val folderId = itemIds.`f/h`
val request = FakeRequest("GET", s"""/query?q=["*"]&workspaceId=${paulWorkspace.id}&workspaceFolderId=${folderId}""")
val q = Json.toJson(WorkspaceFolderChip("workspace", "workspace_folder", paulWorkspace.id, folderId))
val request = FakeRequest("GET", s"""/query?q=[${q},"*"]""")

val results = contentAsJson(controllers.search.search().apply(request)).as[SearchResults].results
results should have length(1)
Expand Down
53 changes: 52 additions & 1 deletion frontend/src/js/components/UtilComponents/InputSupper/Chip.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ export default class Chip extends React.Component {
}

onNegateClicked = () => {
this.props.onNegateClicked(this.props.index);
if (this.props.type !== 'workspace_folder')
this.props.onNegateClicked(this.props.index);
}

refHandler = (element) => {
Expand All @@ -95,6 +96,8 @@ export default class Chip extends React.Component {
switch (this.props.type) {
case 'text':
return <InputChip ref={this.refHandler} value={this.props.value} onChange={this.onChange} onKeyDown={this.onKeyDownText} onFocus={this.onFocus}/>;
case 'workspace_folder':
return <WorkspaceFolderChip ref={this.refHandler} value={this.props.value} onChange={this.onChange} onKeyDown={this.onKeyDownDropdown} onFocus={this.onFocus}/>
case 'date':
return <DateChip ref={this.refHandler} value={this.props.value} onChange={this.onChange} onKeyDown={this.onKeyDownText} dateMode='from_start'/>;
case 'date_ex':
Expand Down Expand Up @@ -159,6 +162,54 @@ class UnknownChip extends React.Component {
}
}

class WorkspaceFolderChip extends React.Component {
static propTypes = {
value: PropTypes.string.isRequired,
onFocus: PropTypes.func.isRequired,
onChange: PropTypes.func.isRequired,
onKeyDown: PropTypes.func.isRequired
}

focus = () => {
this.textInput.focus();
}

focusEnd = () => {
this.textInput.focus();
this.textInput.input.selectionStart = this.textInput.input.selectionEnd = this.textInput.input.value.length;
}

select = () => {
this.textInput.select();
}

onChange = (e) => {
this.props.onChange(e.target.value);
}

onKeyDown = (e) => {
const inputStart = this.textInput.input.selectionStart;
const inputEnd = this.textInput.input.selectionEnd;

this.props.onKeyDown(e, inputStart, inputEnd);
}

render() {
return (
<AutosizeInput ref={i => this.textInput = i}
type='text'
inputClassName='input-supper__inline-input input-supper__inline-input--chip'
value={this.props.value}
onChange={this.onChange}
onKeyDown={this.onKeyDown}
onFocus={this.props.onFocus}
disabled={true}
/>
);
}

}

class InputChip extends React.Component {
static propTypes = {
value: PropTypes.string.isRequired,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ export default class InputSupper extends React.Component {
name: e.n,
value: e.v,
negate: e.op === '-',
chipType: e.t
chipType: e.t,
workspaceId: e.workspaceId,
folderId: e.folderId,
};
}

Expand Down Expand Up @@ -125,7 +127,9 @@ export default class InputSupper extends React.Component {
n: e.name,
v: e.value,
op: e.negate ? '-' : '+',
t: e.chipType
t: e.chipType,
workspaceId: e.workspaceId,
folderId: e.folderId
};
}
return '';
Expand Down
27 changes: 26 additions & 1 deletion frontend/src/js/components/workspace/Workspaces.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ import { reprocessBlob } from '../../actions/workspaces/reprocessBlob';
import { DeleteModal, DeleteStatus } from './DeleteModal';
import { PartialUser } from '../../types/User';
import { getMyPermissions } from '../../actions/users/getMyPermissions';
import buildLink from '../../util/buildLink';
import history from '../../util/history';


type Props = ReturnType<typeof mapStateToProps>
Expand Down Expand Up @@ -542,7 +544,7 @@ class WorkspacesUnconnected extends React.Component<Props, State> {
{key : "copyFilePath", content: copyFilePathContent, icon: "copy"},
// or 'pencil alternate'
{ key: "rename", content: "Rename", icon: "pen square" },
{ key: "remove", content: "Remove from workspace", icon: "trash" }
{ key: "remove", content: "Remove from workspace", icon: "trash" },
];

if (entry.data.addedBy.username === currentUser.username && isWorkspaceLeaf(entry.data)) {
Expand All @@ -551,6 +553,8 @@ class WorkspacesUnconnected extends React.Component<Props, State> {

if (isWorkspaceLeaf(entry.data)){
items.push({ key: "reprocess", content: "Reprocess source file", icon: "redo" });
} else {
items.push({ key: "search", content: "Search in folder", icon: "search" })
}

return <DetectClickOutside onClickOutside={this.closeContextMenu}>
Expand Down Expand Up @@ -594,6 +598,27 @@ class WorkspacesUnconnected extends React.Component<Props, State> {
if (menuItemProps.content === 'Reprocess source file' && (isWorkspaceLeaf(entry.data))) {
this.props.reprocessBlob(workspaceId, entry.data.uri)
}

if (menuItemProps.content === "Search in folder"){
history.push(
buildLink("/search", {
q: JSON.stringify([
"",
{
n: "Workspace Folder",
v: entry.name,
op: "+",
t: "workspace_folder",
workspaceId: workspace.id,
folderId: entry.id,
},
"*"
Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking this might have been a mistake but actually a good shout to start with * so that there are documents visible straight away before the user adds a search

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep that's what I was thinking. Still want to test this in code with a workspace with loads of documents to see if it slows down the initial folder search.

]),
page: 1
}),
)
}

setTimeout(() => this.closeContextMenu(), closeMenuDelay);
}}
/>
Expand Down
Loading