From 095bb83202d44f1dbb5405089db749bdc9dcb70e Mon Sep 17 00:00:00 2001
From: Andrew Kingston <andrew@kingston.dev>
Date: Thu, 4 Aug 2022 13:25:44 +0100
Subject: [PATCH] Fix adding/removing users to group. Remove add all button

---
 .../settings/UserGroupPicker.svelte           | 140 +++++++++++-------
 .../portal/manage/groups/[groupId].svelte     |  98 +++++-------
 2 files changed, 122 insertions(+), 116 deletions(-)

diff --git a/packages/builder/src/components/settings/UserGroupPicker.svelte b/packages/builder/src/components/settings/UserGroupPicker.svelte
index 6eef15efe6..67b970d1bf 100644
--- a/packages/builder/src/components/settings/UserGroupPicker.svelte
+++ b/packages/builder/src/components/settings/UserGroupPicker.svelte
@@ -1,75 +1,109 @@
 <script>
-  import { ActionButton, Icon, Search, Divider, Detail } from "@budibase/bbui"
+  import { Icon, Search, Divider, Layout } from "@budibase/bbui"
+  import { createEventDispatcher } from "svelte"
 
   export let searchTerm = ""
   export let selected
-  export let filtered = []
-  export let addAll
-  export let select
-  export let title
-  export let key
+  export let list = []
+  export let labelKey
+
+  const dispatch = createEventDispatcher()
+
+  $: enrichedList = enrich(list, selected)
+  $: sortedList = sort(enrichedList)
+
+  const enrich = (list, selected) => {
+    return list.map(item => {
+      return {
+        ...item,
+        selected: selected.find(x => x._id === item._id) != null,
+      }
+    })
+  }
+
+  const sort = list => {
+    let sortedList = list.slice()
+    sortedList.sort((a, b) => {
+      if (a.selected === b.selected) {
+        return a[labelKey] < b[labelKey] ? -1 : 1
+      } else if (a.selected) {
+        return -1
+      } else if (b.selected) {
+        return 1
+      }
+      return 0
+    })
+    return sortedList
+  }
 </script>
 
-<div style="padding: var(--spacing-m)">
-  <Search placeholder="Search" bind:value={searchTerm} />
-  <div class="header sub-header">
-    <div>
-      <Detail
-        >{filtered.length} {title}{filtered.length === 1 ? "" : "s"}</Detail
-      >
+<div class="container">
+  <Layout gap="S">
+    <div class="header">
+      <Search placeholder="Search" bind:value={searchTerm} />
     </div>
