refactor: remove unused enum values

amustaque97 · amustaque97 · commit 03305752bde1 · 2026-01-07T10:01:21.000+05:30
diff --git a/sync-team/src/github/api/mod.rs b/sync-team/src/github/api/mod.rs
@@ -522,100 +522,22 @@ pub(crate) enum RulesetEnforcement {
 #[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
 pub(crate) struct RulesetBypassActor {
     /// The ID of the actor that can bypass a ruleset.
-    /// - Required for Integration, RepositoryRole, and Team actor types (use Some(ActorId::Id(n)))
-    /// - Must be 1 for OrganizationAdmin (use Some(ActorId::Id(1)))
-    /// - Must be null for DeployKey (use Some(ActorId::Null))
-    /// - Omitted for EnterpriseOwner (use None)
-    #[serde(
-        skip_serializing_if = "Option::is_none",
-        default,
-        deserialize_with = "deserialize_actor_id"
-    )]
-    pub(crate) actor_id: Option<ActorId>,
+    /// Required for Team and Integration actor types.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub(crate) actor_id: Option<i64>,
     pub(crate) actor_type: RulesetActorType,
     /// The bypass mode for the actor. Defaults to "always" per GitHub API.
-    /// - always: Actor can always bypass
-    /// - pull_request: Actor can only bypass on pull requests (not applicable for DeployKey)
-    /// - exempt: Rules won't run for actor, no audit entry created
     #[serde(skip_serializing_if = "Option::is_none")]
     pub(crate) bypass_mode: Option<RulesetBypassMode>,
 }
 
