You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "jaeyun0503 (via GitHub)" <gi...@apache.org> on 2025/12/06 09:03:24 UTC
[PR] feat: add affiliation attribute to user [texera]
jaeyun0503 opened a new pull request, #4113:
URL: https://github.com/apache/texera/pull/4113
<!--
Thanks for sending a pull request (PR)! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
[Contributing to Texera](https://github.com/apache/texera/blob/main/CONTRIBUTING.md)
2. Ensure you have added or run the appropriate tests for your PR
3. If the PR is work in progress, mark it a draft on GitHub.
4. Please write your PR title to summarize what this PR proposes, we
are following Conventional Commits style for PR titles as well.
5. Be sure to keep the PR description updated to reflect all changes.
-->
### What changes were proposed in this PR?
<!--
Please clarify what changes you are proposing. The purpose of this section
is to outline the changes. Here are some tips for you:
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
3. If it is a refactoring, clarify what has been changed.
3. It would be helpful to include a before-and-after comparison using
screenshots or GIFs.
4. Please consider writing useful notes for better and faster reviews.
-->
### Any related issues, documentation, discussions?
<!--
Please use this section to link other resources if not mentioned already.
1. If this PR fixes an issue, please include `Fixes #1234`, `Resolves #1234`
or `Closes #1234`. If it is only related, simply mention the issue number.
2. If there is design documentation, please add the link.
3. If there is a discussion in the mailing list, please add the link.
-->
### How was this PR tested?
<!--
If tests were added, say they were added here. Or simply mention that if the PR
is tested with existing test cases. Make sure to include/update test cases that
check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how
you tested step by step, ideally copy and paste-able, so that other reviewers can
test and check, and descendants can verify in the future. If tests were not added,
please describe why they were not added and/or why it was difficult to add.
-->
### Was this PR authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this PR,
please include the phrase: 'Generated-by: ' followed by the name of the tool
and its version. If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add affiliation attribute to user [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #4113:
URL: https://github.com/apache/texera/pull/4113#issuecomment-3640083985
@aicam Please review it.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add affiliation attribute to user [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on code in PR #4113:
URL: https://github.com/apache/texera/pull/4113#discussion_r2621095644
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/UserResource.scala:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.resource.dashboard.user
+
+import org.apache.texera.dao.SqlServer
+import org.apache.texera.dao.jooq.generated.tables.daos.UserDao
+import org.apache.texera.dao.jooq.generated.tables.User.USER
+import javax.ws.rs._
+import javax.ws.rs.core.{MediaType, Response}
+
+case class AffiliationUpdateRequest(uid: Int, affiliation: String)
+
+object UserResource {
Review Comment:
I realized that all endpoints related to registration are defined in `amber/src/main/scala/org/apache/texera/web/resource/GmailResource.scala`, please move them to this file to unify all user resources
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResource.scala:
##########
@@ -78,7 +79,8 @@ class AdminUserResource {
USER.GOOGLE_AVATAR,
USER.COMMENT,
USER_LAST_ACTIVE_TIME.LAST_ACTIVE_TIME,
- USER.ACCOUNT_CREATION_TIME
+ USER.ACCOUNT_CREATION_TIME,
+ USER.AFFILIATION
Review Comment:
Is there any way to make this value optional with default empty? currently all usages of User is updated in this PR simply because a new field is added which is actually optional
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add affiliation attribute to user [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on PR #4113:
URL: https://github.com/apache/texera/pull/4113#issuecomment-3634808827
@jaeyun0503 Could you please move the field to the already existing modal of `send request` when user register? this way admin can see the affiliation to know if they should approve or not
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add affiliation attribute to user [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam merged PR #4113:
URL: https://github.com/apache/texera/pull/4113
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add affiliation attribute to user [texera]
Posted by "jaeyun0503 (via GitHub)" <gi...@apache.org>.
jaeyun0503 commented on code in PR #4113:
URL: https://github.com/apache/texera/pull/4113#discussion_r2636258515
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/UserResource.scala:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.resource.dashboard.user
+
+import org.apache.texera.dao.SqlServer
+import org.apache.texera.dao.jooq.generated.tables.daos.UserDao
+import org.apache.texera.dao.jooq.generated.tables.User.USER
+import javax.ws.rs._
+import javax.ws.rs.core.{MediaType, Response}
+
+case class AffiliationUpdateRequest(uid: Int, affiliation: String)
+
+object UserResource {
Review Comment:
I feel like my code does not really belong to `GmailResource.scala` as it is really nothing related to Gmail, so I thought making a new file named `UserResource.scala` would be a better design
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add affiliation attribute to user [texera]
Posted by "jaeyun0503 (via GitHub)" <gi...@apache.org>.
jaeyun0503 commented on PR #4113:
URL: https://github.com/apache/texera/pull/4113#issuecomment-3634872611
> @jaeyun0503 Could you please move the field to the already existing modal of `send request` when user register? this way admin can see the affiliation to know if they should approve or not
I think this is a design question whether to use affiliation for one of the two purposes:
1. To identify the affiliation of users who want to use our system, or
2. To just gather more information from people who are using Texera.
If option 1, then we should go with your implementation so that every time when a new user comes into our system, the admin user can see their affiliation to decide whether to approve or not. If we choose option 2, then I think my implementation works fine since the affiliation part is collected once people are approved into the system. I think we should ask Professor Li to see which option seems better for our system.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add affiliation attribute to user [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #4113:
URL: https://github.com/apache/texera/pull/4113#issuecomment-3635355117
My general thought is that each user can provide their affiliation optionally, as some of them prefer not to provide such information. If the design is still not clear, we can do a short meeting and report the decision here.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add affiliation attribute to user [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on code in PR #4113:
URL: https://github.com/apache/texera/pull/4113#discussion_r2637273947
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/UserResource.scala:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.resource.dashboard.user
+
+import org.apache.texera.dao.SqlServer
+import org.apache.texera.dao.jooq.generated.tables.daos.UserDao
+import org.apache.texera.dao.jooq.generated.tables.User.USER
+import javax.ws.rs._
+import javax.ws.rs.core.{MediaType, Response}
+
+case class AffiliationUpdateRequest(uid: Int, affiliation: String)
+
+object UserResource {
Review Comment:
I agree, your file `UserResource.scala` is more general, so I suggest move Gmail endpoints to yours because they share the same logic
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add affiliation attribute to user [texera]
Posted by "jaeyun0503 (via GitHub)" <gi...@apache.org>.
jaeyun0503 commented on PR #4113:
URL: https://github.com/apache/texera/pull/4113#issuecomment-3638560761
> My general thought is that each user can provide their affiliation optionally, as some of them prefer not to provide such information. If the design is still not clear, we can do a short meeting and report the decision here.
That aligns with my current design where users can optionally enter the affiliation at the very first time they log into the system. I think this PR is good for review.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org