Object creation with dialogues

[colin-grant-work]

This PR adjusts the current dialog system to request all required information before creating a file.

It is an intermediate stage between the current custom dialog system and one based around reuse of the form widget to allow users to create new objects without triggering errors that make the form unusable.

To do

• Make sure UI tests pass.

They behave a little differently locally, so I'm creating the PR to get CI running. Will ping when ready for review.

[harmen-xb]

@colin-grant-work I understand the playwright tests will fail because the creation is now going through a dialog.

If it takes a lot of time to make it work (>2 hours), I would choose to disable and park these tests till we refactored to using the form on more forgiving grammar setup (which I discussed with @martin-fleck-at this afternoon).

We can accept manual testing these interactions for the coming weeks, since the first thing for April will be implementing the new creation flow anyway.

[github-actions]

Unit Test Results

  3 files  + 1   27 suites  +9   2m 29s ⏱️ +16s
 42 tests + 1   42 ✅ + 1  0 💤 ±0  0 ❌ ±0 
126 runs  +44  126 ✅ +44  0 💤 ±0  0 ❌ ±0 

Results for commit 1b2a489. ± Comparison against base commit 16b277e.

♻️ This comment has been updated with latest results.

[colin-grant-work]

@harmen-xb, thanks for the pass :-). It looks like only one test was failing, so it was a quick job to fix them, fortunately.

[harmen-xb]

@colin-grant-work Is this PR ready to be reviewed?

[colin-grant-work]

@harmen-xb, should be, yes. I believe all creation flows are working without validation errors.

[harmen-xb]

@colin-grant-work @martin-fleck-at

I reviewed the changes and this implementation solves issue #81. That's nice, but not the full scope of the story yet.

The key problem we had, is that files were created before the name (and thus id) of the object is known. This was the case in the file explorer and the system diagram.

The implementation up till now solves it for the file explorer, but not the diagram.

For example, when I create an entity in a system diagram the file is immediately created (NewEntity1.entity.cm for example), before I have entered a functional name and pressed enter. So the file name is incorrect from the start. This problem only exists for Entity objects in the system diagram (a.t.m.).
When creating relationships in the system diagram there is already logic which derives the relationship-name based on the parent and child (which should be equal behavior as when using the dialog and not entering a Name).

From both starting points (file explorer and system diagram) the end-user will fill in the name (and not the id) of the object. That name should be translated into an id (so strip whitespaces and remove special chars) and use the id as the base for the filename (so add extension).

Requested changes:

