From df472ac391257c6f103c5c3a2349cb5f74a0d7b3 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Tue, 9 Feb 2021 20:13:09 -0800 Subject: [PATCH] feat: add total users to people management screen (#1878) * feat: add total users to pagination * move this.total in runInAction callback * add total counts + counts to people tabs * progress: use raw pg query * progress: add test * fix: SQL interpolation * Styling and translation of People page Co-authored-by: Tim --- .../{Sidebar/components => }/Bubble.js | 8 +- app/components/Sidebar/Main.js | 6 +- app/components/Tab.js | 4 +- app/components/Tabs.js | 1 + app/menus/CollectionMenu.js | 6 +- app/scenes/Settings/People.js | 49 +++++++----- app/stores/BaseStore.js | 4 +- app/stores/UsersStore.js | 43 ++++++++++- server/api/users.js | 11 +++ server/api/users.test.js | 75 ++++++++++++++++++- server/models/User.js | 32 ++++++++ shared/i18n/locales/en_US/translation.json | 7 +- 12 files changed, 215 insertions(+), 31 deletions(-) rename app/components/{Sidebar/components => }/Bubble.js (92%) diff --git a/app/components/Sidebar/components/Bubble.js b/app/components/Bubble.js similarity index 92% rename from app/components/Sidebar/components/Bubble.js rename to app/components/Bubble.js index e70087e36..2bfd3dc2b 100644 --- a/app/components/Sidebar/components/Bubble.js +++ b/app/components/Bubble.js @@ -3,11 +3,15 @@ import * as React from "react"; import styled from "styled-components"; import { bounceIn } from "shared/styles/animations"; -type Props = { +type Props = {| count: number, -}; +|}; const Bubble = ({ count }: Props) => { + if (!count) { + return null; + } + return {count}; }; diff --git a/app/components/Sidebar/Main.js b/app/components/Sidebar/Main.js index fdbd2e353..d66967256 100644 --- a/app/components/Sidebar/Main.js +++ b/app/components/Sidebar/Main.js @@ -16,11 +16,11 @@ import { useTranslation } from "react-i18next"; import styled from "styled-components"; import CollectionNew from "scenes/CollectionNew"; import Invite from "scenes/Invite"; +import Bubble from "components/Bubble"; import Flex from "components/Flex"; import Modal from "components/Modal"; import Scrollable from "components/Scrollable"; import Sidebar from "./Sidebar"; -import Bubble from "./components/Bubble"; import Collections from "./components/Collections"; import HeaderBlock from "./components/HeaderBlock"; import Section from "./components/Section"; @@ -118,9 +118,7 @@ function MainSidebar() { label={ {t("Drafts")} - {documents.totalDrafts > 0 && ( - - )} + } active={ diff --git a/app/components/Tab.js b/app/components/Tab.js index 75b9e61d4..c616e0a3e 100644 --- a/app/components/Tab.js +++ b/app/components/Tab.js @@ -10,8 +10,8 @@ type Props = { const StyledNavLink = styled(NavLink)` position: relative; - - display: inline-block; + display: inline-flex; + align-items: center; font-weight: 500; font-size: 14px; color: ${(props) => props.theme.textTertiary}; diff --git a/app/components/Tabs.js b/app/components/Tabs.js index 3ee58b495..5ce46c3db 100644 --- a/app/components/Tabs.js +++ b/app/components/Tabs.js @@ -8,6 +8,7 @@ const Tabs = styled.nav` margin-bottom: 12px; overflow-y: auto; white-space: nowrap; + transition: opacity 250ms ease-out; `; export const Separator = styled.span` diff --git a/app/menus/CollectionMenu.js b/app/menus/CollectionMenu.js index 76d325b75..90cd6470e 100644 --- a/app/menus/CollectionMenu.js +++ b/app/menus/CollectionMenu.js @@ -64,6 +64,10 @@ function CollectionMenu({ [history, collection.id] ); + const stopPropagation = React.useCallback((ev: SyntheticEvent<>) => { + ev.stopPropagation(); + }, []); + const handleImportDocument = React.useCallback( (ev: SyntheticEvent<>) => { ev.preventDefault(); @@ -107,7 +111,7 @@ function CollectionMenu({ type="file" ref={file} onChange={handleFilePicked} - onClick={(ev) => ev.stopPropagation()} + onClick={stopPropagation} accept={documents.importFileTypes.join(", ")} tabIndex="-1" /> diff --git a/app/scenes/Settings/People.js b/app/scenes/Settings/People.js index 939367142..7f21d4172 100644 --- a/app/scenes/Settings/People.js +++ b/app/scenes/Settings/People.js @@ -4,12 +4,13 @@ import { observable } from "mobx"; import { observer, inject } from "mobx-react"; import { PlusIcon } from "outline-icons"; import * as React from "react"; +import { withTranslation, type TFunction, Trans } from "react-i18next"; import { type Match } from "react-router-dom"; - import AuthStore from "stores/AuthStore"; import PoliciesStore from "stores/PoliciesStore"; import UsersStore from "stores/UsersStore"; import Invite from "scenes/Invite"; +import Bubble from "components/Bubble"; import Button from "components/Button"; import CenteredContent from "components/CenteredContent"; import Empty from "components/Empty"; @@ -27,12 +28,20 @@ type Props = { users: UsersStore, policies: PoliciesStore, match: Match, + t: TFunction, }; @observer class People extends React.Component { @observable inviteModalOpen: boolean = false; + componentDidMount() { + const { team } = this.props.auth; + if (team) { + this.props.users.fetchCounts(team.id); + } + } + handleInviteModalOpen = () => { this.inviteModalOpen = true; }; @@ -46,7 +55,7 @@ class People extends React.Component { }; render() { - const { auth, policies, match } = this.props; + const { auth, policies, match, t } = this.props; const { filter } = match.params; const currentUser = auth.user; const team = auth.team; @@ -65,15 +74,18 @@ class People extends React.Component { } const can = policies.abilities(team.id); + const { counts } = this.props.users; return ( - -

People

+ +

{t("People")}

- Everyone that has signed into Outline appears here. It’s possible that - there are other users who have access through {team.signinMethods} but - haven’t signed in yet. + + Everyone that has signed into Outline appears here. It’s possible + that there are other users who have access through{" "} + {team.signinMethods} but haven’t signed in yet. + - Active + {t("Active")} - Admins + {t("Admins")} {can.update && ( - Suspended + {t("Suspended")} )} - Everyone + {t("Everyone")} - {can.invite && ( <> - Invited + {t("Invited")} )} No people to see here.} + empty={{t("No people to see here.")}} fetch={this.fetchPage} renderItem={(item) => ( { /> @@ -137,4 +148,8 @@ class People extends React.Component { } } -export default inject("auth", "users", "policies")(People); +export default inject( + "auth", + "users", + "policies" +)(withTranslation()(People)); diff --git a/app/stores/BaseStore.js b/app/stores/BaseStore.js index a8f97e929..e90fae77a 100644 --- a/app/stores/BaseStore.js +++ b/app/stores/BaseStore.js @@ -7,7 +7,7 @@ import BaseModel from "../models/BaseModel"; import type { PaginationParams } from "types"; import { client } from "utils/ApiClient"; -type Action = "list" | "info" | "create" | "update" | "delete"; +type Action = "list" | "info" | "create" | "update" | "delete" | "count"; function modelNameFromClassName(string) { return string.charAt(0).toLowerCase() + string.slice(1); @@ -24,7 +24,7 @@ export default class BaseStore { model: Class; modelName: string; rootStore: RootStore; - actions: Action[] = ["list", "info", "create", "update", "delete"]; + actions: Action[] = ["list", "info", "create", "update", "delete", "count"]; constructor(rootStore: RootStore, model: Class) { this.rootStore = rootStore; diff --git a/app/stores/UsersStore.js b/app/stores/UsersStore.js index 49240a611..ea6c88c14 100644 --- a/app/stores/UsersStore.js +++ b/app/stores/UsersStore.js @@ -1,13 +1,21 @@ // @flow import invariant from "invariant"; import { filter, orderBy } from "lodash"; -import { computed, action, runInAction } from "mobx"; +import { observable, computed, action, runInAction } from "mobx"; import User from "models/User"; import BaseStore from "./BaseStore"; import RootStore from "./RootStore"; import { client } from "utils/ApiClient"; export default class UsersStore extends BaseStore { + @observable counts: { + active: number, + admins: number, + all: number, + invited: number, + suspended: number, + } = {}; + constructor(rootStore: RootStore) { super(rootStore, User); } @@ -52,21 +60,25 @@ export default class UsersStore extends BaseStore { @action promote = (user: User) => { + this.counts.admins += 1; return this.actionOnUser("promote", user); }; @action demote = (user: User) => { + this.counts.admins -= 1; return this.actionOnUser("demote", user); }; @action suspend = (user: User) => { + this.counts.suspended += 1; return this.actionOnUser("suspend", user); }; @action activate = (user: User) => { + this.counts.suspended -= 1; return this.actionOnUser("activate", user); }; @@ -76,10 +88,39 @@ export default class UsersStore extends BaseStore { invariant(res && res.data, "Data should be available"); runInAction(`invite`, () => { res.data.users.forEach(this.add); + this.counts.invited += res.data.sent.length; + this.counts.all += res.data.sent.length; }); return res.data; }; + @action + fetchCounts = async (teamId: string): Promise<*> => { + const res = await client.post(`/users.count`, { teamId }); + invariant(res && res.data, "Data should be available"); + + this.counts = res.data.counts; + return res.data; + }; + + @action + async delete(user: User, options: Object = {}) { + super.delete(user, options); + if (!user.isSuspended && user.lastActiveAt) { + this.counts.active -= 1; + } + if (user.isInvited) { + this.counts.invited -= 1; + } + if (user.isAdmin) { + this.counts.admins -= 1; + } + if (user.isSuspended) { + this.counts.suspended -= 1; + } + this.counts.all -= 1; + } + notInCollection = (collectionId: string, query: string = "") => { const memberships = filter( this.rootStore.memberships.orderedData, diff --git a/server/api/users.js b/server/api/users.js index 7f5e22e3c..aec2be379 100644 --- a/server/api/users.js +++ b/server/api/users.js @@ -55,6 +55,17 @@ router.post("users.list", auth(), pagination(), async (ctx) => { }; }); +router.post("users.count", auth(), async (ctx) => { + const { user } = ctx.state; + const counts = await User.getCounts(user.teamId); + + ctx.body = { + data: { + counts, + }, + }; +}); + router.post("users.info", auth(), async (ctx) => { ctx.body = { data: presentUser(ctx.state.user), diff --git a/server/api/users.test.js b/server/api/users.test.js index 6c667de5f..43cce4546 100644 --- a/server/api/users.test.js +++ b/server/api/users.test.js @@ -2,7 +2,8 @@ import TestServer from "fetch-test-server"; import app from "../app"; -import { buildUser } from "../test/factories"; +import { buildTeam, buildUser } from "../test/factories"; + import { flushdb, seed } from "../test/support"; const server = new TestServer(app.callback()); @@ -353,3 +354,75 @@ describe("#users.activate", () => { expect(body).toMatchSnapshot(); }); }); + +describe("#users.count", () => { + it("should count active users", async () => { + const team = await buildTeam(); + const user = await buildUser({ teamId: team.id }); + const res = await server.post("/api/users.count", { + body: { token: user.getJwtToken() }, + }); + const body = await res.json(); + + expect(res.status).toEqual(200); + expect(body.data.counts.all).toEqual(1); + expect(body.data.counts.admins).toEqual(0); + expect(body.data.counts.invited).toEqual(0); + expect(body.data.counts.suspended).toEqual(0); + expect(body.data.counts.active).toEqual(1); + }); + + it("should count admin users", async () => { + const team = await buildTeam(); + const user = await buildUser({ teamId: team.id, isAdmin: true }); + const res = await server.post("/api/users.count", { + body: { token: user.getJwtToken() }, + }); + const body = await res.json(); + + expect(res.status).toEqual(200); + expect(body.data.counts.all).toEqual(1); + expect(body.data.counts.admins).toEqual(1); + expect(body.data.counts.invited).toEqual(0); + expect(body.data.counts.suspended).toEqual(0); + expect(body.data.counts.active).toEqual(1); + }); + + it("should count suspended users", async () => { + const team = await buildTeam(); + const user = await buildUser({ teamId: team.id }); + await buildUser({ teamId: team.id, suspendedAt: new Date() }); + const res = await server.post("/api/users.count", { + body: { token: user.getJwtToken() }, + }); + const body = await res.json(); + + expect(res.status).toEqual(200); + expect(body.data.counts.all).toEqual(2); + expect(body.data.counts.admins).toEqual(0); + expect(body.data.counts.invited).toEqual(0); + expect(body.data.counts.suspended).toEqual(1); + expect(body.data.counts.active).toEqual(1); + }); + + it("should count invited users", async () => { + const team = await buildTeam(); + const user = await buildUser({ teamId: team.id, lastActiveAt: null }); + const res = await server.post("/api/users.count", { + body: { token: user.getJwtToken() }, + }); + const body = await res.json(); + + expect(res.status).toEqual(200); + expect(body.data.counts.all).toEqual(1); + expect(body.data.counts.admins).toEqual(0); + expect(body.data.counts.invited).toEqual(1); + expect(body.data.counts.suspended).toEqual(0); + expect(body.data.counts.active).toEqual(0); + }); + + it("should require authentication", async () => { + const res = await server.post("/api/users.count"); + expect(res.status).toEqual(401); + }); +}); diff --git a/server/models/User.js b/server/models/User.js index d11f94966..5670d7fe5 100644 --- a/server/models/User.js +++ b/server/models/User.js @@ -51,6 +51,9 @@ const User = sequelize.define( isSuspended() { return !!this.suspendedAt; }, + isInvited() { + return !this.lastActiveAt; + }, avatarUrl() { const original = this.getDataValue("avatarUrl"); if (original) { @@ -267,4 +270,33 @@ User.afterCreate(async (user, options) => { ]); }); +User.getCounts = async function (teamId: string) { + const countSql = ` + SELECT + COUNT(CASE WHEN "suspendedAt" IS NOT NULL THEN 1 END) as "suspendedCount", + COUNT(CASE WHEN "isAdmin" = true THEN 1 END) as "adminCount", + COUNT(CASE WHEN "lastActiveAt" IS NULL THEN 1 END) as "invitedCount", + COUNT(CASE WHEN "suspendedAt" IS NULL AND "lastActiveAt" IS NOT NULL THEN 1 END) as "activeCount", + COUNT(*) as count + FROM users + WHERE "deletedAt" IS NULL + AND "teamId" = :teamId + `; + const results = await sequelize.query(countSql, { + type: sequelize.QueryTypes.SELECT, + replacements: { + teamId, + }, + }); + const counts = results[0]; + + return { + active: parseInt(counts.activeCount), + admins: parseInt(counts.adminCount), + all: parseInt(counts.count), + invited: parseInt(counts.invitedCount), + suspended: parseInt(counts.suspendedCount), + }; +}; + export default User; diff --git a/shared/i18n/locales/en_US/translation.json b/shared/i18n/locales/en_US/translation.json index d242db6dc..98e5a9893 100644 --- a/shared/i18n/locales/en_US/translation.json +++ b/shared/i18n/locales/en_US/translation.json @@ -306,6 +306,12 @@ "Use the {{ meta }}+K shortcut to search from anywhere in your knowledge base": "Use the {{ meta }}+K shortcut to search from anywhere in your knowledge base", "No documents found for your search filters. <1>Create a new document?": "No documents found for your search filters. <1>Create a new document?", "Clear filters": "Clear filters", + "Everyone that has signed into Outline appears here. It’s possible that there are other users who have access through {team.signinMethods} but haven’t signed in yet.": "Everyone that has signed into Outline appears here. It’s possible that there are other users who have access through {team.signinMethods} but haven’t signed in yet.", + "Active": "Active", + "Admins": "Admins", + "Suspended": "Suspended", + "Everyone": "Everyone", + "No people to see here.": "No people to see here.", "Profile saved": "Profile saved", "Profile picture updated": "Profile picture updated", "Unable to upload new profile picture": "Unable to upload new profile picture", @@ -323,7 +329,6 @@ "You joined": "You joined", "Joined": "Joined", "{{ time }} ago.": "{{ time }} ago.", - "Suspended": "Suspended", "Edit Profile": "Edit Profile", "{{ userName }} hasn’t updated any documents yet.": "{{ userName }} hasn’t updated any documents yet." }