feat: Add missing fields to EfUser model

eclipsefdn-profile-api#78 (closed)

efservices/src/main/java/org/eclipsefoundation/efservices/api/models/EfUser.java

@@ -23,15 +23,14 @@ import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
 import com.google.auto.value.AutoValue;
 
 /**
- * This model contains all user profile data. It is the entity served by the
- * profile-api. Used to allow access to current user profile information.
- * Provides a method for creating a stubbed bot user.
+ * This model contains all user profile data. It is the entity served by the profile-api. Used to allow access to current user profile
+ * information. Provides a method for creating a stubbed bot user.
  */
 @AutoValue
 @JsonDeserialize(builder = AutoValue_EfUser.Builder.class)
 public abstract class EfUser {
 
-    public abstract Integer getUid();
+    public abstract String getUid();
 
     public abstract String getName();
 
@@ -63,7 +62,7 @@ public abstract class EfUser {
     public abstract String getOrg();
 
     @Nullable
-    public abstract String getOrgId();
+    public abstract Integer getOrgId();
 
     public abstract String getJobTitle();
 
@@ -76,6 +75,18 @@ public abstract class EfUser {
 
     public abstract List<String> getInterests();
 
+    @Nullable
+    public abstract List<String> getWorkingGroupInterests();
+
+    @Nullable
+    public abstract List<String> getPublicFields();
+
+    @Nullable
+    public abstract List<String> getMailHistory();
+
+    @Nullable
+    public abstract List<String> getMailAlternate();
+
     @Nullable
     public abstract String getEcaUrl();
 
@@ -105,8 +116,9 @@ public abstract class EfUser {
      * @return a stubbed Eclipse user bot object.
      */
     public static EfUser createBotStub(String name, String mail) {
-        return EfUser.builder()
-                .setUid(0)
+        return EfUser
+                .builder()
+                .setUid("0")
                 .setName(name)
                 .setMail(mail)
                 .setEca(Eca.builder().build())
@@ -117,11 +129,16 @@ public abstract class EfUser {
                 .setFullName("")
                 .setPublisherAgreements(Collections.emptyMap())
                 .setTwitterHandle("")
-                .setOrg("")
+                .setOrg(null)
                 .setJobTitle("bot")
                 .setWebsite("")
                 .setCountry(Country.builder().build())
-                .setInterests(Collections.emptyList()).build();
+                .setInterests(Collections.emptyList())
+                .setWorkingGroupInterests(Collections.emptyList())
+                .setPublicFields(Collections.emptyList())
+                .setMailHistory(Collections.emptyList())
+                .setMailAlternate(Collections.emptyList())
+                .build();
     }
 
     public static Builder builder() {
@@ -132,7 +149,7 @@ public abstract class EfUser {
     @JsonPOJOBuilder(withPrefix = "set")
     public abstract static class Builder {
 
-        public abstract Builder setUid(Integer uid);
+        public abstract Builder setUid(String uid);
 
         public abstract Builder setName(String name);
 
@@ -158,7 +175,7 @@ public abstract class EfUser {
 
         public abstract Builder setOrg(@Nullable String org);
 
-        public abstract Builder setOrgId(@Nullable String id);
+        public abstract Builder setOrgId(@Nullable Integer id);
 
         public abstract Builder setJobTitle(String title);
 
@@ -170,6 +187,13 @@ public abstract class EfUser {
 
         public abstract Builder setInterests(List<String> interests);
 
+        public abstract Builder setWorkingGroupInterests(@Nullable List<String> interests);
+
+        public abstract Builder setPublicFields(@Nullable List<String> fields);
+
+        public abstract Builder setMailHistory(@Nullable List<String> mail);
+
+        public abstract Builder setMailAlternate(@Nullable List<String> mail);
 
         public abstract Builder setEcaUrl(@Nullable String url);
 
@@ -196,8 +220,7 @@ public abstract class EfUser {
         public abstract boolean getCanContributeSpecProject();
 
         public static Builder builder() {
-            return new AutoValue_EfUser_Eca.Builder().setCanContributeSpecProject(Boolean.FALSE)
-                    .setSigned(Boolean.FALSE);
+            return new AutoValue_EfUser_Eca.Builder().setCanContributeSpecProject(Boolean.FALSE).setSigned(Boolean.FALSE);
         }
 
         @AutoValue.Builder

[Martin Lowe]

This is a breaking change that will be kind of annoying to fix in our various tests. Is there a reason we're switching types here? From what I understand this will always be a number so it shouldn't be a problem

[Zachary Sabourin]

I'm matching the return from D7. It's technically a String and shouldn't have been an int at all

[Martin Lowe]

Technically a string, in that drupal doesn't return non-string types. It is functionally a number in the system as the nid is always a number though. Jackson should be able to handle conversions for us to make the values more accurate if we need it

[Zachary Sabourin]

Drupal can return numbers. org_id is returned as an int from D7.

Ex: https://api.eclipse.org/account/profile/mmilinkovich

[Martin Lowe]

Drupal can return numbers. org_id is returned as an int from D7.

I stand corrected on that part of my point, I hadn't realized that as I haven't looked at the raw of that output in a while. The other point still stands though, the nid from Drupal is a serial column and is still a number in Drupal 9 as well.

Our models don't have to be perfect replicas of the data, just useful representations of them. Does the type switch fix something or improve usability by making this change? If this didn't create as many breaking changes as it does, I wouldn't mind, but this would affect a lot of packages for no gain that I can see

[Zachary Sabourin]

I'd argue that if any consumers of this API would be expecting a string, it will likely break for them when it goes live as an int.

The org id is also changing to an int, wouldn't that also be a breaking change for our other APIs?