#675 More stable tests and added back commit queue

browser/data-browser/src/components/TableEditor/Cell.tsx

@@ -129,10 +129,14 @@ export function Cell({
         return;
       }
 
+      if (isActive && cursorMode === CursorMode.Edit) {
+        return;
+      }
+
       setCursorMode(CursorMode.Visual);
       setActiveCell(rowIndex, columnIndex);
     },
-    [setActiveCell, columnIndex, shouldEnterEditMode],
+    [setActiveCell, columnIndex, shouldEnterEditMode, cursorMode, isActive],
   );
 
   const handleClick = useCallback(() => {

browser/data-browser/src/routes/History/useVersions.ts

@@ -26,7 +26,7 @@ export function useVersions(resource: Resource): UseVersionsResult {
       .finally(() => {
         setLoading(false);
       });
-  }, [resource.getSubject()]);
+  }, [resource]);
 
   return { versions, loading, error };
 }

browser/data-browser/src/views/ChatRoomPage.tsx

@@ -413,19 +413,13 @@ function MessagesPage({ subject, setReplyTo }: MessagesPageProps) {
   const resource = useResource(subject);
   const [messages] = useArray(resource, properties.chatRoom.messages);
   const [nextPage] = useString(resource, properties.chatRoom.nextPage);
-  const [inView] = useState(true);
-  const ref = useRef<HTMLDivElement>(null);
-
-  if (!inView) {
-    return <>Not in view...</>;
-  }
 
   if (!resource.isReady()) {
     return <>loading...</>;
   }
 
   return (
-    <div ref={ref}>
+    <div>
       {nextPage && <MessagesPage subject={nextPage} setReplyTo={setReplyTo} />}
       {messages.map(message => (
         <Message

browser/data-browser/src/views/DocumentPage.tsx

@@ -72,7 +72,7 @@ function DocumentPageEdit({
   const [elements, setElements] = useArray(
     resource,
     properties.document.elements,
-    { commit: true, validate: false, commitDebounce: 0 },
+    { commit: false, validate: false, commitDebounce: 0 },
   );
 
   const titleRef = React.useRef<HTMLInputElement>(null);
@@ -201,9 +201,10 @@ function DocumentPageEdit({
         newElement.set(properties.parent, resource.getSubject(), store),
         newElement.set(properties.description, '', store),
       ]);
-      newElement.save(store);
       await setElements(newElements);
       focusElement(position);
+      await newElement.save(store);
+      await resource.save(store);
     } catch (e) {
       setErr(e);
     }
@@ -236,6 +237,7 @@ function DocumentPageEdit({
     if (elements.length === 1) {
       setElements([]);
       focusElement(0);
+      resource.save(store);
 
       return;
     }
@@ -254,6 +256,7 @@ function DocumentPageEdit({
       addElement(index + 1);
     } else {
       focusElement(index + 1);
+      resource.save(store);
     }
   }
 
@@ -263,6 +266,7 @@ function DocumentPageEdit({
     elements.splice(to, 0, element);
     setElements(elements);
     focusElement(to);
+    resource.save(store);
   }
 
   function handleSortEnd(event: DragEndEvent): void {
@@ -285,6 +289,7 @@ function DocumentPageEdit({
     toast.success('Upload succeeded!');
     fileSubjects.map(subject => elements.push(subject));
     setElements([...elements]);
+    resource.save(store);
   }
 
   /** Add a new line, or move to the last line if it is empty */

browser/data-browser/tests/documents.spec.ts

@@ -10,6 +10,7 @@ import {
   openNewSubjectWindow,
   timestamp,
   before,
+  waitForCommitOnCurrentResource,
 } from './test-utils';
 test.describe('documents', async () => {
   test.beforeEach(before);
@@ -36,6 +37,7 @@ test.describe('documents', async () => {
 
     // multi-user
     const currentSubject = await getCurrentSubject(page);
+    await waitForCommitOnCurrentResource(page);
     const page2 = await openNewSubjectWindow(browser, currentSubject!);
     await expect(
       page2.locator(`text=${teststring}`),

browser/data-browser/tests/e2e.spec.ts

@@ -33,6 +33,7 @@ import {
   waitForCommit,
   openAgentPage,
   fillSearchBox,
+  waitForCommitOnCurrentResource,
 } from './test-utils';
 
 test.describe('data-browser', async () => {
@@ -231,20 +232,13 @@ test.describe('data-browser', async () => {
       'Chat message not appearing directly after sending',
     ).toBeVisible();
 
-    const dropdownId = await page
-      .locator(contextMenu)
-      .getAttribute('aria-controls');
-
-    await page.click(contextMenu);
-    await page
-      .locator(`[id="${dropdownId}"] >> [data-test="menu-item-share"]`)
-      .click();
-    await publicReadRightLocator(page).click();
-    await page.click('text=save');
-
     const page2 = await openNewSubjectWindow(browser, chatRoomUrl);
     // Second user
     await signIn(page2);
+
+    // TODO: TEMP FIX, NO LONGER NEEDED IF #686 IS FIXED
+    page2.reload();
+
     await expect(page2.locator(`text=${teststring}`)).toBeVisible();
     const teststring2 = `My reply: ${timestamp()}`;
     await page2.fill('[data-test="message-input"]', teststring2);
@@ -298,7 +292,7 @@ test.describe('data-browser', async () => {
     // wait for commit debounce
     // await page.waitForTimeout(2000);
     // make sure no commits are waiting for each other
-    await page.waitForLoadState('networkidle');
+    await waitForCommitOnCurrentResource(page);
     await page.reload();
     await expect(
       page.locator(`text=${alphabet}`).first(),
@@ -438,7 +432,9 @@ test.describe('data-browser', async () => {
       page.locator(`[data-test="sidebar"] >> text=${d1}`),
       "Sidebar doesn't show child resource title",
     ).toBeVisible();
-    await page.waitForLoadState('networkidle');
+
+    await waitForCommitOnCurrentResource(page);
+
     await page.reload();
     await expect(
       page.locator(`[data-test="sidebar"] >> text=${d1}`),
@@ -551,8 +547,14 @@ test.describe('data-browser', async () => {
     await expect(
       page.getByRole('heading', { name: 'Second Title', level: 1 }),
     ).toBeVisible();
-    // Wait for commit debounce
-    await page.waitForTimeout(500);
+
+    // The history page does not update when the resource changes so we need to wait to be sure the commit is done
+    // before opening the history page.
+    await waitForCommitOnCurrentResource(page, {
+      set: {
+        ['https://atomicdata.dev/properties/name']: 'Second Title',
+      },
+    });
 
     await contextMenuClick('history', page);
 

browser/data-browser/tests/ontology.spec.ts

@@ -13,6 +13,8 @@ test.describe('Ontology', async () => {
   test.beforeEach(before);
 
   test('Create and edit ontology', async ({ page }) => {
+    test.slow();
+
     const pickOption = async (query: Locator) => {
       await page.waitForTimeout(100);
       await query.hover();

browser/data-browser/tests/tables.spec.ts

@@ -10,6 +10,8 @@ test.describe('tables', async () => {
   test.beforeEach(before);
 
   test('create and fill', async ({ page }) => {
+    test.slow();
+
     const newColumn = async (type: string) => {
       await page.getByRole('button', { name: 'Add column' }).click();
       await page.click(`text=${type}`);
@@ -51,7 +53,7 @@ test.describe('tables', async () => {
       await page.waitForTimeout(300);
       // Wait for the table to refresh by checking if the next row is visible
       await expect(
-        page.getByRole('rowheader', { name: `${currentRowNumber}` }),
+        page.getByRole('rowheader', { name: `${currentRowNumber + 1}` }),
       ).toBeAttached();
 
       await page.keyboard.type(date);

browser/data-browser/tests/test-utils.ts

@@ -125,6 +125,39 @@ export async function getCurrentSubject(page: Page) {
   return page.locator(addressBar).getAttribute('value');
 }
 
+export async function waitForCommitOnCurrentResource(
+  page: Page,
+  match?: { set?: Record<string, unknown> },
+) {
+  const currentSubject = await getCurrentSubject(page);
+
+  await page.waitForResponse(async response => {
+    const result = await response.json();
+
+    const isForCurrentResource =
+      result['https://atomicdata.dev/properties/subject'] === currentSubject;
+
+    if (!isForCurrentResource) {
+      return false;
+    }
+
+    if (match) {
+      const set = result['https://atomicdata.dev/properties/set'];
+
+      for (const key in match.set) {
+        if (set[key] !== match.set[key]) {
+          return false;
+        }
+      }
+    }
+
+    // Wait for commit response to be processed by the store.
+    await page.waitForTimeout(200);
+
+    return true;
+  });
+}
+
 export async function openAgentPage(page: Page) {
   page.goto(`${FRONTEND_URL}/app/agent`);
 }

browser/lib/src/resource.ts

@@ -60,6 +60,9 @@ export class Resource<C extends OptionalClass = any> {
   private subject: string;
   private propvals: PropVals;
 
+  private inProgressCommit: Promise<void> | undefined;
+  private hasQueue = false;
+
   public constructor(subject: string, newResource?: boolean) {
     if (typeof subject !== 'string') {
       throw new Error(
@@ -305,7 +308,7 @@ export class Resource<C extends OptionalClass = any> {
     for (const commit of commits as unknown as string[]) {
       const commitResource = await store.getResourceAsync(commit);
       const parsedCommit = parseCommitResource(commitResource);
-      const builtResource = await applyCommitToResource(
+      const builtResource = applyCommitToResource(
         previousResource.clone(),
         parsedCommit,
       );
@@ -467,13 +470,26 @@ export class Resource<C extends OptionalClass = any> {
       throw new Error('No agent has been set or passed, you cannot save.');
     }
 
+    if (this.hasQueue) {
+      return;
+    }
+
     // If the parent of this resource is new we can't save yet so we add it to a batched that gets saved when the parent does.
     if (this.isParentNew(store)) {
       store.batchResource(this.getSubject());
 
       return;
     }
 
+    if (this.inProgressCommit) {
+      this.hasQueue = true;
+      await this.inProgressCommit;
+      this.hasQueue = false;
+      this.inProgressCommit = undefined;
+
+      return this.save(store, differentAgent);
+    }
+
     // The previousCommit is required in Commits. We should use the `lastCommit` value on the resource.
     // This makes sure that we're making adjustments to the same version as the server.
     const lastCommit = this.get(properties.commit.lastCommit)?.toString();
@@ -484,6 +500,14 @@ export class Resource<C extends OptionalClass = any> {
 
     const wasNew = this.new;
 
+    let reportDone: () => void = () => undefined;
+
+    this.inProgressCommit = new Promise(resolve => {
+      reportDone = () => {
+        resolve();
+      };
+    });
+
     // Cloning the CommitBuilder to prevent race conditions, and keeping a back-up of current state for when things go wrong during posting.
     const oldCommitBuilder = this.commitBuilder.clone();
     this.commitBuilder = new CommitBuilder(this.getSubject());
@@ -520,6 +544,8 @@ export class Resource<C extends OptionalClass = any> {
         await store.saveBatchForParent(this.getSubject());
       }
 
+      reportDone();
+
       return createdCommit.id as string;
     } catch (e) {
       // Logic for handling error if the previousCommit is wrong.
@@ -540,13 +566,16 @@ export class Resource<C extends OptionalClass = any> {
         }
 
         // Try again!
+        reportDone();
+
         return await this.save(store, agent);
       }
 
       // If it fails, revert to the old resource with the old CommitBuilder
       this.commitBuilder = oldCommitBuilder;
       this.commitError = e;
       store.addResources(this, { skipCommitCompare: true });
+      reportDone();
       throw e;
     }
   }