fix: Removing unnecessary user info returned from the backend

[carloea2]

What changes were proposed in this PR?

Currently the endpoint /api/workflow/owner_user/?wid=X returns all the information about the user, including name, email, etc. But the frontend only needs the name. This PR limits the information returned from the backend to the user name only.

The main changes are as follow:

1. Change the endpoint name from /api/workflow/owner_user to /api/workflow/owner_name
2. Change the SQL query to only return name as plain text.
3. Change related uses of the endpoint in the frontend to match the new signature.
4. Added a new WorkflowResourceDashboardUserSpec to test this endpoint and support future testing of related endpoints.

Any related issues, documentation, discussions?

No

How was this PR tested?

Tests:

Manually tested:

Was this PR authored or co-authored using generative AI tooling?

No

[carloea2]

@chenlica

[carloea2]

@chenlica please review it at your convenience.

[chenlica]

@aglinxinyuan Please review this PR.

[chenlica]

@aglinxinyuan I reviewed this PR before. Please review it as well.

[aglinxinyuan]

This PR requires major changes. Please don’t introduce features that don’t align with the PR title or with our prior discussion.

Based on our discussion, the agreed plan was “Removing unnecessary user info returned from the backend.” The title accurately reflects that goal. However, this PR significantly modifies the endpoint behavior by making the response dynamic based on the input:

2. Change the endpoint /api/workflow/owner_info to accept a new parameter fields to specify the information needed.
3. Change the SQL query to accept the new request parameter fields and only return allowed fields (e.g., user name).

Those changes were not part of our plan. We should stick to the agreed design: this endpoint should accept only a workflow ID and always return the owner’s name.

If there are additional changes you’d like to propose, please raise them in a GitHub issue and address them in a separate PR. In general, I don’t think those changes are necessary, as this endpoint is intended to return only the name.

[chenlica]

@aglinxinyuan, this content of this PR was suggested by me in a discussion with @carloea2 . I agree with your comments though. We could spllit this PR into two. The first one just removes unnecessary user information, and the second one will include the refactoring in the current PR. What do you think?

[aglinxinyuan]

@aglinxinyuan, this content of this PR was suggested by me in a discussion with @carloea2 . I agree with your comments though. We could spllit this PR into two. The first just remove unnecessary user information, and the second one will include the refactoring in the current PR. What do you think?

Sure. Introducing a fields parameter when the only supported value is name feels unnecessary to me. I don’t have the context from the follow-up discussion, but this would make more sense once there are multiple possible values. We can introduce the fields parameter later, when we actually need that flexibility.

[carloea2]

Got it. Then I will go back to a previous version with only getOwnerName, thank you.

[chenlica]

@aglinxinyuan I do prefer a fields parameter in case in the future we want to return other user information to the frontend.

[aglinxinyuan]

@aglinxinyuan I do prefer a fields parameter in case in the future we want to return other user information to the frontend.

We already have several endpoints that return additional user information. This endpoint lives under the workflow sub-URL and is intended to be used only for workflow-related resources. If we decide to unify all user-related endpoints in the future, it may be helpful to first review and align the existing user-related URLs.

Introducing a fields parameter here could be interpreted as exposing database-level details to the frontend, which ideally should not need to be aware of individual fields. Additionally, hardcoding field names as strings may increase the risk of the frontend and backend becoming out of sync over time.

Would it be reasonable to defer adding this parameter until there is a concrete use case that requires it?

[chenlica]

In this case let's ignore this fields parameter.

@carloea2 If you agree, let's limit this scope of this PR by just returning the user name. We will NOT do another PR to introduce this fields parameter.

[carloea2]

Yes I agree, the changes are available now. Thanks.

[aglinxinyuan]

LGTM!

[carloea2]

Thanks

[carloea2]

@chenlica can you rerun the workflows?

[chenlica]

@carloea2 One check failed. Please take a look.

[carloea2]

MockTexeraDB is not thread-safe, so that is the reason why my test is failing, is that supposed to be like that...?

I can simply move my tests to WorkflowResource, but I think at some time we will need to add new classes that use MockTexeraDB, or is the design to use the same spec for those using DB? I am confused to be honest.

This is what is happening if my understanding of the logs is correct:

The test fails because the embedded Postgres database it relies on is being shut down while the spec is still running: initializeDBAndReplaceDSLContext() starts an EmbeddedPostgres instance and replaces a shared/global SqlServer DSLContext, but shutdownDB() later closes that embedded DB and clears the shared instance. When another suite does the same thing in parallel or in an unlucky order, the WorkflowResource ends up requesting a connection from a DataSource that has already been stopped, triggering the “Error getting connection” failure.