From 5aed99b4a69d087128b224de065e33cd080b8466 Mon Sep 17 00:00:00 2001
From: vladfreeze <vladfreeze@gmail.com>
Date: Thu, 2 Dec 2021 11:29:26 +0100
Subject: [PATCH] Improved design

---
 .../components/DynamicTable/DynamicTable.jsx  |  2 +-
 jsx/src/components/GroupEdit/GroupEdit.jsx    | 70 ++++++++++---------
 jupyterhub/apihandlers/base.py                |  4 +-
 jupyterhub/apihandlers/groups.py              |  8 ++-
 jupyterhub/orm.py                             |  2 +-
 jupyterhub/tests/test_api.py                  | 66 +++++++++++++----
 6 files changed, 98 insertions(+), 54 deletions(-)

diff --git a/jsx/src/components/DynamicTable/DynamicTable.jsx b/jsx/src/components/DynamicTable/DynamicTable.jsx
index 7063cc9e..35508e2e 100644
--- a/jsx/src/components/DynamicTable/DynamicTable.jsx
+++ b/jsx/src/components/DynamicTable/DynamicTable.jsx
@@ -253,4 +253,4 @@ export default class DynamicTable extends React.Component {
       </div>
     );
   }
-}
\ No newline at end of file
+}
diff --git a/jsx/src/components/GroupEdit/GroupEdit.jsx b/jsx/src/components/GroupEdit/GroupEdit.jsx
index ad6779f9..16ebcaec 100644
--- a/jsx/src/components/GroupEdit/GroupEdit.jsx
+++ b/jsx/src/components/GroupEdit/GroupEdit.jsx
@@ -5,21 +5,18 @@ import PropTypes from "prop-types";
 import GroupSelect from "../GroupSelect/GroupSelect";
 import DynamicTable from "../DynamicTable/DynamicTable";
 
-
 function hasDuplicates(array) {
   var valuesSoFar = Object.create(null);
   for (var i = 0; i < array.length; ++i) {
-      var value = array[i];
-      if (value in valuesSoFar) {
-          return true;
-      }
-      valuesSoFar[value] = true;
+    var value = array[i];
+    if (value in valuesSoFar) {
+      return true;
+    }
+    valuesSoFar[value] = true;
   }
   return false;
 }
 
