From 0eee907c883a4e219816253cc86a0eac233c0c57 Mon Sep 17 00:00:00 2001
From: Jeremy Lin <jeremy.lin@gmail.com>
Date: Sun, 13 Sep 2020 02:03:16 -0700
Subject: [PATCH] Simplify implementation of `UserOrgType::cmp()`

Also move `UserOrgType::from_str()` closer to the definition of `UserOrgType`
since it references specific enum values.
---
 src/db/models/organization.rs | 61 +++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs
index aa9d9494..6a4012dd 100644
--- a/src/db/models/organization.rs
+++ b/src/db/models/organization.rs
@@ -44,24 +44,28 @@ pub enum UserOrgType {
     Manager = 3,
 }
 
+impl UserOrgType {
+    pub fn from_str(s: &str) -> Option<Self> {
+        match s {
+            "0" | "Owner" => Some(UserOrgType::Owner),
+            "1" | "Admin" => Some(UserOrgType::Admin),
+            "2" | "User" => Some(UserOrgType::User),
+            "3" | "Manager" => Some(UserOrgType::Manager),
+            _ => None,
+        }
+    }
+}
+
 impl Ord for UserOrgType {
     fn cmp(&self, other: &UserOrgType) -> Ordering {
-        if self == other {
-            Ordering::Equal
-        } else {
-            match self {
-                UserOrgType::Owner => Ordering::Greater,
-                UserOrgType::Admin => match other {
-                    UserOrgType::Owner => Ordering::Less,
-                    _ => Ordering::Greater,
-                },
-                UserOrgType::Manager => match other {
-                    UserOrgType::Owner | UserOrgType::Admin => Ordering::Less,
-                    _ => Ordering::Greater,
-                },
-                UserOrgType::User => Ordering::Less,
-            }
-        }
+        // For easy comparison, map each variant to an access level (where 0 is lowest).
+        static ACCESS_LEVEL: [i32; 4] = [
+            3, // Owner
+            2, // Admin
+            0, // User
+            1, // Manager
+        ];
+        ACCESS_LEVEL[*self as usize].cmp(&ACCESS_LEVEL[*other as usize])
     }
 }
 
@@ -129,18 +133,6 @@ impl PartialOrd<UserOrgType> for i32 {
     }
 }
 
-impl UserOrgType {
-    pub fn from_str(s: &str) -> Option<Self> {
-        match s {
-            "0" | "Owner" => Some(UserOrgType::Owner),
-            "1" | "Admin" => Some(UserOrgType::Admin),
-            "2" | "User" => Some(UserOrgType::User),
-            "3" | "Manager" => Some(UserOrgType::Manager),
-            _ => None,
-        }
-    }
-}
-
 /// Local methods
 impl Organization {
     pub fn new(name: String, billing_email: String) -> Self {
@@ -533,3 +525,16 @@ impl UserOrganization {
         }}
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    #[allow(non_snake_case)]
+    fn partial_cmp_UserOrgType() {
+        assert!(UserOrgType::Owner > UserOrgType::Admin);
+        assert!(UserOrgType::Admin > UserOrgType::Manager);
+        assert!(UserOrgType::Manager > UserOrgType::User);
+    }
+}