Note deletion, events testing, and improved CI

Added API and events around note deletion

Improved github workflows, added logic to avoid duplicate runs in CI and merged build + test jobs

Added support for running workflows in multiple environments, commented window-2019 as test don't pass, but they will be fixed in another PR to avoid scope creep here.

.github/workflows/ci.yml

@@ -0,0 +1,48 @@
+name: CI
+
+on:
+ push:
+ branches:
+ - '**'
+ # The following will also make the workflow run on all PRs, internal and external.
+ # This would create duplicate runs, that we are skipping by adding the "if" to the jobs below.
+ # See https://github.community/t/duplicate-checks-on-push-and-pull-request-simultaneous-event/18012
+ pull_request:
+
+jobs:
+ lint:
+ name: Lint
+ runs-on: ubuntu-18.04
+ if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != 'foambubble/foam'
+ steps:
+ - uses: actions/checkout@v1
+ - name: Setup Node
+ uses: actions/setup-node@v1
+ with:
+ node-version: '12'
+ - name: Install Dependencies
+ run: yarn
+ - name: Check Lint Rules
+ run: yarn lint
+
+ test:
+ name: Build and Test
+ strategy:
+ matrix:
+ os: [macos-10.15, ubuntu-18.04] # add windows-2019 after fixing tests for it
+ runs-on: ${{ matrix.os }}
+ if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != 'foambubble/foam'
+ env:
+ OS: ${{ matrix.os }}
+ steps:
+ - uses: actions/checkout@v1
+ - name: Setup Node
+ uses: actions/setup-node@v1
+ with:
+ node-version: '12'
+ - name: Install Dependencies
+ run: yarn
+ - name: Build Packages
+ run: yarn build
+ - name: Run Tests
+ run: yarn test

packages/foam-core/src/bootstrap.ts