-    <div>
-      <ActionButton on:click={addAll} emphasized size="S">Add all</ActionButton>
-    </div>
-  </div>
-  <Divider noMargin />
-  <div>
-    {#each filtered as item}
-      <div
-        on:click={() => {
-          select(item._id)
-        }}
-        style="padding-bottom: var(--spacing-m)"
-        class="selection"
-      >
-        <div>
-          {item[key]}
-        </div>
-
-        {#if selected.includes(item._id)}
-          <div>
-            <Icon
-              color="var(--spectrum-global-color-blue-600);"
-              name="Checkmark"
-            />
+    <Divider noMargin />
+    <div class="items">
+      {#each sortedList as item}
+        <div
+          on:click={() => {
+            dispatch(item.selected ? "deselect" : "select", item._id)
+          }}
+          class="item"
+        >
+          <div class="text">
+            {item[labelKey]}
           </div>
-        {/if}
-      </div>
-    {/each}
-  </div>
+          {#if item.selected}
+            <div>
+              <Icon
+                color="var(--spectrum-global-color-blue-600);"
+                name="Checkmark"
+              />
+            </div>
+          {/if}
+        </div>
+      {/each}
+    </div>
+  </Layout>
 </div>
 
 <style>
+  .container {
+    width: 280px;
+  }
   .header {
     align-items: center;
-    padding: var(--spacing-m) 0 var(--spacing-m) 0;
-    display: flex;
-    justify-content: space-between;
+    display: grid;
+    gap: var(--spacing-m);
+    grid-template-columns: 1fr;
   }
-
-  .selection {
+  .items {
+    max-height: 242px;
+    overflow: auto;
+    overflow-x: hidden;
+    margin: 0 calc(-1 * var(--spacing-m));
+    margin-top: -8px;
+  }
+  .item {
     align-items: end;
     display: flex;
     justify-content: space-between;
     cursor: pointer;
+    padding: var(--spacing-s) var(--spacing-l);
+    background: var(--spectrum-global-color-gray-50);
+    transition: background 130ms ease-out;
+    gap: var(--spacing-xl);
   }
-
-  .selection > :first-child {
-    padding-top: var(--spacing-m);
+  .item:hover {
+    background: var(--spectrum-global-color-gray-100);
+    cursor: pointer;
   }
-
-  .sub-header {
-    display: flex;
-    justify-content: space-between;
+  .text {
+    flex: 1 1 auto;
+    width: 0;
+    overflow: hidden;
+    text-overflow: ellipsis;
+    white-space: nowrap;
   }
 </style>
diff --git a/packages/builder/src/pages/builder/portal/manage/groups/[groupId].svelte b/packages/builder/src/pages/builder/portal/manage/groups/[groupId].svelte
index 962fed800a..948e77a66e 100644
--- a/packages/builder/src/pages/builder/portal/manage/groups/[groupId].svelte
+++ b/packages/builder/src/pages/builder/portal/manage/groups/[groupId].svelte
@@ -31,7 +31,6 @@
   let popoverAnchor
   let popover
   let searchTerm = ""
-  let selectedUsers = []
   let prevSearch = undefined
   let pageInfo = createPaginationStore()
   let loaded = false
@@ -41,9 +40,7 @@
   $: page = $pageInfo.page
   $: fetchUsers(page, searchTerm)
   $: group = $groups.find(x => x._id === groupId)
-  $: filtered =
-    $users.data?.filter(x => !group?.users.map(y => y._id).includes(x._id)) ||
-    []
+  $: filtered = $users.data
   $: groupApps = $apps.filter(x => group?.apps.includes(x.appId))
   $: {
     if (loaded && !group?._id) {
@@ -51,53 +48,24 @@
     }
   }
 
-  async function addAll() {
-    selectedUsers = [...selectedUsers, ...filtered.map(u => u._id)]
-
-    let reducedUserObjects = filtered.map(u => {
-      return {
-        _id: u._id,
-        email: u.email,
-      }
-    })
-    group.users = [...reducedUserObjects, ...group.users]
-
-    await groups.actions.save(group)
-
-    $users.data.forEach(async user => {
-      let userToEdit = await users.get(user._id)
-      let userGroups = userToEdit.userGroups || []
-      userGroups.push(groupId)
-      await users.save({
-        ...userToEdit,
-        userGroups,
-      })
-    })
-  }
-
-  async function selectUser(id) {
-    let selectedUser = selectedUsers.includes(id)
-    if (selectedUser) {
-      selectedUsers = selectedUsers.filter(id => id !== selectedUser)
-      let newUsers = group.users.filter(user => user._id !== id)
-      group.users = newUsers
-    } else {
-      let enrichedUser = $users.data
-        .filter(user => user._id === id)
-        .map(u => {
-          return {
-            _id: u._id,
-            email: u.email,
-          }
-        })[0]
-      selectedUsers = [...selectedUsers, id]
-      group.users.push(enrichedUser)
+  const adduserToGroup = async id => {
+    const user = await users.get(id)
+    if (!user?._id) {
+      return
     }
 
-    await groups.actions.save(group)
+    // Check we haven't already been added
+    if (group.users?.find(x => x._id === user._id)) {
+      return
+    }
 
-    let user = await users.get(id)
+    // Update group
+    await groups.actions.save({
+      ...group,
+      users: [...group.users, { _id: user._id, email: user.email }],
+    })
 
+    // Update user
     let userGroups = user.userGroups || []
     userGroups.push(groupId)
     await users.save({
@@ -106,17 +74,23 @@
     })
   }
 
-  async function removeUser(id) {
-    let newUsers = group.users.filter(user => user._id !== id)
-    group.users = newUsers
-    let user = await users.get(id)
+  const removeUserFromGroup = async id => {
+    const user = await users.get(id)
+    if (!user?._id) {
+      return
+    }
 
-    await users.save({
-      ...user,
-      userGroups: [],
+    // Update group
+    await groups.actions.save({
+      ...group,
+      users: group.users.filter(x => x._id !== id),
     })
 
-    await groups.actions.save(group)
+    // Update user
+    await users.save({
+      ...user,
+      userGroups: user.userGroups.filter(x => x !== groupId),
+    })
   }
 
   async function fetchUsers(page, search) {
@@ -150,7 +124,6 @@
       notifications.success("User group deleted successfully")
       $goto("./")
     } catch (error) {
-      console.log(error)
       notifications.error(`Failed to delete user group`)
     }
   }
@@ -216,13 +189,12 @@
           </div>
           <Popover align="right" bind:this={popover} anchor={popoverAnchor}>
             <UserGroupPicker
-              key={"email"}
-              title={"User"}
               bind:searchTerm
-              bind:selected={selectedUsers}
-              bind:filtered
-              {addAll}
-              select={selectUser}
+              labelKey="email"
+              selected={group.users}
+              list={$users.data}
+              on:select={e => adduserToGroup(e.detail)}
+              on:deselect={e => removeUserFromGroup(e.detail)}
             />
           </Popover>
         </div>
@@ -236,7 +208,7 @@
                 hoverable
               >
                 <Icon
-                  on:click={() => removeUser(user._id)}
+                  on:click={() => removeUserFromGroup(user._id)}
                   hoverable
                   size="S"
                   name="Close"