-/// Custom deserializer for actor_id that distinguishes between null and missing
-fn deserialize_actor_id<'de, D>(deserializer: D) -> Result<Option<ActorId>, D::Error>
-where
-    D: serde::Deserializer<'de>,
-{
-    use serde::{Deserialize, de};
-
-    struct ActorIdVisitor;
-
-    impl<'de> de::Visitor<'de> for ActorIdVisitor {
-        type Value = Option<ActorId>;
-
-        fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
-            formatter.write_str("an integer, null, or missing field")
-        }
-
-        fn visit_none<E>(self) -> Result<Self::Value, E>
-        where
-            E: de::Error,
-        {
-            Ok(Some(ActorId::Null))
-        }
-
-        fn visit_some<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
-        where
-            D: serde::Deserializer<'de>,
-        {
-            let id = i64::deserialize(deserializer)?;
-            Ok(Some(ActorId::Id(id)))
-        }
-
-        fn visit_unit<E>(self) -> Result<Self::Value, E>
-        where
-            E: de::Error,
-        {
-            Ok(Some(ActorId::Null))
-        }
-
-        fn visit_i64<E>(self, v: i64) -> Result<Self::Value, E>
-        where
-            E: de::Error,
-        {
-            Ok(Some(ActorId::Id(v)))
-        }
-
-        fn visit_u64<E>(self, v: u64) -> Result<Self::Value, E>
-        where
-            E: de::Error,
-        {
-            Ok(Some(ActorId::Id(v as i64)))
-        }
-    }
-
-    deserializer.deserialize_option(ActorIdVisitor)
-}
-
-/// Represents the actor_id field which can be a number or null
-#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
-#[serde(untagged)]
-pub(crate) enum ActorId {
-    /// A specific actor ID (used for Integration, RepositoryRole, Team, OrganizationAdmin)
-    Id(i64),
-    /// Explicitly null (required for DeployKey)
-    Null,
-}
-
 #[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
 #[serde(rename_all = "PascalCase")]
 pub(crate) enum RulesetActorType {
+    /// GitHub App integration
     Integration,
-    OrganizationAdmin,
-    RepositoryRole,
+    /// GitHub Team
     Team,
-    DeployKey,
-    EnterpriseOwner,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
@@ -730,7 +652,7 @@ mod tests {
     fn test_bypass_actor_serialization() {
         // Test Team actor with ID
         let team_actor = RulesetBypassActor {
-            actor_id: Some(ActorId::Id(234)),
+            actor_id: Some(234),
             actor_type: RulesetActorType::Team,
             bypass_mode: Some(RulesetBypassMode::Always),
         };
@@ -743,7 +665,7 @@ mod tests {
 
         // Test Integration actor with ID
         let integration_actor = RulesetBypassActor {
-            actor_id: Some(ActorId::Id(123456)),
+            actor_id: Some(123456),
             actor_type: RulesetActorType::Integration,
             bypass_mode: Some(RulesetBypassMode::Always),
         };
@@ -754,48 +676,22 @@ mod tests {
             "Integration actor should serialize with numeric actor_id"
         );
 
-        // Test OrganizationAdmin with ID 1 (per GitHub API spec)
-        let org_admin_actor = RulesetBypassActor {
-            actor_id: Some(ActorId::Id(1)),
-            actor_type: RulesetActorType::OrganizationAdmin,
-            bypass_mode: None, // Use API default
-        };
-        let json = serde_json::to_string(&org_admin_actor)
-            .expect("OrganizationAdmin actor serialization should succeed");
-        assert_eq!(
-            json, r#"{"actor_id":1,"actor_type":"OrganizationAdmin"}"#,
-            "OrganizationAdmin should use actor_id:1 and omit bypass_mode when None"
-        );
-
-        // Test DeployKey with null actor_id (per GitHub API spec)
-        let deploy_key_actor = RulesetBypassActor {
-            actor_id: Some(ActorId::Null),
-            actor_type: RulesetActorType::DeployKey,
-            bypass_mode: Some(RulesetBypassMode::Always),
-        };
-        let json = serde_json::to_string(&deploy_key_actor)
-            .expect("DeployKey actor serialization should succeed");
-        assert_eq!(
-            json, r#"{"actor_id":null,"actor_type":"DeployKey","bypass_mode":"always"}"#,
-            "DeployKey should serialize actor_id as null"
-        );
-
-        // Test EnterpriseOwner with None actor_id (field omitted per GitHub API spec)
-        let enterprise_actor = RulesetBypassActor {
+        // Test with None actor_id (field omitted)
+        let actor_no_id = RulesetBypassActor {
             actor_id: None,
-            actor_type: RulesetActorType::EnterpriseOwner,
-            bypass_mode: Some(RulesetBypassMode::Exempt),
+            actor_type: RulesetActorType::Team,
+            bypass_mode: Some(RulesetBypassMode::Always),
         };
-        let json = serde_json::to_string(&enterprise_actor)
-            .expect("EnterpriseOwner actor serialization should succeed");
+        let json = serde_json::to_string(&actor_no_id)
+            .expect("Actor without ID serialization should succeed");
         assert_eq!(
-            json, r#"{"actor_type":"EnterpriseOwner","bypass_mode":"exempt"}"#,
-            "EnterpriseOwner should omit actor_id field entirely"
+            json, r#"{"actor_type":"Team","bypass_mode":"always"}"#,
+            "Actor without ID should omit actor_id field"
         );
 
         // Test pull_request bypass mode
         let pr_actor = RulesetBypassActor {
-            actor_id: Some(ActorId::Id(789)),
+            actor_id: Some(789),
             actor_type: RulesetActorType::Team,
             bypass_mode: Some(RulesetBypassMode::PullRequest),
         };
@@ -809,15 +705,11 @@ mod tests {
 
     #[test]
     fn test_bypass_actor_deserialization() {
-        // Test deserializing from GitHub API response
+        // Test deserializing Team actor from GitHub API response
         let json = r#"{"actor_id":234,"actor_type":"Team","bypass_mode":"always"}"#;
         let actor: RulesetBypassActor =
             serde_json::from_str(json).expect("Should deserialize valid Team actor");
-        assert_eq!(
-            actor.actor_id,
-            Some(ActorId::Id(234)),
-            "actor_id should be numeric"
-        );
+        assert_eq!(actor.actor_id, Some(234), "actor_id should be numeric");
         assert_eq!(
             actor.actor_type,
             RulesetActorType::Team,
@@ -829,62 +721,23 @@ mod tests {
             "bypass_mode should be Always"
         );
 
-        // Test with null actor_id (DeployKey)
-        // Custom deserializer ensures JSON null becomes Some(ActorId::Null), not None
-        let json = r#"{"actor_id":null,"actor_type":"DeployKey","bypass_mode":"always"}"#;
-        let actor: RulesetBypassActor = serde_json::from_str(json)
-            .expect("Should deserialize DeployKey actor with null actor_id");
-        assert_eq!(
-            actor.actor_id,
-            Some(ActorId::Null),
-            "JSON null should deserialize to Some(ActorId::Null) with custom deserializer"
-        );
-        assert_eq!(actor.actor_type, RulesetActorType::DeployKey);
+        // Test deserializing Integration actor
+        let json = r#"{"actor_id":456,"actor_type":"Integration","bypass_mode":"always"}"#;
+        let actor: RulesetBypassActor =
+            serde_json::from_str(json).expect("Should deserialize valid Integration actor");
+        assert_eq!(actor.actor_id, Some(456));
+        assert_eq!(actor.actor_type, RulesetActorType::Integration);
 
         // Test with missing bypass_mode (should default to None)
-        let json = r#"{"actor_id":1,"actor_type":"OrganizationAdmin"}"#;
-        let actor: RulesetBypassActor = serde_json::from_str(json)
-            .expect("Should deserialize OrganizationAdmin without bypass_mode");
-        assert_eq!(actor.actor_id, Some(ActorId::Id(1)));
+        let json = r#"{"actor_id":1,"actor_type":"Team"}"#;
+        let actor: RulesetBypassActor =
+            serde_json::from_str(json).expect("Should deserialize Team without bypass_mode");
+        assert_eq!(actor.actor_id, Some(1));
         assert_eq!(
             actor.bypass_mode, None,
             "bypass_mode should be None when omitted from JSON"
         );
 
-        // Test with missing actor_id (EnterpriseOwner)
-        let json = r#"{"actor_type":"EnterpriseOwner","bypass_mode":"exempt"}"#;
-        let actor: RulesetBypassActor = serde_json::from_str(json)
-            .expect("Should deserialize EnterpriseOwner without actor_id");
-        assert_eq!(
-            actor.actor_id, None,
-            "actor_id should be None when omitted from JSON"
-        );
-        assert_eq!(actor.actor_type, RulesetActorType::EnterpriseOwner);
-        assert_eq!(actor.bypass_mode, Some(RulesetBypassMode::Exempt));
-
-        // Test all actor types can be deserialized
-        let actor_types = [
-            ("Integration", RulesetActorType::Integration),
-            ("OrganizationAdmin", RulesetActorType::OrganizationAdmin),
-            ("RepositoryRole", RulesetActorType::RepositoryRole),
-            ("Team", RulesetActorType::Team),
-            ("DeployKey", RulesetActorType::DeployKey),
-            ("EnterpriseOwner", RulesetActorType::EnterpriseOwner),
-        ];
-        for (type_str, expected_type) in actor_types {
-            let json = format!(
-                r#"{{"actor_id":1,"actor_type":"{}","bypass_mode":"always"}}"#,
-                type_str
-            );
-            let actor: RulesetBypassActor = serde_json::from_str(&json)
-                .unwrap_or_else(|e| panic!("Should deserialize actor type {}: {}", type_str, e));
-            assert_eq!(
-                actor.actor_type, expected_type,
-                "actor_type {} should deserialize correctly",
-                type_str
-            );
-        }
-
         // Test all bypass modes can be deserialized
         let bypass_modes = [
             ("always", RulesetBypassMode::Always),
diff --git a/sync-team/src/github/mod.rs b/sync-team/src/github/mod.rs
@@ -990,7 +990,7 @@ pub(crate) fn construct_bypass_actors(
         match github_write.resolve_team_database_id(org, team_name) {
             Ok(Some(actor_id)) => {
                 bypass_actors.push(api::RulesetBypassActor {
-                    actor_id: Some(api::ActorId::Id(actor_id)),
+                    actor_id: Some(actor_id),
                     actor_type: api::RulesetActorType::Team,
                     bypass_mode: Some(api::RulesetBypassMode::Always),
                 });
@@ -1025,7 +1025,7 @@ pub(crate) fn construct_bypass_actors(
         match github_write.resolve_user_database_id(user_login, org) {
             Ok(Some(actor_id)) => {
                 bypass_actors.push(api::RulesetBypassActor {
-                    actor_id: Some(api::ActorId::Id(actor_id)),
+                    actor_id: Some(actor_id),
                     actor_type: api::RulesetActorType::Integration,
                     bypass_mode: Some(api::RulesetBypassMode::Always),
                 });
@@ -1849,11 +1849,10 @@ fn log_ruleset(
         } else {
             writeln!(result, "        Bypass Actors:")?;
             for actor in bypass_actors {
-                let actor_id_str = match &actor.actor_id {
-                    Some(api::ActorId::Id(id)) => id.to_string(),
-                    Some(api::ActorId::Null) => "null".to_string(),
-                    None => "omitted".to_string(),
-                };
+                let actor_id_str = actor
+                    .actor_id
+                    .map(|id| id.to_string())
+                    .unwrap_or_else(|| "none".to_string());
                 let mode_str = actor.bypass_mode.as_ref().map_or("default", |m| match m {
                     api::RulesetBypassMode::Always => "always",
                     api::RulesetBypassMode::PullRequest => "pull_request",
