Yicong-Huang commented on code in PR #7464:
URL: https://github.com/apache/texera/pull/7464#discussion_r3870645713
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala:
##########
@@ -680,99 +633,92 @@ class WorkflowResourceSpec
)
}
- "/search API" should "be able to search for resources in different tables"
in {
+ "/search API" should "be able to search for resources by keyword" in {
- // create different types of resources, project, workflow, and file
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
// search
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return all resources when no keyword provided" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray(""))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return multiple matching resources from a single resource type"
in {
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
- projectResource.createProject(sessionUser1, "common project1")
- projectResource.createProject(sessionUser1, "common project2")
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("common"))
+ SearchQueryParams(getKeywordsArray("test"))
)
assert(DashboardClickableFileEntryList.results.length == 2)
}
it should "handle multiple keywords correctly" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("test", "project1"))
+ SearchQueryParams(getKeywordsArray("test", "workflow1"))
)
assert(
DashboardClickableFileEntryList.results.length == 1
- ) // should only return the project
+ ) // should only return test_workflow1
}
it should "filter results by different resourceType" in {
Review Comment:
The fixture seeds three workflows and no datasets, which weakens what this
test can prove. The `all` (== 3) and `workflow` (== 3) assertions hold whether
or not `resourceType` filters at all. The `dataset` assertion (`isEmpty`) only
rules out a filter that is ignored entirely; it never shows the dataset path
returns the right rows.
Seeding one dataset alongside the workflows makes the split discriminate
positively: `all` == 4, `workflow` == 3, `dataset` == 1.
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchema.scala:
##########
@@ -130,16 +122,10 @@ object UnifiedResourceSchema {
* Attributes specific to workflows:
* - `wid`: Workflow ID, as an `Integer`.
* - `workflowUserAccess`: Access privileges associated with the workflow, as
a `PrivilegeEnum`.
- * - `projectsOfWorkflow`: IDs of projects associated with the workflow,
concatenated as a `String`.
* - `uid`: User ID associated with the workflow, as an `Integer`.
* - `userName`: Name of the user associated with the workflow, as a `String`.
* - `userEmail`: Email of the user associated with the workflow, as a
`String`.
*
- * Attributes specific to projects:
- * - `pid`: Project ID, as an `Integer`.
- * - `projectOwnerId`: ID of the project owner, as an `Integer`.
- * - `projectColor`: Color associated with the project, as a `String`.
- *
* Attributes specific to files:
Review Comment:
This "Attributes specific to files" block documents five slots that the
current 20-slot constructor no longer has. The class documentation is already
being edited in this PR. Dropping the stale block here keeps the doc and the
constructor in step, rather than leaving a second cleanup for later.
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala:
##########
@@ -54,15 +53,14 @@ object DashboardResource {
The following class describe the available params from the frontend for
full text search.
* @param user The authenticated user performing the search.
* @param keywords A list of search keywords. The API will return
resources that match any of these keywords.
- * @param resourceType The type of the resources to include in the
search results. Acceptable values are "workflow", "project", "file" and "" (for
all types).
+ * @param resourceType The type of the resources to include in the
search results. Acceptable values are "workflow", "file" and "" (for all types).
Review Comment:
This rewritten line still advertises `"file"` as an acceptable
`resourceType` and omits `"dataset"`. Those are exactly backwards: the doc now
names the unsupported type and hides the supported one. Since the line is being
edited anyway, listing `"workflow"`, `"dataset"` and `""` costs nothing.
Same Scaladoc block, line 178: "allows to specify" should read "allows
specifying".
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/SearchQueryBuilder.scala:
##########
@@ -39,12 +39,11 @@ object SearchQueryBuilder {
// DATASET_RESOURCE_TYPE aliases ResourceType.Dataset, whose value doubles
as the leading
// segment of a storage logical path, so renaming that prefix silently
changes search results
// with no compile error here.
- // TODO: give every resource type (file, workflow, project, dataset, model)
a single shared
+ // TODO: give every resource type (file, workflow, dataset, model) a single
shared
// constant/enum instead of these raw strings, so the search contract
has one source of
// truth and no longer borrows an unrelated storage constant.
val FILE_RESOURCE_TYPE = "file"
Review Comment:
`FILE_RESOURCE_TYPE` has no referents left anywhere in the tree — the file
resource type it named is gone, and this PR removes the last builder that could
have used it. Worth deleting in the same cleanup rather than leaving a constant
that reads as a supported type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