@@ -33,7 +33,8 @@ export const bootstrap = async (config: FoamConfig, services: Services) => {
  graph.setNote(await parser.parse(uri, content));
  });
  services.dataStore.onDidDelete(async uri => {
- // TODO add deleteNote to graph
+ const note = graph.getNoteByURI(uri);
+ note && graph.deleteNote(note.id);
  });
 
  return {

packages/foam-core/src/markdown-provider.ts

@@ -15,7 +15,6 @@ import {
  uriToSlug,
  extractHashtags,
  extractTagsFromProp,
- nameToSlug,
 } from './utils';
 import { ID } from './types';
 import { ParserPlugin } from './plugins';

packages/foam-core/src/note-graph.ts

@@ -1,6 +1,6 @@
 import { Graph } from 'graphlib';
 import { URI, ID, Note, NoteLink } from './types';
-import { computeRelativeURI, nameToSlug } from './utils';
+import { computeRelativeURI, nameToSlug, isSome } from './utils';
 import { Event, Emitter } from './common/event';
 
 export type GraphNote = Note & {
@@ -19,6 +19,7 @@ export type NotesQuery = { slug: string } | { title: string };
 
 export interface NoteGraphAPI {
  setNote(note: Note): GraphNote;
+ deleteNote(noteId: ID): GraphNote | null;
  getNotes(query?: NotesQuery): GraphNote[];
  getNote(noteId: ID): GraphNote | null;
  getNoteByURI(uri: URI): GraphNote | null;
@@ -27,7 +28,7 @@ export interface NoteGraphAPI {
  getBacklinks(noteId: ID): GraphConnection[];
  onDidAddNote: Event<GraphNote>;
  onDidUpdateNote: Event<GraphNote>;
- onDidRemoveNote: Event<GraphNote>;
+ onDidDeleteNote: Event<GraphNote>;
 }
 
 export type Middleware = (next: NoteGraphAPI) => Partial<NoteGraphAPI>;
@@ -40,30 +41,25 @@ export const createGraph = (middlewares: Middleware[]): NoteGraphAPI => {
 export class NoteGraph implements NoteGraphAPI {
  onDidAddNote: Event<GraphNote>;
  onDidUpdateNote: Event<GraphNote>;
- onDidRemoveNote: Event<GraphNote>;
+ onDidDeleteNote: Event<GraphNote>;
 
  private graph: Graph;
  private createIdFromURI: (uri: URI) => ID;
  private onDidAddNoteEmitter = new Emitter<GraphNote>();
  private onDidUpdateNoteEmitter = new Emitter<GraphNote>();
- private onDidRemoveNoteEmitter = new Emitter<GraphNote>();
+ private onDidDeleteEmitter = new Emitter<GraphNote>();
 
  constructor() {
  this.graph = new Graph();
  this.onDidAddNote = this.onDidAddNoteEmitter.event;
  this.onDidUpdateNote = this.onDidUpdateNoteEmitter.event;
- this.onDidRemoveNote = this.onDidRemoveNoteEmitter.event;
+ this.onDidDeleteNote = this.onDidDeleteEmitter.event;
  this.createIdFromURI = uri => uri;
  }
 
  public setNote(note: Note): GraphNote {
  const id = this.createIdFromURI(note.source.uri);
- const noteExists = this.graph.hasNode(id);
- if (noteExists) {
- (this.graph.outEdges(id) || []).forEach(edge => {
- this.graph.removeEdge(edge);
- });
- }
+ const oldNote = this.doDelete(id, false);
  const graphNote: GraphNote = {
  ...note,
  id: id,
@@ -81,12 +77,28 @@ export class NoteGraph implements NoteGraphAPI {
  };
  this.graph.setEdge(graphNote.id, targetId, connection);
  });
- noteExists
+ isSome(oldNote)
  ? this.onDidUpdateNoteEmitter.fire(graphNote)
  : this.onDidAddNoteEmitter.fire(graphNote);
  return graphNote;
  }
 
+ public deleteNote(noteId: ID): GraphNote | null {
+ return this.doDelete(noteId, true);
+ }
+
+ private doDelete(noteId: ID, fireEvent: boolean): GraphNote | null {
+ const note = this.getNote(noteId);
+ if (isSome(note)) {
+ this.graph.removeNode(noteId);
+ (this.graph.outEdges(noteId) || []).forEach(edge => {
+ this.graph.removeEdge(edge);
+ });
+ fireEvent && this.onDidDeleteEmitter.fire(note);
+ }
+ return note;
+ }
+
  public getNotes(query?: NotesQuery): GraphNote[] {
  // prettier-ignore
  const filterFn =
@@ -130,14 +142,15 @@ export class NoteGraph implements NoteGraphAPI {
  public dispose() {
  this.onDidAddNoteEmitter.dispose();
  this.onDidUpdateNoteEmitter.dispose();
- this.onDidRemoveNoteEmitter.dispose();
+ this.onDidDeleteEmitter.dispose();
  }
 }
 
 const backfill = (next: NoteGraphAPI, middleware: Middleware): NoteGraphAPI => {
  const m = middleware(next);
  return {
  setNote: m.setNote || next.setNote,
+ deleteNote: m.deleteNote || next.deleteNote,
  getNotes: m.getNotes || next.getNotes,
  getNote: m.getNote || next.getNote,
  getNoteByURI: m.getNoteByURI || next.getNoteByURI,
@@ -146,6 +159,6 @@ const backfill = (next: NoteGraphAPI, middleware: Middleware): NoteGraphAPI => {
  getBacklinks: m.getBacklinks || next.getBacklinks,
  onDidAddNote: next.onDidAddNote,
  onDidUpdateNote: next.onDidUpdateNote,
- onDidRemoveNote: next.onDidRemoveNote,
+ onDidDeleteNote: next.onDidDeleteNote,
  };
 };

packages/foam-core/src/utils/hashtags.ts

@@ -1,4 +1,4 @@
-import { isSome, isNumeric } from './core';
+import { isSome } from './core';
 const HASHTAG_REGEX = /(^|[ ])#([\w_-]*[a-zA-Z][\w_-]*\b)/gm;
 const WORD_REGEX = /(^|[ ])([\w_-]*[a-zA-Z][\w_-]*\b)/gm;
 

packages/foam-core/test/core.test.ts

@@ -188,6 +188,93 @@ describe('Graph querying', () => {
  });
 });
 
+describe('graph events', () => {
+ it('fires "add" event when adding a new note', () => {
+ const graph = new NoteGraph();
+ const callback = jest.fn();
+ const listener = graph.onDidAddNote(callback);
+ graph.setNote(
+ createTestNote({ uri: '/dir1/page-a.md', title: 'My Title' })
+ );
+ expect(callback).toHaveBeenCalledTimes(1);
+ listener.dispose();
+ });
+ it('fires "updated" event when changing an existing note', () => {
+ const graph = new NoteGraph();
+ const callback = jest.fn();
+ const listener = graph.onDidUpdateNote(callback);
+ graph.setNote(
+ createTestNote({ uri: '/dir1/page-a.md', title: 'My Title' })
+ );
+ graph.setNote(
+ createTestNote({ uri: '/dir1/page-a.md', title: 'Another title' })
+ );
+ expect(callback).toHaveBeenCalledTimes(1);
+ listener.dispose();
+ });
+ it('fires "delete" event when removing a note', () => {
+ const graph = new NoteGraph();
+ const callback = jest.fn();
+ const listener = graph.onDidDeleteNote(callback);
+ const note = graph.setNote(
+ createTestNote({ uri: '/dir1/page-a.md', title: 'My Title' })
+ );
+ graph.deleteNote(note.id);
+ expect(callback).toHaveBeenCalledTimes(1);
+ listener.dispose();
+ });
+ it('does not fire "delete" event when removing a non-existing note', () => {
+ const graph = new NoteGraph();
+ const callback = jest.fn();
+ const listener = graph.onDidDeleteNote(callback);
+ const note = graph.setNote(
+ createTestNote({ uri: '/dir1/page-a.md', title: 'My Title' })
+ );
+ graph.deleteNote('non-existing-note');
+ expect(callback).toHaveBeenCalledTimes(0);
+ listener.dispose();
+ });
+ it('happy lifecycle', () => {
+ const graph = new NoteGraph();
+ const addCallback = jest.fn();
+ const updateCallback = jest.fn();
+ const deleteCallback = jest.fn();
+ const listeners = [
+ graph.onDidAddNote(addCallback),
+ graph.onDidUpdateNote(updateCallback),
+ graph.onDidDeleteNote(deleteCallback),
+ ];
+
+ const note = graph.setNote(
+ createTestNote({ uri: '/dir1/page-a.md', title: 'My Title' })
+ );
+ expect(addCallback).toHaveBeenCalledTimes(1);
+ expect(updateCallback).toHaveBeenCalledTimes(0);
+ expect(deleteCallback).toHaveBeenCalledTimes(0);
+
+ graph.setNote(
+ createTestNote({ uri: '/dir1/page-a.md', title: 'Another Title' })
+ );
+ expect(addCallback).toHaveBeenCalledTimes(1);
+ expect(updateCallback).toHaveBeenCalledTimes(1);
+ expect(deleteCallback).toHaveBeenCalledTimes(0);
+
+ graph.setNote(
+ createTestNote({ uri: '/dir1/page-a.md', title: 'Yet Another Title' })
+ );
+ expect(addCallback).toHaveBeenCalledTimes(1);
+ expect(updateCallback).toHaveBeenCalledTimes(2);
+ expect(deleteCallback).toHaveBeenCalledTimes(0);
+
+ graph.deleteNote(note.id);
+ expect(addCallback).toHaveBeenCalledTimes(1);
+ expect(updateCallback).toHaveBeenCalledTimes(2);
+ expect(deleteCallback).toHaveBeenCalledTimes(1);
+
+ listeners.forEach(l => l.dispose());
+ });
+});
+
 describe('graph middleware', () => {
  it('can intercept calls to the graph', async () => {
  const graph = createGraph([

packages/foam-vscode/src/features/dataviz.ts

@@ -18,9 +18,11 @@ const feature: FoamFeature = {
 
  const noteAddedListener = foam.notes.onDidAddNote(onFoamChanged);
  const noteUpdatedListener = foam.notes.onDidUpdateNote(onFoamChanged);
+ const noteDeletedListener = foam.notes.onDidDeleteNote(onFoamChanged);
  panel.onDidDispose(() => {
  noteAddedListener.dispose();
  noteUpdatedListener.dispose();
+ noteDeletedListener.dispose();
  });
 
  vscode.window.onDidChangeActiveTextEditor(e => {