Bit of refactoring in Projects (#1105)

* fix name and id in Workspace and Project

* tidy

* remove log

* changesets

Author: josephjclark

.changeset/blue-mails-beam.md

@@ -0,0 +1,5 @@
+---
+'@openfn/engine-multi': patch
+---
+
+Remove log line

.changeset/loud-cities-add.md

@@ -0,0 +1,5 @@
+---
+'@openfn/project': patch
+---
+
+Internal refactor for clarity

packages/engine-multi/src/worker/thread/run.ts

@@ -86,7 +86,6 @@ register({
         },
       },
     };
-    logger.log(' >>', options.profile, options.profilePollInterval);
 
     return execute(plan.id!, async () => {
       const start = Date.now();

packages/project/package.json

@@ -37,6 +37,7 @@
     "@openfn/lexicon": "workspace:^",
     "@openfn/logger": "workspace:*",
     "glob": "^11.0.2",
+    "human-id": "^4.1.1",
     "lodash": "^4.17.21",
     "lodash-es": "^4.17.21",
     "ohm-js": "^17.2.1",

packages/project/src/Project.ts

@@ -1,3 +1,4 @@
+import { humanId } from 'human-id';
 import Workflow from './Workflow';
 import * as serializers from './serialize';
 import fromAppState, { FromAppStateConfig } from './parse/from-app-state';
@@ -19,47 +20,22 @@ type MergeOptions = {
 const maybeCreateWorkflow = (wf: any) =>
   wf instanceof Workflow ? wf : new Workflow(wf);
 
-// TODO --------------
-// I think this needs renaming to config
-// and it's part of the workspace technically
-// I need to support custom props
-// When serializing, for now, we always write defaults
-// --------------
-// repo-wide options
-type RepoOptions = {
-  /**default workflow root when serializing to fs (relative to openfn.yaml) */
-  // TODO deprecate this
-  workflowRoot?: string;
-
-  formats: {
-    openfn: FileFormats;
-    workflow: FileFormats;
-    project: FileFormats;
-  };
-};
-
-// A local collection of openfn projects?
-// class Repo {
-
-//   projects: {}
-// }
-
-// TODO maybe use an npm for this, or create  util
-
-// TODO this need to be controlled by the workspace
-
 // A single openfn project
 // could be an app project or a checked out fs
 export class Project {
   // what schema version is this?
   // And how are we tracking this?
   // version;
 
-  /** project name */
+  /** Human readable project name. This corresponds to the label in Lightning */
   name?: string;
+
+  /** Project id. Must be url safe. May be derived from the name. NOT a UUID */
+  id: string;
+
   description?: string;
 
-  // array of version shas
+  // array of version hashes
   history: string[] = [];
 
   workflows: Workflow[];
@@ -79,12 +55,6 @@ export class Project {
 
   config: WorkspaceConfig;
 
-  // load a project from a state file (project.json)
-  // or from a path (the file system)
-  // TODO presumably we can detect a state file? Not a big deal?
-
-  // collections for the project
-  // TODO to be well typed
   collections: any;
 
   static from(
@@ -129,10 +99,16 @@ export class Project {
   // stuff that's external to the actual project and managed by the repo
 
   // TODO maybe the constructor is (data, Workspace)
-  constructor(data: l.Project, repoConfig: RepoOptions = {}) {
-    this.setConfig(repoConfig);
+  constructor(data: l.Project, config: RepoOptions = {}) {
+    this.setConfig(config);
+
+    this.id =
+      data.id ?? data.name
+        ? slugify(data.name)
+        : humanId({ separator: '-', capitalize: false });
 
     this.name = data.name;
+
     this.description = data.description;
     this.openfn = data.openfn;
     this.options = data.options;
@@ -154,19 +130,12 @@ export class Project {
     throw new Error(`Cannot serialize ${type}`);
   }
 
-  // would like a better name for this
-  // stamp? id? sha?
-  // this builds a version string for the current state
-  getVersionHash() {}
-
-  // what else might we need?
-
-  // get workflow by name or id
-  // this is fuzzy, but is that wrong?
+  // get workflow by name, id or uuid
   getWorkflow(idOrName: string) {
     return (
       this.workflows.find((wf) => wf.id == idOrName) ||
-      this.workflows.find((wf) => wf.name === idOrName)
+      this.workflows.find((wf) => wf.name === idOrName) ||
+      this.workflows.find((wf) => wf.openfn?.uuid === idOrName)
     );
   }
 

packages/project/src/Workspace.ts

@@ -16,7 +16,7 @@ const PROJECT_EXTENSIONS = ['.yaml', '.yml'];
 
 export class Workspace {
   config?: WorkspaceConfig;
-  projectMeta: ProjectMeta;
+  activeProject: ProjectMeta;
 
   private projects: Project[] = [];
   private projectPaths = new Map<string, string>();
@@ -26,7 +26,6 @@ export class Workspace {
     let context;
     try {
       const { type, content } = findWorkspaceFile(workspacePath);
-      console.log(content);
       context = loadWorkspaceFile(content, type);
       this.isValid = true;
     } catch (e) {
@@ -36,7 +35,7 @@ export class Workspace {
     }
 
     this.config = buildConfig(context.workspace);
-    this.projectMeta = context.project;
+    this.activeProject = context.project;
 
     const projectsPath = path.join(workspacePath, this.config.dirs.projects);
 
@@ -73,9 +72,12 @@ export class Workspace {
     return this.projects;
   }
 
-  // TODO clear up name/id confusion
+  /** Get a project by its id or UUID */
   get(id: string) {
-    return this.projects.find((p) => p.name === id);
+    return (
+      this.projects.find((p) => p.id === id) ??
+      this.projects.find((p) => p.openfn?.uuid === id)
+    );
   }
 
   getProjectPath(id: string) {
@@ -84,7 +86,7 @@ export class Workspace {
 
   getActiveProject() {
     // TODO should use id, not name
-    return this.projects.find((p) => p.name === this.projectMeta?.name);
+    return this.projects.find((p) => p.id === this.activeProject?.id);
   }
 
   // TODO this needs to return default values
@@ -94,8 +96,7 @@ export class Workspace {
   }
 
   get activeProjectId() {
-    // TODO should return activeProject.id
-    return this.projectMeta?.name;
+    return this.activeProject?.id;
   }
 
   get valid() {

packages/project/src/parse/from-fs.ts

@@ -53,6 +53,7 @@ export const parseProject = async (options: FromFsConfig = {}) => {
   }
 
   const proj = {
+    name: state?.name,
     openfn: context.project,
     config: config,
     workflows: [],
@@ -81,7 +82,7 @@ export const parseProject = async (options: FromFsConfig = {}) => {
         const wfState = (state && state.getWorkflow(wf.id)) ?? {};
         wf.openfn = {
           uuid: wfState.openfn?.uuid ?? null,
-          // TODO do we need to transfer more stuff?
+          // TODO do we need to transfer more stuff? Options maybe?
         };
 
         //console.log('Loading workflow at ', filePath); // TODO logger.debug

packages/project/src/util/config.ts

@@ -146,13 +146,10 @@ export const loadWorkspaceFile = (
 };
 
 export const findWorkspaceFile = (dir: string = '.') => {
-  console.log({ dir });
   let content, type;
   try {
     type = 'yaml';
-    console.log(path.resolve(path.join(dir, 'openfn.yaml')));
     content = readFileSync(path.resolve(path.join(dir, 'openfn.yaml')), 'utf8');
-    console.log({ content });
   } catch (e) {
     // Not found - try and parse as JSON
     try {

packages/project/test/workspace.test.ts

@@ -5,27 +5,28 @@ import test from 'ava';
 // TODO need a test on the legacy and new yaml formats here
 mock({
   '/ws/openfn.yaml': jsonToYaml({
-    name: 'some-project-name',
     project: {
+      id: 'project-1',
       uuid: '1234',
-      name: 'some-project-name',
     },
-    formats: {
-      openfn: 'yaml',
-      project: 'yaml',
-      workflow: 'yaml',
-      custom: true, // Note tha this will be excluded
+    workspace: {
+      formats: {
+        openfn: 'yaml',
+        project: 'yaml',
+        workflow: 'yaml',
+        custom: true, // Note that this will be excluded
+      },
+      // deliberately exclude dirs
+      custom: true,
     },
-    // deliberately exclude dirs
-    custom: true,
   }),
   '/ws/.projects/staging@app.openfn.org.yaml': jsonToYaml({
-    id: 'some-id',
-    name: 'some-project-name',
+    name: 'Project 1',
+    id: '<uuid-1>',
     workflows: [
       {
-        name: 'simple-workflow',
-        id: 'wf-id',
+        name: 'simple-workflow', // TODO clean up
+        id: 'wf-id', // TODO clean up
         jobs: [
           {
             name: 'Transform data to FHIR standard',
@@ -97,18 +98,18 @@ test('workspace-list: list projects in the workspace', (t) => {
   const ws = new Workspace('/ws');
   t.is(ws.list().length, 1);
   t.deepEqual(
-    ws.list().map((w) => w.name),
-    ['some-project-name']
+    ws.list().map((w) => w.id),
+    ['project-1']
   );
 });
 
 test('workspace-get: get projects in the workspace', (t) => {
   const ws = new Workspace('/ws');
-  const found = ws.get('some-project-name');
+  const found = ws.get('project-1');
   t.truthy(found);
   t.is(found?.workflows.length, 2);
   t.deepEqual(
-    found?.workflows.map((w) => w.name),
+    found?.workflows.map((w) => w.id),
     ['simple-workflow', 'another-workflow']
   );
 });
@@ -131,8 +132,8 @@ test('load config', (t) => {
 
 test('load project meta', (t) => {
   const ws = new Workspace('/ws');
-  t.deepEqual(ws.projectMeta, {
+  t.deepEqual(ws.activeProject, {
     uuid: '1234',
-    name: 'some-project-name',
+    id: 'project-1',
   });
 });