Yicong-Huang commented on code in PR #8144:
URL: https://github.com/apache/texera/pull/8144#discussion_r3890821689


##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala:
##########
@@ -104,7 +104,7 @@ object DashboardResource {
         val q3 = ProjectSearchQueryBuilder.constructQuery(uid, params, 
includePublic)
         val q4 = DatasetSearchQueryBuilder.constructQuery(uid, params, 
includePublic)
         q1.unionAll(q3).unionAll(q4)
-      case _ => throw new IllegalArgumentException(s"Unknown resource type: 
${params.resourceType}")
+      case _ => throw new BadRequestException(s"Unknown resource type: 
${params.resourceType}")

Review Comment:
   The Scaladoc at :57 has drifted from the arm this line just turned into a 
contract: it advertises `"file"` as an acceptable `resourceType` and omits 
`"dataset"`, which is the reverse of what the dispatch does. 
`SearchQueryBuilder.FILE_RESOURCE_TYPE = "file"` still exists as a constant 
with no arm referencing it.
   
   Before this PR the contradiction was between a doc and an unhandled crash; 
now it is between a doc and a deliberate 400, which is what makes it worth a 
line.
   
   Not reachable from the product — `search.component.ts:64` narrows 
`selectedType` to `"workflow" | "dataset" | null` and the typed pages hardcode 
their own values, so only the `SearchService` type unions still carry `"file"`. 
Documentation and a dead constant, no runtime path.



##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/DashboardResourceSpec.scala:
##########
@@ -25,6 +25,7 @@ import 
org.apache.texera.web.resource.dashboard.DashboardResource.SearchQueryPar
 import org.jooq.impl.{DSL => JDSL}
 import org.jooq.{OrderField, SQLDialect}
 
+import javax.ws.rs.BadRequestException

Review Comment:
   This sits above the `org.scalatest.*` imports, splitting the `org.*` group 
in two. The repo groups `java`/`javax` last — see 
`DashboardResource.scala:22-33`, and both sibling specs importing this same 
class (`FeedbackResourceSpec.scala:29-34`, 
`UserConfigResourceSpec.scala:27-30`).
   
   Neither gate catches it: scalafmt is pinned at 2.6.4 with no 
`rewrite.rules`, and `.scalafix.conf` runs only `ProcedureSyntax` + 
`RemoveUnused`.



-- 
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]

Reply via email to