Ability to create share url slug (#4550)

* feat: share url slug

* feat: add col urlId

* feat: allow updating urlId

* fix: typo

* fix: migrations

* fix: urlId model validation

* fix: input label

* fix: debounce slug request

* feat: link preview

* fix: send slug variant in response if available

* fix: temporary redirect to slug variant if available

* fix: move up the custom link field

* fix: process and display backend err

* fix: reset custom link state on popover close and remove isCopied

* fix: document link preview

* fix: set urlId when available

* fix: keep unique(urlId, teamId)

* fix: codeql

* fix: get rid of preview type

* fix: width not needed for block elem

* fix: migrations

* fix: array not required

* fix: use val

* fix: validation on shareId and test

* fix: allow clearing urlId

* fix: do not escape

* fix: unique error text

* fix: keep team

app/components/Input.tsx

@@ -5,6 +5,7 @@ import { VisuallyHidden } from "reakit/VisuallyHidden";
 import styled from "styled-components";
 import breakpoint from "styled-components-breakpoint";
 import Flex from "~/components/Flex";
+import Text from "~/components/Text";
 import { undraggableOnDesktop } from "~/styles";
 
 const RealTextarea = styled.textarea<{ hasIcon?: boolean }>`
@@ -120,6 +121,7 @@ export type Props = React.InputHTMLAttributes<
   flex?: boolean;
   short?: boolean;
   margin?: string | number;
+  error?: string;
   icon?: React.ReactNode;
   innerRef?: React.Ref<any>;
   onFocus?: (ev: React.SyntheticEvent) => unknown;
@@ -155,6 +157,7 @@ class Input extends React.Component<Props> {
       icon,
       label,
       margin,
+      error,
       className,
       short,
       flex,
@@ -197,11 +200,26 @@ class Input extends React.Component<Props> {
             )}
           </Outline>
         </label>
+        <TextWrapper>
+          <StyledText type="danger" size="xsmall">
+            {error}
+          </StyledText>
+        </TextWrapper>
       </Wrapper>
     );
   }
 }
 
+export const TextWrapper = styled.span`
+  min-height: 16px;
+  display: block;
+  margin-top: -16px;
+`;
+
+export const StyledText = styled(Text)`
+  margin-bottom: 0;
+`;
+
 export const ReactHookWrappedInput = React.forwardRef(
   (props: Omit<Props, "innerRef">, ref: React.Ref<any>) => {
     return <Input {...{ ...props, innerRef: ref }} />;

app/components/Text.ts

@@ -1,7 +1,7 @@
 import styled from "styled-components";
 
 type Props = {
-  type?: "secondary" | "tertiary";
+  type?: "secondary" | "tertiary" | "danger";
   size?: "large" | "small" | "xsmall";
 };
 
@@ -16,6 +16,8 @@ const Text = styled.p<Props>`
       ? props.theme.textSecondary
       : props.type === "tertiary"
       ? props.theme.textTertiary
+      : props.type === "danger"
+      ? props.theme.brand.red
       : props.theme.text};
   font-size: ${(props) =>
     props.size === "large"

app/models/Share.ts

@@ -20,6 +20,10 @@ class Share extends BaseModel {
   @observable
   documentId: string;
 
+  @Field
+  @observable
+  urlId: string;
+
   documentTitle: string;
 
   documentUrl: string;

app/scenes/Document/components/SharePopover.tsx

@@ -1,15 +1,21 @@
 import { formatDistanceToNow } from "date-fns";
 import invariant from "invariant";
+import { debounce, isEmpty } from "lodash";
 import { observer } from "mobx-react";
 import { ExpandedIcon, GlobeIcon, PadlockIcon } from "outline-icons";
 import * as React from "react";
 import { useTranslation, Trans } from "react-i18next";
 import styled from "styled-components";
+import { SHARE_URL_SLUG_REGEX } from "@shared/utils/urlHelpers";
 import Document from "~/models/Document";
 import Share from "~/models/Share";
 import Button from "~/components/Button";
 import CopyToClipboard from "~/components/CopyToClipboard";
 import Flex from "~/components/Flex";
+import Input, {
+  TextWrapper,
+  StyledText as DocumentLinkPreview,
+} from "~/components/Input";
 import Notice from "~/components/Notice";
 import Switch from "~/components/Switch";
 import Text from "~/components/Text";
@@ -40,9 +46,10 @@ function SharePopover({
   const { t } = useTranslation();
   const { shares } = useStores();
   const { showToast } = useToasts();
-  const [isCopied, setIsCopied] = React.useState(false);
   const [expandedOptions, setExpandedOptions] = React.useState(false);
   const [isEditMode, setIsEditMode] = React.useState(false);
+  const [slugValidationError, setSlugValidationError] = React.useState("");
+  const [urlSlug, setUrlSlug] = React.useState("");
   const timeout = React.useRef<ReturnType<typeof setTimeout>>();
   const buttonRef = React.useRef<HTMLButtonElement>(null);
   const can = usePolicy(share ? share.id : "");
@@ -73,6 +80,13 @@ function SharePopover({
     return () => (timeout.current ? clearTimeout(timeout.current) : undefined);
   }, [document, visible, team.sharing]);
 
+  React.useEffect(() => {
+    if (!visible) {
+      setUrlSlug(share?.urlId || "");
+      setSlugValidationError("");
+    }
+  }, [share, visible]);
+
   const handlePublishedChange = React.useCallback(
     async (event) => {
       const share = shares.getByDocumentId(document.id);
@@ -110,9 +124,7 @@ function SharePopover({
   );
 
   const handleCopied = React.useCallback(() => {
-    setIsCopied(true);
     timeout.current = setTimeout(() => {
-      setIsCopied(false);
       onRequestClose();
       showToast(t("Share link copied"), {
         type: "info",
@@ -120,6 +132,38 @@ function SharePopover({
     }, 250);
   }, [t, onRequestClose, showToast]);
 
+  const handleUrlSlugChange = React.useMemo(
+    () =>
+      debounce(async (ev) => {
+        const share = shares.getByDocumentId(document.id);
+        invariant(share, "Share must exist");
+
+        const val = ev.target.value;
+        setUrlSlug(val);
+        if (val && !SHARE_URL_SLUG_REGEX.test(val)) {
+          setSlugValidationError(
+            t("Only lowercase letters, digits and dashes allowed")
+          );
+        } else {
+          setSlugValidationError("");
+          if (share.urlId !== val) {
+            try {
+              await share.save({
+                urlId: isEmpty(val) ? null : val,
+              });
+            } catch (err) {
+              if (err.message.includes("must be unique")) {
+                setSlugValidationError(
+                  t("Sorry, this link has already been used")
+                );
+              }
+            }
+          }
+        }
+      }, 500),
+    [t, document.id, shares]
+  );
+
   const userLocale = useUserLocale();
   const locale = userLocale ? dateLocale(userLocale) : undefined;
   let shareUrl = team.sharing ? share?.url ?? "" : `${team.url}${document.url}`;
@@ -211,6 +255,31 @@ function SharePopover({
 
       {expandedOptions && (
         <>
+          <Separator />
+          <SwitchWrapper>
+            <Input
+              type="text"
+              label={t("Custom link")}
+              onChange={handleUrlSlugChange}
+              error={slugValidationError}
+              defaultValue={urlSlug}
+            />
+            {!slugValidationError && urlSlug && (
+              <DocumentLinkPreviewWrapper>
+                <DocumentLinkPreview type="secondary" size="small">
+                  <Trans>The document will be available at</Trans>
+                  <br />
+                  <a
+                    href={urlSlug ? `${team.url}/s/${urlSlug}` : ""}
+                    target="_blank"
+                    rel="noopener noreferrer"
+                  >
+                    {urlSlug ? `${team.url}/s/${urlSlug}` : ""}
+                  </a>
+                </DocumentLinkPreview>
+              </DocumentLinkPreviewWrapper>
+            )}
+          </SwitchWrapper>
           <Separator />
           <SwitchWrapper>
             <Switch
@@ -252,7 +321,7 @@ function SharePopover({
         <CopyToClipboard text={shareUrl} onCopy={handleCopied}>
           <Button
             type="submit"
-            disabled={isCopied || (!share && team.sharing)}
+            disabled={(!share && team.sharing) || slugValidationError}
             ref={buttonRef}
           >
             {t("Copy link")}
@@ -301,4 +370,8 @@ const SwitchText = styled(Text)`
   font-size: 15px;
 `;
 
+const DocumentLinkPreviewWrapper = styled(TextWrapper)`
+  margin-top: -12px;
+`;
+
 export default observer(SharePopover);

server/commands/documentLoader.ts

@@ -1,5 +1,7 @@
 import invariant from "invariant";
-import { Op } from "sequelize";
+import { Op, WhereOptions } from "sequelize";
+import isUUID from "validator/lib/isUUID";
+import { SHARE_URL_SLUG_REGEX } from "@shared/utils/urlHelpers";
 import {
   NotFoundError,
   InvalidRequestError,
@@ -12,6 +14,7 @@ import { authorize, can } from "@server/policies";
 type Props = {
   id?: string;
   shareId?: string;
+  teamId?: string;
   user?: User;
   includeState?: boolean;
 };
@@ -25,6 +28,7 @@ type Result = {
 export default async function loadDocument({
   id,
   shareId,
+  teamId,
   user,
   includeState,
 }: Props): Promise<Result> {
@@ -36,14 +40,35 @@ export default async function loadDocument({
     throw AuthenticationError(`Authentication or shareId required`);
   }
 
+  const shareUrlId =
+    shareId && !isUUID(shareId) && SHARE_URL_SLUG_REGEX.test(shareId)
+      ? shareId
+      : undefined;
+
+  if (shareUrlId && !teamId) {
+    throw InvalidRequestError(
+      "teamId required for fetching share using shareUrlId"
+    );
+  }
+
   if (shareId) {
-    share = await Share.findOne({
+    let whereClause: WhereOptions<Share> = {
-      where: {
+      revokedAt: {
+        [Op.is]: null,
+      },
+      id: shareId,
+    };
+    if (shareUrlId) {
+      whereClause = {
         revokedAt: {
           [Op.is]: null,
         },
-        id: shareId,
+        teamId,
-      },
+        urlId: shareUrlId,
+      };
+    }
+    share = await Share.findOne({
+      where: whereClause,
       include: [
         {
           // unscoping here allows us to return unpublished documents

server/migrations/20221206163421-add-share-url-slug.js

@@ -0,0 +1,43 @@
+"use strict";
+
+module.exports = {
+  async up(queryInterface, Sequelize) {
+    try {
+      await queryInterface.sequelize.transaction(async (transaction) => {
+        await queryInterface.addColumn(
+          "shares",
+          "urlId",
+          {
+            type: Sequelize.STRING,
+            allowNull: true,
+            transaction,
+          },
+        );
+
+        await queryInterface.addConstraint("shares", {
+          fields: ["urlId", "teamId"],
+          type: "unique",
+          transaction,
+        });
+      });
+    } catch(err) {
+      throw err;
+    }
+  },
+
+  async down(queryInterface, Sequelize) {
+    try {
+      await queryInterface.sequelize.transaction(async (transaction) => {
+        await queryInterface.removeConstraint(
+          "shares",
+          "shares_urlId_teamId_uk",
+          { transaction }
+        );
+
+        await queryInterface.removeColumn("shares", "urlId", { transaction });
+      });
+    } catch (err) {
+      throw err;
+    }
+  },
+};

server/models/Share.ts

@@ -7,7 +7,10 @@ import {
   Scopes,
   DataType,
   Default,
+  AllowNull,
+  Is,
 } from "sequelize-typescript";
+import { SHARE_URL_SLUG_REGEX } from "@shared/utils/urlHelpers";
 import Collection from "./Collection";
 import Document from "./Document";
 import Team from "./Team";
@@ -85,6 +88,14 @@ class Share extends IdModel {
   @Column
   views: number;
 
+  @AllowNull
+  @Is({
+    args: SHARE_URL_SLUG_REGEX,
+    msg: "Must be only alphanumeric and dashes",
+  })
+  @Column
+  urlId: string | null | undefined;
+
   // getters
 
   get isRevoked() {
@@ -92,7 +103,9 @@ class Share extends IdModel {
   }
 
   get canonicalUrl() {
-    return `${this.team.url}/s/${this.id}`;
+    return this.urlId
+      ? `${this.team.url}/s/${this.urlId}`
+      : `${this.team.url}/s/${this.id}`;
   }
 
   // associations

server/presenters/share.ts

@@ -9,6 +9,7 @@ export default function present(share: Share, isAdmin = false) {
     documentUrl: share.document?.url,
     published: share.published,
     url: share.canonicalUrl,
+    urlId: share.urlId,
     createdBy: presentUser(share.user),
     includeChildDocuments: share.includeChildDocuments,
     lastAccessedAt: share.lastAccessedAt || undefined,

server/routes/api/documents/documents.test.ts

@@ -466,10 +466,12 @@ describe("#documents.info", () => {
   it("should require a valid shareId", async () => {
     const res = await server.post("/api/documents.info", {
       body: {
-        shareId: 123,
+        shareId: "share_id",
       },
     });
+    const body = await res.json();
     expect(res.status).toEqual(400);
+    expect(body.message).toEqual("shareId: Invalid input");
   });
 });
 

server/routes/api/documents/documents.ts

@@ -41,6 +41,7 @@ import {
   presentPolicies,
 } from "@server/presenters";
 import { APIContext } from "@server/types";
+import { getTeamFromContext } from "@server/utils/passport";
 import slugify from "@server/utils/slugify";
 import { assertPresent } from "@server/validation";
 import env from "../../../env";
@@ -394,10 +395,12 @@ router.post(
   async (ctx: APIContext<T.DocumentsInfoReq>) => {
     const { id, shareId, apiVersion } = ctx.input;
     const { user } = ctx.state;
+    const teamFromCtx = await getTeamFromContext(ctx);
     const { document, share, collection } = await documentLoader({
       id,
       shareId,
       user,
+      teamId: teamFromCtx?.id,
     });
     const isPublic = cannot(user, "read", document);
     const serializedDocument = await presentDocument(document, {

server/routes/api/documents/schema.ts

@@ -1,5 +1,7 @@
 import { isEmpty } from "lodash";
+import isUUID from "validator/lib/isUUID";
 import { z } from "zod";
+import { SHARE_URL_SLUG_REGEX } from "@shared/utils/urlHelpers";
 
 const DocumentsSortParamsSchema = z.object({
   /** Specifies the attributes by which documents will be sorted in the list */
@@ -98,7 +100,10 @@ export const DocumentsInfoSchema = z
     id: z.string().optional(),
 
     /** Share Id, if available */
-    shareId: z.string().uuid().optional(),
+    shareId: z
+      .string()
+      .refine((val) => isUUID(val) || SHARE_URL_SLUG_REGEX.test(val))
+      .optional(),
 
     /** Version of the API to be used */
     apiVersion: z.number().optional(),

server/routes/api/shares.test.ts

@@ -464,6 +464,70 @@ it("should require authorization", async () => {
 });
 
 describe("#shares.update", () => {
+  it("should fail for invalid urlId", async () => {
+    const { user, document } = await seed();
+    const share = await buildShare({
+      documentId: document.id,
+      teamId: user.teamId,
+    });
+    const res = await server.post("/api/shares.update", {
+      body: {
+        token: user.getJwtToken(),
+        id: share.id,
+        urlId: "url_id",
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(400);
+    expect(body.message).toEqual(
+      "Must be only alphanumeric and dashes (urlId)"
+    );
+  });
+
+  it("should update urlId", async () => {
+    const { user, document } = await seed();
+    const share = await buildShare({
+      documentId: document.id,
+      teamId: user.teamId,
+    });
+    const res = await server.post("/api/shares.update", {
+      body: {
+        token: user.getJwtToken(),
+        id: share.id,
+        urlId: "url-id",
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.urlId).toEqual("url-id");
+  });
+
+  it("should allow clearing urlId", async () => {
+    const { user, document } = await seed();
+    const share = await buildShare({
+      documentId: document.id,
+      teamId: user.teamId,
+    });
+    await server.post("/api/shares.update", {
+      body: {
+        token: user.getJwtToken(),
+        id: share.id,
+        urlId: "url-id",
+      },
+    });
+
+    const res = await server.post("/api/shares.update", {
+      body: {
+        token: user.getJwtToken(),
+        id: share.id,
+        urlId: null,
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.urlId).toBeNull();
+  });
+
   it("should allow user to update a share", async () => {
     const { user, document } = await seed();
     const share = await buildShare({

server/routes/api/shares.ts

@@ -1,4 +1,5 @@
 import Router from "koa-router";
+import { isUndefined } from "lodash";
 import { Op, WhereOptions } from "sequelize";
 import { NotFoundError } from "@server/errors";
 import auth from "@server/middlewares/authentication";
@@ -162,7 +163,7 @@ router.post("shares.list", auth(), pagination(), async (ctx) => {
 });
 
 router.post("shares.update", auth(), async (ctx) => {
-  const { id, includeChildDocuments, published } = ctx.request.body;
+  const { id, includeChildDocuments, published, urlId } = ctx.request.body;
   assertUuid(id, "id is required");
 
   const { user } = ctx.state;
@@ -191,6 +192,10 @@ router.post("shares.update", auth(), async (ctx) => {
     share.includeChildDocuments = includeChildDocuments;
   }
 
+  if (!isUndefined(urlId)) {
+    share.urlId = urlId;
+  }
+
   await share.save();
   await Event.create({
     name: "shares.update",

server/routes/app.ts

@@ -4,9 +4,11 @@ import util from "util";
 import { Context, Next } from "koa";
 import { escape } from "lodash";
 import { Sequelize } from "sequelize";
+import isUUID from "validator/lib/isUUID";
 import documentLoader from "@server/commands/documentLoader";
 import env from "@server/env";
 import presentEnv from "@server/presenters/env";
+import { getTeamFromContext } from "@server/utils/passport";
 import prefetchTags from "@server/utils/prefetchTags";
 
 const isProduction = env.ENVIRONMENT === "production";
@@ -87,11 +89,19 @@ export const renderShare = async (ctx: Context, next: Next) => {
   let share, document;
 
   try {
+    const team = await getTeamFromContext(ctx);
     const result = await documentLoader({
       id: documentSlug,
       shareId,
+      teamId: team?.id,
     });
     share = result.share;
+    if (isUUID(shareId) && share && share.urlId) {
+      // Redirect temporarily because the url slug
+      // can be modified by the user at any time
+      ctx.redirect(`/s/${share.urlId}`);
+      ctx.status = 307;
+    }
     document = result.document;
 
     if (share && !ctx.userAgent.isBot) {

shared/i18n/locales/en_US/translation.json

@@ -475,6 +475,8 @@
   "Backlinks": "Backlinks",
   "Anyone with the link <1></1>can view this document": "Anyone with the link <1></1>can view this document",
   "Share": "Share",
+  "Only lowercase letters, digits and dashes allowed": "Only lowercase letters, digits and dashes allowed",
+  "Sorry, this link has already been used": "Sorry, this link has already been used",
   "Share this document": "Share this document",
   "This document is shared because the parent <em>{{ documentTitle }}</em> is publicly shared": "This document is shared because the parent <em>{{ documentTitle }}</em> is publicly shared",
   "Publish to internet": "Publish to internet",
@@ -484,6 +486,8 @@
   "Share nested documents": "Share nested documents",
   "Nested documents are publicly available": "Nested documents are publicly available",
   "Nested documents are not shared": "Nested documents are not shared",
+  "Custom link": "Custom link",
+  "The document will be available at": "The document will be available at",
   "Automatically redirect to the editor": "Automatically redirect to the editor",
   "Users with edit permission will be redirected to the main app": "Users with edit permission will be redirected to the main app",
   "All users see the same publicly shared view": "All users see the same publicly shared view",

shared/utils/urlHelpers.ts

@@ -53,3 +53,5 @@ export function signin(service = "slack"): string {
 }
 
 export const SLUG_URL_REGEX = /^(?:[0-9a-zA-Z-_~]*-)?([a-zA-Z0-9]{10,15})$/;
+
+export const SHARE_URL_SLUG_REGEX = /^[0-9a-z-]+$/;