Closed philippfromme closed 2 years ago
To consider: Does a user need to construct the path herself? Or does bpmnlint take care of it?
The fundamental BPMNLint feature we're talking about here is: Be able to address sub-paths within an element.
Another way to think about it: We could also turn this thing upside down.
No path is introduced, instead elements are reported using the standard way:
Reporter.report(zeebeTaskDefinition, 'must have <zeebe:type> property');
// or
Reporter.report(buildPath(serviceTask, zebeeTaskDefinition), 'must have <zeebe:type> property');
@nikku That's a good question. We could introduce this as a general feature, yes.
The utility for getting the path would have to be available in the respective plugins (e.g. bpmnlint-plugin-camunda-compat). I'm not sure what Reporter.report(buildPath(serviceTask, zebeeTaskDefinition), 'must have <zeebe:type> property');
would look like? Reporter.report([ 'ServiceTask_1', 'extensionElements', 0 'type' ], 'must have <zeebe:type> property');
? That would introduce a breaking change and also require us to parse the path assuming the first key is the element's ID. I'm not a huge fan of that idea. My current approach looks like this:
class Reporter {
constructor({ moddleRoot, rule }) {
this.rule = rule;
this.moddleRoot = moddleRoot;
this.messages = [];
this.report = this.report.bind(this);
}
report(id, message, options) {
let report = {
id,
message
};
if (options) {
report = {
...report,
options // optional
};
}
this.messages.push(report);
}
}
The rule itself would look something like this:
// ...
return {
message: 'Element of type <zeebe:TaskDefinition> must have <zeebe:type> property',
path: [ ...getPath(taskDefinition, node), 'zeebe:type' ] // get the path of `taskDefinition` relative to `node`
};
As @smbea is going to start working on https://github.com/camunda/bpmnlint-plugin-camunda-compat/issues/7 we need to come to an agreement regarding the solution for this issue.
@nikku What are your thoughts? Shall we have a chat about this?
How would the CLI output look like in your idea?
We have to think this end-to-end, and if that means a major breaking change, I'd be fine with it.
I guess you'd expect something similar to:
> bpmnlint invoice.bpmn
/Projects/process-application/resources/invoice.bpmn
ServiceTask_1#extensionElements.values.0 error Element of type <zeebe:TaskDefinition> must have <zeebe:type> property camunda-cloud-1-0
✖ 1 problem (1 error, 0 warnings)
In that case we'd probably make the path concept known to bpmnlint. 🤔
The API could still look like Reporter#report(id, message, path)
with path
being the only optional parameter. The CLI output could be formatted accordingly.
Had a short conversation with @philippfromme on the topic:
Reporter#report(id, message, path: [string|number][])
$parent
for traversal (bpmnlint, bpmn-js-properties-panel)
moddle
does not guarantee that it is being maintained properlyCreated https://github.com/bpmn-io/moddle/issues/41 as a follow-up. I don't think we'll be able to tackle it in due time.
Sketch:
// getPath will be required not only by bpmnlint plugins (e.g. bpmnlint-plugin-camunda-compat)
// but also by bpmn-js-properties-panel
import { getPath } from '@bpmn-io/moddle-helpers';
// ...
reporter.report('ServiceTask_1', 'Task definition missing');
reporter.report('ServiceTask_1', 'Task definition type missing', [ 'extensionElements', 'values', 0, 'type' ]); // create path manually
reporter.report('ServiceTask_1', 'Task definition retries missing', [ ...getPath(moddleElement, parentModdleElement), 'retries' ]); // create path using utility
console.log(reporter.messages);
// [
// {
// id: 'ServiceTask_1',
// message: 'Task definition missing'
// },
// {
// id: 'ServiceTask_1',
// message: 'Task definition type missing',
// path: [ 'extensionElements', 'values', 0, 'type' ]
// },
// {
// id: 'ServiceTask_1',
// message: 'Task definition retries missing',
// path: [ 'extensionElements', 'values', 0, 'retries' ]
// }
// ]
CLI output:
> bpmnlint invoice.bpmn
/Projects/process-application/resources/invoice.bpmn
ServiceTask_1 error Task definition missing camunda-cloud-1-0
ServiceTask_1#extensionElements.values.0.type error Task definition type missing camunda-cloud-1-0
ServiceTask_1#extensionElements.values.0.retries error Task definition retries missing camunda-cloud-1-0
✖ 3 problems (3 errors, 0 warnings)
When using
Reporter#report
one can only provide the element ID and the error message. There is no way to provide additional properties e.g. the path of the property that has caused the error to be reported. I'd image something along the lines of:Adding an optional third argument wouldn't introduce any breaking changes.
Related to https://github.com/bpmn-io/internal-docs/issues/430