aglinxinyuan commented on code in PR #7055:
URL: https://github.com/apache/texera/pull/7055#discussion_r3694712963
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala:
##########
@@ -321,7 +321,7 @@ object WorkflowExecutionsResource {
WORKFLOW_EXECUTIONS.VID,
WORKFLOW_EXECUTIONS.CUID,
USER.NAME,
- USER.GOOGLE_AVATAR,
+ USER.AVATAR,
Review Comment:
I don't think this one holds — jOOQ is mapping these positionally, not by
name.
`DefaultRecordMapper` only uses parameter names when they're actually in the
bytecode, and there's no `-parameters` in `scalacOptions` (build.sbt sets
`javacOptions` only), so a Scala case class with no no-arg constructor falls to
`ImmutablePOJOMapperWithoutParameterNames`, which is
`constructor.newInstance(record.intoArray())` — pure index.
The behavioural proof is already in this class:
`WorkflowExecutionEntry.completionTime` is fed by
`WORKFLOW_EXECUTIONS.LAST_UPDATE_TIME` at position 9. Those names don't match
under any normalization, and completion times render fine in the
execution-history table today
(`workflow-execution-history.component.html:212`). Same shape in
`AdminUserResource.UserInfo`, where `lastLogin` comes from
`USER_LAST_ACTIVE_TIME.LAST_ACTIVE_TIME`. If the mapper were name-based, both
would have been null since long before this PR.
12 columns, 12 fields, same order — `USER.AVATAR` still lands on
`googleAvatar`.
Same reasoning applies to the comment this PR adds in
`AdminUserResource.list` ("fetchInto matches a case class by field NAME") — the
aliases there are harmless, but the stated rationale is wrong and I'd rather it
not get cargo-culted. @Neilk1021 the spec you asked for is still worth adding,
and it settles this either way.
--
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]
