Conversation
Add setName, setNotificationSetting, setPermission, and setParentTeamId methods following the existing setDescription and setPrivacy pattern. Each uses PATCH to the team API endpoint. Fixes hub4j#2218
rozza-sb
left a comment
rozza-sb
left a comment
There was a problem hiding this comment.
Some notes on notifications and permissions
| * @throws IOException | ||
| * the io exception | ||
| */ | ||
| public void setNotificationSetting(String notificationSetting) throws IOException { |
There was a problem hiding this comment.
Looking at GHTeamBuilder, this doesn't already exist in this repo as a concept under a team. However, I think this would work better as an enum so that we can only provide valid values to the request
There was a problem hiding this comment.
Feel free to add enum methods and have the string methods call those to validate the string input. Still need the string methods though.
| * @throws IOException | ||
| * the io exception | ||
| */ | ||
| public void setPermission(String permission) throws IOException { |
There was a problem hiding this comment.
GHOrganization.Permission exists for this already, so should be reused here
Addresses review feedback from @rozza-sb on hub4j#2219: - setNotificationSetting now takes a new GHTeam.NotificationSetting enum (NOTIFICATIONS_ENABLED / NOTIFICATIONS_DISABLED) instead of a raw String. The enum overrides toString() to return the exact API values and is passed to .with("notification_setting", ...) as a String so transformEnum() doesn't rewrite the underscores. - setPermission now takes GHOrganization.Permission (ADMIN, MAINTAIN, PULL, PUSH, TRIAGE) so callers can only pass valid values. The enum names lowercase cleanly into the API values the Teams endpoint expects, and reuses the enum that already exists in this package instead of inventing a new team-scoped one.
|
Thanks for the feedback @rozza-sb - addressed in 8dadafe:
No existing callers of either setter outside |
There was a problem hiding this comment.
Each is a single PATCH call to the team endpoint. An atomic multi-property update builder could follow as a separate enhancement if maintainers want one.
Let's just go straight to that multi-property update builder. So, GHLabel for an example of the behavior
Also, of course testing needed .
| public enum NotificationSetting { | ||
|
|
||
| /** Notifications enabled: members receive notifications when the team is @mentioned. */ | ||
| NOTIFICATIONS_ENABLED("notifications_enabled"), |
There was a problem hiding this comment.
Enums must have an UNKNOWN. See other enums such as PRIVACY and their usage.
You probably don't need the string either as Jackson will automatically convert for you.
| * @throws IOException | ||
| * the io exception | ||
| */ | ||
| public void setName(String name) throws IOException { |
There was a problem hiding this comment.
We're moving to not have set methods on objects. I don't see any on here yet...
3 participants
GHTeam currently only has
setDescriptionandsetPrivacy. The GitHub API supports updating several more properties.This adds 4 missing setters, all following the same PATCH pattern as the existing ones:
setName(String name)setNotificationSetting(String notificationSetting)setPermission(String permission)setParentTeamId(Long parentTeamId)(nullable, to support removing parent)Each is a single PATCH call to the team endpoint. An atomic multi-property update builder could follow as a separate enhancement if maintainers want one.
Fixes #2218
This contribution was developed with AI assistance (Codex).