-
-
 const GroupEdit = (props) => {
   var [selected, setSelected] = useState([]),
     [changed, setChanged] = useState(false),
@@ -55,8 +52,8 @@ const GroupEdit = (props) => {
 
   var { group_data } = location.state;
   var [propobject, setProp] = useState(group_data.properties);
-  var [propkeys, setPropKeys]= useState([]);
-  var [propvalues, setPropValues]= useState([]);
+  var [propkeys, setPropKeys] = useState([]);
+  var [propvalues, setPropValues] = useState([]);
 
   if (!group_data) return <div></div>;
 
@@ -77,19 +74,24 @@ const GroupEdit = (props) => {
           setChanged(true);
         }}
       />
-      <div className="container">
       <div className="row">
+        <div className="col-md-10 col-md-offset-1 col-lg-8 col-lg-offset-2">
           <div className="alert alert-info">Manage group properties</div>
+        </div>
       </div>
+      <div className="row">
+        <div className="col-md-10 col-md-offset-1 col-lg-8 col-lg-offset-2">
+          <DynamicTable
+            current_propobject={group_data.properties}
+            setProp={setProp}
+            setPropKeys={setPropKeys}
+            setPropValues={setPropValues}
+            setChanged={setChanged}
+            //Add keys
+          />
+        </div>
       </div>
-      <DynamicTable
-        current_propobject={group_data.properties}
-        setProp={setProp}
-        setPropKeys={setPropKeys}
-        setPropValues={setPropValues}
-        setChanged ={setChanged}
-        //Add keys
-      />
+
       <div className="row">
         <div className="col-md-10 col-md-offset-1 col-lg-8 col-lg-offset-2">
           <button id="return" className="btn btn-light">
@@ -119,22 +121,24 @@ const GroupEdit = (props) => {
                 promiseQueue.push(
                   removeFromGroup(removed_users, group_data.name)
                 );
-              if (hasDuplicates(propkeys)==true){
-                error.textContent = "Duplicate key found!"
-                error.style.color = "red"
-              } else{
-                error.textContent = ""
-                propkeys.forEach((key, i) => propobject[key] = propvalues[i]);
+              if (hasDuplicates(propkeys) == true) {
+                error.textContent = "Duplicate key found!";
+                error.style.color = "red";
+              } else {
+                error.textContent = "";
+                propkeys.forEach((key, i) => (propobject[key] = propvalues[i]));
               }
-              if (propobject != group_data.properties && hasDuplicates(propkeys)== false) {
-                promiseQueue.push(
-                  updateProp(propobject, group_data.name)
-                );
+              if (
+                propobject != group_data.properties &&
+                hasDuplicates(propkeys) == false
+              ) {
+                promiseQueue.push(updateProp(propobject, group_data.name));
               }
               Promise.all(promiseQueue)
                 .then(() => {
-                  updateGroups(0, limit)
-                    .then((data) => dispatchPageUpdate(data, 0)); 
+                  updateGroups(0, limit).then((data) =>
+                    dispatchPageUpdate(data, 0)
+                  );
                 })
                 .catch((err) => console.log(err));
             }}
@@ -142,8 +146,8 @@ const GroupEdit = (props) => {
             Apply
           </button>
           <div>
-                <span id="error"></span>
-            </div>
+            <span id="error"></span>
+          </div>
           <button
             id="delete-group"
             className="btn btn-danger"
diff --git a/jupyterhub/apihandlers/base.py b/jupyterhub/apihandlers/base.py
index 9331ff49..51019d42 100644
--- a/jupyterhub/apihandlers/base.py
+++ b/jupyterhub/apihandlers/base.py
@@ -298,10 +298,10 @@ class APIHandler(BaseHandler):
             'name': group.name,
             'roles': [r.name for r in group.roles],
             'users': [u.name for u in group.users],
-            'properties': group.properties
+            'properties': group.properties,
         }
         access_map = {
-            'read:groups': {'kind', 'name', 'users','properties'},
+            'read:groups': {'kind', 'name', 'users', 'properties'},
             'read:groups:name': {'kind', 'name'},
             'read:roles:groups': {'kind', 'name', 'roles'},
         }
diff --git a/jupyterhub/apihandlers/groups.py b/jupyterhub/apihandlers/groups.py
index 57750d2f..4ac0d983 100644
--- a/jupyterhub/apihandlers/groups.py
+++ b/jupyterhub/apihandlers/groups.py
@@ -180,21 +180,23 @@ class GroupUsersAPIHandler(_GroupAPIHandler):
         self.db.commit()
         self.write(json.dumps(self.group_model(group)))
 
+
 class GroupPropertiesAPIHandler(_GroupAPIHandler):
     """Modify a group's properties"""
-    
+
     @needs_scope('groups')
     def put(self, group_name):
         group = self.find_group(group_name)
         data = self.get_json_body()
-        #self._check_group_model(data)
+        # self._check_group_model(data)
         if not isinstance(data, dict):
             raise web.HTTPError(400, "Must specify properties")
         self.log.info("Updating properties of group %s", group_name)
         group.properties = data
         self.db.commit()
         self.write(json.dumps(self.group_model(group)))
- 
+
+
 default_handlers = [
     (r"/api/groups", GroupListAPIHandler),
     (r"/api/groups/([^/]+)", GroupAPIHandler),
diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py
index 20975f0e..6adfe71f 100644
--- a/jupyterhub/orm.py
+++ b/jupyterhub/orm.py
@@ -223,7 +223,7 @@ class Group(Base):
     id = Column(Integer, primary_key=True, autoincrement=True)
     name = Column(Unicode(255), unique=True)
     users = relationship('User', secondary='user_group_map', backref='groups')
-    properties = Column(JSONDict, default= {}) 
+    properties = Column(JSONDict, default={})
 
     def __repr__(self):
         return "<%s %s (%i users)>" % (
diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py
index 740e7760..79aed81f 100644
--- a/jupyterhub/tests/test_api.py
+++ b/jupyterhub/tests/test_api.py
@@ -1613,8 +1613,20 @@ async def test_groups_list(app):
     r.raise_for_status()
     reply = r.json()
     assert reply == [
-        {'kind': 'group', 'name': 'alphaflight', 'users': [], 'roles': [], 'properties': {}},
-        {'kind': 'group', 'name': 'betaflight', 'users': [], 'roles': [],'properties': {}},
+        {
+            'kind': 'group',
+            'name': 'alphaflight',
+            'users': [],
+            'roles': [],
+            'properties': {},
+        },
+        {
+            'kind': 'group',
+            'name': 'betaflight',
+            'users': [],
+            'roles': [],
+            'properties': {},
+        },
     ]
 
     # Test offset for pagination
@@ -1622,7 +1634,15 @@ async def test_groups_list(app):
     r.raise_for_status()
     reply = r.json()
     assert r.status_code == 200
-    assert reply == [{'kind': 'group', 'name': 'betaflight', 'users': [], 'roles': [],'properties': {}}]
+    assert reply == [
+        {
+            'kind': 'group',
+            'name': 'betaflight',
+            'users': [],
+            'roles': [],
+            'properties': {},
+        }
+    ]
 
     r = await api_request(app, "groups?offset=10")
     r.raise_for_status()
@@ -1634,13 +1654,29 @@ async def test_groups_list(app):
     r.raise_for_status()
     reply = r.json()
     assert r.status_code == 200
-    assert reply == [{'kind': 'group', 'name': 'alphaflight', 'users': [], 'roles': [],'properties': {}}]
+    assert reply == [
+        {
+            'kind': 'group',
+            'name': 'alphaflight',
+            'users': [],
+            'roles': [],
+            'properties': {},
+        }
+    ]
 
     # 0 is rounded up to 1
     r = await api_request(app, "groups?limit=0")
     r.raise_for_status()
     reply = r.json()
-    assert reply == [{'kind': 'group', 'name': 'alphaflight', 'users': [], 'roles': [],'properties': {}}]
+    assert reply == [
+        {
+            'kind': 'group',
+            'name': 'alphaflight',
+            'users': [],
+            'roles': [],
+            'properties': {},
+        }
+    ]
 
 
 @mark.group
@@ -1769,7 +1805,9 @@ async def test_group_add_delete_users(app):
 
     group = orm.Group.find(db, name='alphaflight')
     assert sorted(u.name for u in group.users) == sorted(names[2:])
-@mark.group  
+
+
+@mark.group
 async def test_group_add_properties(app):
     db = app.db
     group = orm.Group(name='alphaflight')
@@ -1778,8 +1816,8 @@ async def test_group_add_properties(app):
     r = await api_request(app, 'groups/alphaflight/properties', method='put', data='{}')
     assert r.status_code == 200
 
-    properties_object = {'cpu': "8", 'ram': "4", 'image':"testimage"}
-    
+    properties_object = {'cpu': "8", 'ram': "4", 'image': "testimage"}
+
     r = await api_request(
         app,
         'groups/alphaflight/properties',
@@ -1788,12 +1826,12 @@ async def test_group_add_properties(app):
     )
     r.raise_for_status()
     group = orm.Group.find(db, name='alphaflight')
-    
-    
-    assert sorted([k for k in group.properties]) == sorted([k for k in properties_object])
-    assert sorted([group.properties[k] for k in group.properties]) == sorted([properties_object[k] for k in properties_object])
-    
-    
+
+    assert sorted(k for k in group.properties) == sorted(k for k in properties_object)
+    assert sorted(group.properties[k] for k in group.properties) == sorted(
+        properties_object[k] for k in properties_object
+    )
+
 
 # -----------------
 # Service API tests