(core) Fixing UserManger releted tests

Summary: Some tests were not compatible with the new ACUser search component.

Test Plan: Existing

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3643

app/client/lib/ACUserManager.ts

@@ -1,11 +1,8 @@
-import {ACResults, ACIndex, ACItem, buildHighlightedDom, normalizeText} from "app/client/lib/ACIndex";
+import {ACIndex, ACItem, ACResults, buildHighlightedDom, normalizeText} from "app/client/lib/ACIndex";
 import {cssSelectItem} from "app/client/lib/ACSelect";
 import {Autocomplete, IAutocompleteOptions} from "app/client/lib/autocomplete";
-import {cssMenuItem} from "popweasel";
-
-import {testId, colors, theme} from "app/client/ui2018/cssVars";
+import {colors, testId, theme} from "app/client/ui2018/cssVars";
 import {menuCssClass} from "app/client/ui2018/menus";
-import {dom, DomElementArg, Holder, IDisposableOwner, Observable, styled, computed, Computed} from "grainjs";
 import {
   cssEmailInput,
   cssEmailInputContainer,
@@ -17,6 +14,8 @@ import {
   cssMemberText,
 } from "app/client/ui/UserItem";
 import {createUserImage, cssUserImage} from "app/client/ui/UserImage";
+import {Computed, computed, dom, DomElementArg, Holder, IDisposableOwner, Observable, styled} from "grainjs";
+import {cssMenuItem} from "popweasel";
 
 export interface ACUserItem extends ACItem {
   value: string;
@@ -33,16 +32,17 @@ export function buildACMemberEmail(
   options: {
     acIndex: ACIndex<ACUserItem>;
     emailObs: Observable<string>;
-    save: (value: string) => Promise<void> | void;
-    isInputValid: Observable<boolean>;
+    save: (value: string) => void;
     prompt?: {email: string},
   },
   ...args: DomElementArg[]
 ) {
-  const { acIndex, emailObs, save, isInputValid, prompt } = options;
+  const {acIndex, emailObs, save, prompt} = options;
   const acHolder = Holder.create<Autocomplete<ACUserItem>>(owner);
   let emailInput: HTMLInputElement;
 
+  const isValid = Observable.create(owner, true);
+
   const isOpen = () => !acHolder.isEmpty();
   const acOpen = () => acHolder.isEmpty() && Autocomplete.create(acHolder, emailInput, acOptions);
   const acClose = () => acHolder.clear();
@@ -52,7 +52,7 @@ export function buildACMemberEmail(
     emailInput.value = emailObs.get();
     emailInput.focus();
   };
-  const openOrCommit = () => {
+  const onEnter = () => {
     isOpen() ? commitIfValid() : acOpen();
   };
 
@@ -62,14 +62,16 @@ export function buildACMemberEmail(
       emailObs.set(item.value);
     }
     emailInput.setCustomValidity("");
+    isValid.set(emailInput.checkValidity());
+
     const selectedEmail = item?.value || emailObs.get();
     try {
-      if (selectedEmail && isInputValid.get()) {
+      if (selectedEmail && isValid.get()) {
         save(emailObs.get());
         finish();
       }
     } catch (e) {
-        emailInput.setCustomValidity(e.message);
+      emailInput.setCustomValidity(e.message);
     } finally {
       emailInput.reportValidity();
     }
@@ -79,7 +81,7 @@ export function buildACMemberEmail(
     const cleanText = normalizeText(text);
     const items = results.items
       .filter(item => item.cleanText.includes(cleanText))
-      .sort((a,b) => a.cleanText.localeCompare(b.cleanText));
+      .sort((a, b) => a.cleanText.localeCompare(b.cleanText));
     results.items = items;
     if (!results.items.length) {
       const newObject = {
@@ -102,7 +104,7 @@ export function buildACMemberEmail(
         "+",
         cssUserImage.cls("-large"),
         cssUserImagePlus.cls('-invalid', (use) => !use(enableAdd),
-      )),
+        )),
       cssMemberText(
         cssMemberPrimaryPlus("Invite new member"),
         cssMemberSecondaryPlus(
@@ -121,7 +123,7 @@ export function buildACMemberEmail(
     )
   ));
 
-  const enableAdd: Computed<boolean> = computed((use) => Boolean(use(emailObs) && use(isInputValid)));
+  const enableAdd: Computed<boolean> = computed((use) => Boolean(use(emailObs) && use(isValid)));
 
   const acOptions: IAutocompleteOptions<ACUserItem> = {
     attach: null,
@@ -136,7 +138,7 @@ export function buildACMemberEmail(
     cssMailIcon("Mail"),
     (emailInput = cssEmailInput(
       emailObs,
-      {onInput: true, isValid: isInputValid},
+      {onInput: true, isValid},
       {type: "email", placeholder: "Enter email address"},
       dom.on("input", acOpen),
       dom.on("focus", acOpen),
@@ -144,51 +146,43 @@ export function buildACMemberEmail(
       dom.on("blur", acClose),
       dom.onKeyDown({
         Escape: finish,
-        Enter: openOrCommit,
+        Enter: onEnter,
         ArrowDown: acOpen,
         Tab: commitIfValid,
       }),
-      ...args
-      )),
+    )),
     cssEmailInputContainer.cls('-green', enableAdd),
+    ...args
   );
 
+  // Reset custom validity that we sometimes set.
+  owner.autoDispose(emailObs.addListener(() => emailInput.setCustomValidity("")));
+
   if (prompt) { setTimeout(() => emailInput.focus(), 0); }
 
   return result;
 }
 
-const cssMemberPrimaryPlus = styled(
-  cssMemberPrimary,
-  `
+const cssMemberPrimaryPlus = styled(cssMemberPrimary, `
   .${cssSelectItem.className}.selected & {
     color: ${theme.menuItemSelectedFg};
   }
-`
-);
+`);
 
-const cssMemberSecondaryPlus = styled(
-  cssMemberSecondary,
-  `
+const cssMemberSecondaryPlus = styled(cssMemberSecondary, `
   .${cssSelectItem.className}.selected & {
     color: ${theme.menuItemSelectedFg};
   }
-`
-);
+`);
 
-const cssMatchText = styled(
-  "span",
-  `
+const cssMatchText = styled("span", `
   color: ${theme.autocompleteMatchText};
   .${cssSelectItem.className}.selected & {
     color: ${theme.autocompleteSelectedMatchText};
   }
-`
-);
+`);
 
-const cssUserImagePlus = styled(
-  cssUserImage,
-  `
+const cssUserImagePlus = styled(cssUserImage, `
   background-color: ${colors.lightGreen};
   margin: auto 0;
 
@@ -200,5 +194,4 @@ const cssUserImagePlus = styled(
     background-color: ${theme.menuItemIconSelectedFg};
     color: ${theme.menuItemSelectedBg};
   }
-`
-);
+`);

app/client/lib/autocomplete.ts

@@ -21,6 +21,11 @@ export interface IAutocompleteOptions<Item extends ACItem> {
   // Popper options for positioning the popup.
   popperOptions?: Partial<PopperOptions>;
 
+  // To which element to append the popup content. Null means triggerElem.parentNode; string is
+  // a selector for the closest matching ancestor of triggerElem, e.g. 'body'.
+  // Defaults to the document body.
+  attach?: Element|string|null;
+
   // Given a search term, return the list of Items to render.
   search(searchText: string): Promise<ACResults<Item>>;
 
@@ -32,10 +37,6 @@ export interface IAutocompleteOptions<Item extends ACItem> {
 
   // A callback triggered when user clicks one of the choices.
   onClick?(): void;
-
-  // To which element to append the popup content. Null means triggerElem.parentNode and is the
-  // default; string is a selector for the closest matching ancestor of triggerElem, e.g. 'body'.
-  attach?: Element|string|null;
 }
 
 /**

app/client/models/UserManagerModel.ts

@@ -32,6 +32,8 @@ export interface UserManagerModel {
   activeUser: FullUser|null;                   // Populated if current user is logged in.
   gristDoc: GristDoc|null;                     // Populated if there is an open document.
 
+  // Analyze the relation that users have to the resource or site.
+  annotate(): void;
   // Resets all unsaved changes
   reset(): void;
   // Recreate annotations, factoring in any changes on the back-end.
@@ -253,7 +255,6 @@ export class UserManagerModelImpl extends Disposable implements UserManagerModel
     return member.email === this.activeUser?.email;
   }
 
-  // Analyze the relation that users have to the resource or site.
   public annotate() {
     // Only attempt for documents for now.
     // TODO: extend to workspaces.

app/client/ui/UserManager.ts

@@ -10,7 +10,7 @@ import {isLongerThan} from 'app/common/gutil';
 import {FullUser} from 'app/common/LoginSessionAPI';
 import * as roles from 'app/common/roles';
 import {Organization, PermissionData, UserAPI} from 'app/common/UserAPI';
-import {Computed, Disposable, keyframes, observable, Observable, dom, DomElementArg, styled} from 'grainjs';
+import {Computed, Disposable, dom, DomElementArg, keyframes, Observable, observable, styled} from 'grainjs';
 import pick = require('lodash/pick');
 
 import {ACIndexImpl, normalizeText} from 'app/client/lib/ACIndex';
@@ -204,9 +204,8 @@ export class UserManager extends Disposable {
   }
 
   public buildDom() {
-    const acmemberEmail = this.autoDispose(new ACMemberEmail(
+    const acMemberEmail = this.autoDispose(new ACMemberEmail(
       this._onAdd.bind(this),
-      (member) => this._model.isActiveUser(member),
       this._model.membersEdited.get(),
       this._options.prompt,
     ));
@@ -219,7 +218,7 @@ export class UserManager extends Disposable {
     }
 
     return [
-      acmemberEmail.buildDom(),
+      acMemberEmail.buildDom(),
       this._buildOptionsDom(),
       this._dom = shadowScroll(
         testId('um-members'),
@@ -531,7 +530,7 @@ function getUserItem(member: IEditableMember): ACUserItem {
     name: member.name,
     picture: member?.picture,
     id: member.id,
-  }
+  };
 }
 
 /**
@@ -539,11 +538,9 @@ function getUserItem(member: IEditableMember): ACUserItem {
  */
 export class ACMemberEmail extends Disposable {
   private _email = this.autoDispose(observable<string>(""));
-  private _isValid = this.autoDispose(observable<boolean>(false));
 
   constructor(
     private _onAdd: (email: string, role: roles.NonGuestRole) => void,
-    private _isActiveUser: (member: IEditableMember) => boolean,
     private _members: Array<IEditableMember>,
     private _prompt?: {email: string}
   ) {
@@ -562,7 +559,6 @@ export class ACMemberEmail extends Disposable {
         acIndex,
         emailObs: this._email,
         save: this._handleSave.bind(this),
-        isInputValid: this._isValid,
         prompt: this._prompt,
       },
       testId('um-member-new')
@@ -570,12 +566,7 @@ export class ACMemberEmail extends Disposable {
   }
 
   private _handleSave(selectedEmail: string) {
-    const member = this._members.find(member => member.email === selectedEmail);
-    if (!member) {
-      this._onAdd(selectedEmail, roles.VIEWER);
-    } else if (!this._isActiveUser(member)) {
-      member?.effectiveAccess.set(roles.VIEWER);
-    }
+    this._onAdd(selectedEmail, roles.VIEWER);
   }
 }