Yicong-Huang commented on code in PR #8151:
URL: https://github.com/apache/texera/pull/8151#discussion_r3937451626
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala:
##########
@@ -152,28 +152,23 @@ object DashboardResource {
searchQueryParams.orderBy match {
case pattern(column, order) =>
val field = getColumnField(column)
- field match {
- case Some(value) =>
- List(order match {
- case "Asc" => value.asc()
- case "Desc" => value.desc().nullsLast()
- })
- case None => List()
- }
- case _ => List() // Default case if the orderBy string doesn't match the
pattern
+ List(order match {
+ case "Asc" => field.asc()
+ case "Desc" => field.desc().nullsLast()
+ })
+ case _ => throw new BadRequestException(s"Unknown orderBy:
${searchQueryParams.orderBy}")
}
}
// Helper method to map column names to actual database fields based on
resource type
Review Comment:
`getColumnField` never keyed on resource type — it maps the order column,
and this PR just rewrote its signature on the line below.
```suggestion
// Maps an order-column name from the orderBy grammar to its
unified-schema field.
```
While you are here: the `@param orderBy` doc at :69 is now the grammar this
endpoint enforces with a 400, and it lists four of the eight values :150
accepts. `ExecutionTimeDesc` is missing, which is the value the dashboard
itself sends (`sort-method.ts:25`).
--
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]