• I notice the name field in the dialog now is very restrictive (it doesn't allow spaces and special characters), can you change this to conform to the grammar?
• For object types which only require a name, the dialog doesn't specify a label for the input. Can you add it so it's clear the 'Name' is asked?
• Add logic to 'Create Entity' diagram tool so entity files are created after the name is entered (and derive id + filename based on it)

[colin-grant-work]

Add logic to 'Create Entity' diagram tool so entity files are created after the name is entered (and derive id + filename based on it)

Am I right that this is the crux of the matter? Basically, you're happy with the behavior from the file tree entry point, but would like the same behavior when entering the entity creation flow from a diagram?

I notice the name field in the dialog now is very restrictive (it doesn't allow spaces and special characters), can you change this to conform to the grammar?

Happy to switch this, but I want to make sure that I understand correctly, as I don't believe I changed the validation logic. Here it is on main, and here it is before my first PR. But the goal now is to allow users to input names conforming to the grammar for names and then transform them appropriately for the ID field?

[harmen-xb]

Add logic to 'Create Entity' diagram tool so entity files are created after the name is entered (and derive id + filename based on it)

Am I right that this is the crux of the matter? Basically, you're happy with the behavior from the file tree entry point, but would like the same behavior when entering the entity creation flow from a diagram?

Yes 😄

I notice the name field in the dialog now is very restrictive (it doesn't allow spaces and special characters), can you change this to conform to the grammar?

Happy to switch this, but I want to make sure that I understand correctly, as I don't believe I changed the validation logic. Here it is on main, and here it is before my first PR. But the goal now is to allow users to input names conforming to the grammar for names and then transform them appropriately for the ID field?

Alright, I get your point. I wasn't aware this validation was already on the input of creating files before.

From now going forward I would rather only ask the name property of modelled objects to the user and derive the id automatically. The filename should then be {id}.{objectype}.cm. For example a user creates a new entity with the name 'Example Entity', the the id should be 'Example_Entity' and the filename 'Example_Entity.entity.cm'. The filename is validated with this validation:

crossmodel/extensions/crossmodel-lang/src/language-server/cross-model-validator.ts

Lines 101 to 123 in efc3b7d

	protected checkMatchingFilename(node: AstNode, accept: ValidationAcceptor): void {
	if (!isSemanticRoot(node)) {
	return;
	}
	if (!node.id) {
	// diagrams may not have ids set and therefore are not required to match the filename
	return;
	}
	const document = findDocument(node);
	if (!document) {
	return;
	}
	const basename = UriUtils.basename(document.uri);
	const extname = ModelFileExtensions.getFileExtension(basename) ?? UriUtils.extname(document.uri);
	const basenameWithoutExt = basename.slice(0, -extname.length);
	if (basenameWithoutExt.toLowerCase() !== node.id.toLocaleLowerCase()) {
	accept('warning', `Filename should match element id: ${node.id}`, {
	node,
	property: ID_PROPERTY,
	data: { code: CrossModelIssueCodes.FilenameNotMatching }
	});
	}
	}

The above is true for all modelled objects, but I think the data model itself will deviate, because a data model in essence is a npm package (with a package.json file with a name in it). The name property of the data model has different restrictions because of the use of npm (The "name" field contains your package's name and must be lowercase without any spaces. May contain hyphens, dots, and underscores.)

The use of id, name and filename can be confusing I think, but hopefully the above helps.

[colin-grant-work]

• I notice the name field in the dialog now is very restrictive (it doesn't allow spaces and special characters), can you change this to conform to the grammar?
• For object types which only require a name, the dialog doesn't specify a label for the input. Can you add it so it's clear the 'Name' is asked?
• Add logic to 'Create Entity' diagram tool so entity files are created after the name is entered (and derive id + filename based on it)

@harmen-xb, I've pushed one commit that addresses the second of your requests and ensures that all the dialogues have a label for the 'Name' part and a second that addresses the first, that we treat the input as a name - very free in its form - and derive the ID from it. I consider the second commit a bit riskier, since I'm less sure about what structure we want to impose on the name.

Regarding the third request, to add the same flow to the entity creation from diagrams, I've asked @martin-fleck-at to have a look at it. Things behave differently in the diagram because we currently create the file before we get any user input at all so that the entity exists for the diagram to refer to - otherwise it goes locks up because of an invalid reference. I'm not as familiar with the GLSP side of things, so Martin has a better chance of getting it done in time for you to have a look at it before the demo.

[martin-fleck-at]

• Add logic to 'Create Entity' diagram tool so entity files are created after the name is entered (and derive id + filename based on it)
  Regarding the third request, to add the same flow to the entity creation from diagrams, I've asked @martin-fleck-at to have a look at it. Things behave differently in the diagram because we currently create the file before we get any user input at all so that the entity exists for the diagram to refer to - otherwise it goes locks up because of an invalid reference. I'm not as familiar with the GLSP side of things, so Martin has a better chance of getting it done in time for you to have a look at it before the demo.

Unfortunately I don't have much time to look at the diagram creation right now and focused on #116 instead. I think getting the diagram creation in would be a great next step but I don't believe it should prevent this PR from being merged (as soon as the CI is happy), what do you think @harmen-xb?

[harmen-xb]

@colin-grant-work Thanks for the update, entering the name feels much more user friendly.

I tested creating all object types, and the creation works but there one issue:

• When creating a relationship the parent and child drop-downs only work when creating the relationship in a folder named "relationships". So for new data models I have to manually create a folder named "relationships" and then right click on that folder and create the new relationship in order to get the drop-downs populated with the entities in the local model.

For now I can use this branch as base for the demo, I can work around the issue, but before merging I think this should be resolved.

[colin-grant-work]

When creating a relationship the parent and child drop-downs only work when creating the relationship in a folder named "relationships".

Hm... that would be an odd condition. The code just queries the server for referenceable entities in scope. It's possible that the scope computation at the top level (i.e. without any subfolders) goes wrong. I'll take a look at that.

[colin-grant-work]

@harmen-xb, it looks like the current scope computation does not recognize that the directory in which a package.json resides belongs to the scope of that package.json. To me, that seems incorrect, but a naive attempt to change that logic results in some test failures in the completion provider. I can probably fix the symptom by providing a dummy child path for the documents we're planning to create - would that be alright for the current requirements?

[colin-grant-work]

Or maybe it doesn't cause issues with the tests - after my first run failed, subsequent runs passed. Could have been a funny state of the build.

[harmen-xb]

@colin-grant-work Thanks! Now it works as expected.