nexedi / erp5

For reviewers: about erp5_json_form , there's also a good documentation on https://lab.nexedi.com/bk/rjs_json_form/blob/master/README.md ( and tests in https://lab.nexedi.com/bk/rjs_json_form/tree/master/test )

One thing I believe could be added to this readme is a few lines on how erp5_json_form "interprets" the json schema, to help json schema authors writing schema looking good in this UI. For example:

• string json schema type is rendered as as <input type="string">
• integer json schema type is rendered as as <input type="number">. ( how minimum and maximum are supported ? )
• number json <input type="number" step="0.01"> with step being XXX
• enum is rendered as a select, { "type": "string", "enum": ["Street", "Avenue", "Boulevard"] } is rendered as something like :

<select>
   <option value="Street">Street</option>
   <option value="Avenue">Avenue</option>
   <option value="Boulevard">Boulevard</option>

BTW, is there a way to use different display names, to produce something like :

<select>
   <option value="Street">Street - small road</option>
   <option value="Avenue">Avenue - larger road. This is the recommended option</option>
   <option value="Boulevard">Boulevard</option>

• default key in the json is used as the value of the field
• description does something ? maybe a title attribute on the field ? or

etc...

---

erp5_json_form seems cool. Is there already some planned usages ?

We have a few places where we generate form based on json schema, one used here in erp5_graph_editor, where we could use erp5_json_form instead of implementing a partial support (graph editor only supports number, text and list of theses ).

There's another one for slapos.vifib.com instance editor based on software release json schemas ( I don't know exactly where is the code )

[edit: fix the escaping of html that was rendered as actual HTML]

all this zip support in javascript is also pending for inclusion in jio as jio!38 isn't it ?

yes, it is.

isn't it better for tracebacks to keep a named function here ?

I understand there's already a local updatePanel function and that there's a naming conflict when we want to call the local updatePanel.

Can we rename the local functions ?

I use plural, because updateHeader is same in this file, so it would be nice to handle them in the same way.

i agree. i fix it in cc7f2930.

Someone with deeper knowledge of this code and of the integration plan should do a more in depth review, but I can say it looks nice.

I did a quick test of importing software/erp5's input schema in the editor (using "Upload and rewrite this document", so to edit the schema itself). At first I thought I broke it, but it turned out it was just taking time (probably rendering the many many widgets - this schema is large). After a few seconds, the schema was visible as widgets, and at a very quick glance it got it all correct. This is a very nice achievement !

Then, I replaced the meta-schema URL with software/erp5's input schema, so to edit some slapos parameters for an ERP5 instance. And again, after a few seconds it displayed stuff. It found errors in the schema thanks to this:

• /frontend/software-type was set as having type object, but a string by default. Type should be string.
• the longstanding issue of missing tcpv4-port definition.

I fixed these. I would gladly edit these using your UI (and actually did it initially), but could not find a way to get the modified schema file out of it. So I redid with a text editor.

Then, the remaining error is that it does not seem happy with an "additionalProperties", probably the one from "caucase" section. I am not sure why (but my jsonschema knowledge is getting rusty).

I also noticed that it did not access subschemas, like in:

    "mariadb": {
      "description": "Relational database service",
      "additionalProperties": {
        "$ref": "./instance-mariadb-schema.json#/properties"
      },
      "type": "object"
    },

While I can imagine we do not want to access arbitrary URL, the code already did: it fetched the base schema. So maybe it could go the extra mile and recursively fetch schemas ? Or maybe there is an error in how it is referenced/what it contains ?

I also loaded a set of real-world parameters for an ERP5 instance, and it was mostly fine. The only glith I could spot is on mariadb parameters:

{
  "mariadb": {
    "database-list": [
      {"name": "foo_prod"},
      {"name": "foo_prod2"}
    ]
  }
}

While this works, it produces a slightly-weird-looking pair of trashcans (to delete entries). I think list items should be displayed with their index next:

(trashcan) 0: (trashcan) <first item first property>
              (trashcan) <first item second property>
(trashcan) 1: (trashcan) <second item first property>

Overall, I am impressed by the technical achievement, it is very functional on one of our most complex schemas. On the UI level, I think there is room for improvements, but I am terrible at GUI design so I can't really criticise...

So maybe it could go the extra mile and recursively fetch schemas ?

To use it as a SlapOS instance parameter editor, I think it would be needed.

if I understand correctly, https://lab.nexedi.com/bk/rjs_json_form/blob/master/README.md explains that parent gadget can export a downloadJSON function for this exact purpose, so maybe it's just a matter of having a function like this pseudo-code:

rJS(window)
  .allowPublicAcquisition("downloadJSON", function (arr) {
     if (url.startsWith("urn:jio:reference?")) {
       ...
    }
   // fallback to network if it's a url 
   return fetch(base + arr[0]).then(function(r){ return r.json(); );
}

( with the proper jio api instead of fetch )

@jerome thank you for your comment, i corrected the documentation.

You were asking about a generation of the selection in which titles differ from values. Yes, this feature is implemented. I added its description to the documentation.

mentioned in merge request slapos!416

@vpelletier thank you for your quick reply and nice evaluation of my work.

Recursive schema generation is implemented. I hope that it allows to define entities like tcpv4 in separate library json schema file.

This command zip -r slapos.zip ./ --include \*.json can be used to get zip file containing all json schemes from slapos. After uploading resulting zip archive into schema_editor_application all mutual urls between schemes are valid.

The source of the issue you described is a result of 2 misprints in the json schema:

• the value of mariadb/additionalProperties/$ref property should be "./instance-mariadb-schema.json" instead of ./instance-mariadb-schema.json#/properties" . Here the schema url "./instance-mariadb-schema.json#/properties" is correct too, but developer i think wanted to point to another part of schema.

• inside the schema at url ./instance-mariadb-schema.json the type should be explicitly defined as object. If type or enum or const is not defined then user should have a choice which object type should be added. So the schema is valid if type or enum or const is not defined but developer get an unexpected form.

Thank you that you raised these points.

I would gladly edit these using your UI (and actually did it initially), but could not find a way to get the modified schema file out of it. So I redid with a text editor.

I agree it's a very useful feature, but i'm not sure that it's in my current tasks. @romain , what do you think?

added 1 commit

• 4bdf6c80 - erp5_officejs: schema_editor: schema reference update if file uploaded

Compare with previous version

added 3 commits

• fee2cd74 - erp5_officejs: schema_editor: use Query builder instead string concatination. thanks @jerome
• 4a5d6374 - erp5_officejs: schema_editor: create undefined json_document insteat clean string
• 39a1054a - erp5_officejs: schema_editor: jslint fix

Compare with previous version

Thanks @bk, this README is now describing quite well.

Also, if I understand correctly, I was wrong with:

• default key in the json is used as the value of the field

if we look at today's SlapOS instance parameter editor (with this schema), we have:

default is just a kind of "hint", but it's not set as value, because we want to make a difference between cases where users selected a value (that can be same as default value) and cases where users did not select a value and therefore default value will be use. In this version, there default is a placeholder attribute for text inputs and a small text below for selects.

Also as we can see on that screenshot in that version some <label> are generated from the title in json.

It's not on my screenshot, but a title attribute, which is rendered as a tooltip by desktop browsers, is also added. BTW, I feel title is not the best from usability point of view ( https://stackoverflow.com/questions/12539006/tooltips-for-mobile-browsers ) [ edit for clarification: title attribute is the HTML semantics. I'm not saying that json editor should generate fields with some javascript enhanced tooltip or whatever. It's better to follow the HTML semantics ]

For reference, here's the HTML from the screenshot.

<div class="subfield" title="Allow to manually change the port on wich the apache server running monitoring interface is listening. The default value for the webrunner is different from the default value of the standalone stack-monitor server (default 9684)">
  <label>Monitor Port</label>
  <div class="input">
    <input type="number" name="//monitor-port" class="slapos-parameter" placeholder=" ">
    <span class="error"></span>
   </div>
</div>

Shouldn't we also add a few more lines in the rendering rules section of the README to specify how this is handled ?

---

Another suggestion about the naming, instead of downloadJSON, shouldn't it be named something like resolveExternalReference ?

I hope that it allows to define entities like tcpv4 in separate library json schema file.

This was indeed the original intent. Grepping a bit, I see it is also referenced in other files - without declaring it. Some factorisation seems in order.

The source of the issue you described is a result of 2 misprints in the json schema:

Thanks a lot for checking the schema syntax, and for the patch. I confirm the editor is now happy with it, all referenced schemas are properly loaded. Sorry that the issues were in the schema (I wrote it without ever having a tool capable of using it, for a long time it was not even valid json syntax... I'm happy that it is now a valid jsonschema).

Checking a bit more I notice that when editing ERP5 SR parameters, which lack a $schema property, it is then not possible to change the schema used to validate it (ex: changing https://lab.nexedi.com/.../<some hash>/... to https://lab.nexedi.com/.../master/...). Would it make sense to:

• Display the current schema URL in a field separate from the edited content (ex: below "Reference" field)
• When loading a new document or edition, if top level is an object and it has a $schema property (and schema URL is still the default jsonschema one ?), set schema URL to its value. Otherwise, leave schema URL unchanged.

default is just a kind of "hint", but it's not set as value

@jerome is right, I forgot about this. I have another example in ERP5 SR schema, if it helps:

        "zope-family": {
          "description": "Zope family to connect Jupyter to by default",
          "default": "<first instantiated Zope family>",
          "type": "string"
        }

This is where schemas (jsonschema in particular, but also I think schemas in general) reach their limit: sometimes there is semantically a default value, but its concrete value can only be deduced from another part of the structure being validated.

closed

i am sorry. i did not want to close this mr.

reopened

default is just a kind of "hint", but it's not set as value, because we want to make a difference between cases where users selected a value (that can be same as default value) and cases where users did not select a value and therefore default value will be use. In this version, there default is a placeholder attribute for text inputs and a small text below for selects.

@jerome, i can't agree that schema.default is just a kind of "hint" because:

1. According to draft7/draft4 the schema.default is an object (boolean, array, object, number, string);

2. if i correctly understood @romain when he described my task, he told the json_form_gadget should allow to edit json schema generated on the base of metaschema (draft4/draft7). In case if the schema.default is stringified then the schema edit usability would be broken. This is the example of schema.default is object in slapos: https://lab.nexedi.com/nexedi/slapos/blob/master/software/erp5/instance-erp5-input-schema.json#L114

3. if schema.default is used as hint, then the possibility to have default value will be lost.

4. while discussing a behavior of schema.default @romain told, that it should be exactly as in erp5 form.

5. @jp wanted to see this application at http://json-schema.org/implementations.html#web-ui-generation , then schema.default is hint can be an issue.

For hint can be used: schema.description, schema.placeholder, schema.hint, etc.

This is where schemas (jsonschema in particular, but also I think schemas in general) reach their limit: sometimes there is semantically a default value, but its concrete value can only be deduced from another part of the structure being validated.

@vpelletier, can this solution solve the task you described:

"zope-family": {
          "description": "Zope family to connect Jupyter to by default",
          "default_from_document": "#/zope-partition-dict/0/family",
          "type": "string"
        }

and

• zope-partition-dict should be changed to array.

• support of schema.default_from_document should be implemented in jsonform.gadget.

@jerome, i can't agree that schema.default is just a kind of "hint" because: ...

That makes sense, thanks for clarifying this.

Then, does it mean that in SlapOS schemas we should not describe the "default value that the software release will use if user does not enter anything" as default but as something else such as "description/hint/another key" ? I guess in the context of the SlapOS instance parameter, we want to make a difference between "user did not enter value" and "user entered a value that's same as default" ?

Also, keep in mind that I'm commenting based on my initial impressions from what I see in this MR and I don't know exactly what's Nexedi's plan regarding this, so (as always) what I'm saying does not represent Nexedi's opinion.

@jerome me too can't exactly know Nexedi's plan (and don't know who does) - i just wrote what i know, so your opinion and @vpelletier are very important and thank you very much that you comment. Without you this MR would not have any comment.

Then, does it mean that in SlapOS schemas we should not describe the "default value that the software release will use if user does not enter anything" as default but as something else such as "description/hint/another key" ?

Yes, i think so.

I guess in the context of the SlapOS instance parameter, we want to make a difference between "user did not enter value" and "user entered a value that's same as default" ?

Yes, i think the same.

Checking a bit more I notice that when editing ERP5 SR parameters, which lack a $schema property, it is then not possible to change the schema used to validate it (ex: changing https://lab.nexedi.com/...//... to https://lab.nexedi.com/.../master/...).

Would it make sense to: Display the current schema URL in a field separate from the edited content (ex: below "Reference" field) When loading a new document or edition, if top level is an object and it has a $schema property (and schema URL is still the default jsonschema one ?), set schema URL to its value. Otherwise, leave schema URL unchanged.

Schema_editor_application was developed for offline mode, so all schemes should be available in offline mode.

In schema_editor_application a json document is linked to schema by schema_object.reference. The reference of schema Inside a zip archive is the path of file. If a zip archive is uploaded then the schemes inside this zip archive overwrite the schemes inside jio storage if the schema in zip archive has the same reference as the schema stored in jio.

In example you made a developer should:

1. fetching upload by filling the field Fetch and Upload files and Zip archive containing files in Upload menu - https://lab.nexedi.com/bk/slapos/repository/archive.zip?ref=<some hash>

2. create/edit the json document.

3. fetching upload https://lab.nexedi.com/nexedi/slapos/repository/archive.zip?ref=master

4. open json document for validation.

I think that the feature to push schema changes back to lab.nexedi.com as git commit (with amend possibility) is needed. What do you think?

can this solution solve the task you described:

Note that I do not consider this a bug (so I did not intend my comment as a task to add a feature): I consider it a fundamental limitation of a static structure describing another static structure which is to be used by code. Code allows expressing needs which just cannot be described by a static structure.

IOW: The perfect schema is a turing-complete syntax. Which is not a schema, but plain code.

• zope-partition-dict should be changed to array.

This is not possible. It is an object, and must continue to be. It is already in use as an object in all deployed ERP5 instances within the last few years, and this compatibility must not be broken.

• support of schema.default_from_document should be implemented in jsonform.gadget.

I do not think it is a good idea to start adding features to jsonschema "just" for a UI.

So, overal, no, I do not think this is an acceptable solution.

i can't agree that schema.default is just a kind of "hint" because:

(I reorder points for clearer response progression)

2 . [...] In case if the schema.default is stringified then the schema edit usability would be broken.

I do not think it was suggested that all defaults be strings. Just that the value of "default" property in a schema to be displayed "separately" from the input (maybe as shadow text, maybe in a separate zone...) instead of being pre-input in the fields.

3 . if schema.default is used as hint, then the possibility to have default value will be lost.

But what use is the default exactly ? This is the debate on whether the default values at edition time should be stored on the result, or just kept unset on the result, isn't it ?

In the former, any future schema change of the default value will be ignored. This may be good (stability in provided service) or bad (more work needed to get new features which are considered unintrusive enough to be made into default values to begin with). [EDIT]: And, in the context of SlapOS, if we really want stability in provided service, we should arguably not be using a newer version of a software release.

I favour the latter: if we choose the former, then there cannot be a way for the user to cause the latter. And if whe choose the latter then a user desiring the former can always just input the default values they currently see.

So the most flexible approach is to not set default values in produced output. Which means default values do not have to be values themselves valid in their context. Which is exactly what the RFC says (see below)

Then, if the default value is the actual field value, then the code cannot decide whether it was input by user or just left as-is, so the code becomes even more complicated.

1 . According to draft7/draft4 the schema.default is an object (boolean, array, object, number, string);

[...]

5 . @jp wanted to see this application at http://json-schema.org/implementations.html#web-ui-generation , then schema.default is hint can be an issue.

It can be an issue if it deviates from the spec. The RFC recommends the default value to be valid, and adds that There are no restrictions placed on the value of this keyword.. So it is allowed to be anything the schema author wants, but when possible it should be valid in its context.

4 . while discussing a behavior of schema.default @romain told, that it should be exactly as in erp5 form.

I disagree with such decision, it is diretly contradicting point 5.

ERP5 handles default values very poorly in the UI, and @jp told me directly that it is because no decision was ever made on how to handle them properly. Pushing this to schema edition/validation will directly hurt reuse by others.

It can be an issue if it deviates from the spec. The RFC recommends the default value to be valid, and adds that There are no restrictions placed on the value of this keyword.. So it is allowed to be anything the schema author wants, but when possible it should be valid in its context.

yes, i agree. Of cause we can use schema.default as placeholder, as shadow text, etc. because there is no restrictions in RFC. We have only to remember that RFC was developed for validator, not UI generator.

My opinion that if schema.default is not used as a preset then we will not able to use schema.default as preset in future. I checked how other form generators interpret the schema.default.

I found out that all applications which easily can be tested use default as preset.

• Alpaca Forms default is a preset
• Angular Schema Form default is a preset
• Angular2 Schema Form not checked
• Angular6-json-schema-form default is a preset
• JSON Editor default is a preset
• JSON Form (joshfire) default is a preset
• Json Forms (brutusin) default is a preset
• JSONForms (jsonforms.io) not checked
• Jsonary not checked
• Liform-react not checked
• Metawidget site did not open
• pure-form webcomponent default is a preset, placeholder is from description
• React JSON Schema Form (mozilla) default is a preset
• React Schema Form (networknt) default is a preset

As developers expect that default is a preset, then the items 4 and 5 don't contradict one another.

4 . while discussing a behavior of schema.default @romain told, that it should be exactly as in erp5 form.

5 . @jp wanted to see this application at http://json-schema.org/implementations.html#web-ui-generation , then schema.default is hint can be an issue.

We have only to remember that RFC was developed for validator, not UI generator.

I'm not so sure. I belive the default value is useless for a validator:

• is value present ?
  • yes, is it the right type ? (recursively)
  • no, is it allowed to be absent ?
• are all required properties present ? (or are no forbidden properties present, depending on whether the above it done from schema or from input...)

I think all these are possible to express without ever refering to a default value. And I do not expect the validator to alter its input, only to give a pass-or-fail score.

So I think the presence of a "default" item in the RFC is only useful for UI generators, so they must have considered this.

If they cared about schema authors (which could be another reason to have a "default"), they would have allowed some form of inline comments, explicitly to be ignored by any code parsing the schemas, as json does not allow inline comments.

My opinion that if schema.default is not used as a preset then we will not able to use schema.default as preset in future.

To me this is the sign that doing so would reduce the expressiveness of the UI: we could express something before the change, and cannot anymore after the change.

Also, nothing prevents an integrator (ex: vifib.net when using the UI generator to offer a way to edit instance parameters) from pre-processing input with the schema, applying defaults as presets, and passing thje result to the UI generator. Being preset or not is then independent from the UI.

As developers expect that default is a preset

Thanks for the thorough testing.

These UI generator developpers visibly understood default as a preset. I would argue this unnecesarily limits their usefulness.

Maybe this would set us appart and be an advantage ?

Downloading the JSON can be done like @vincentB did for the onlyoffice apps.

About the default value in the UI, my point of view is that it must be used directly as the input preset. If a schema requires 20 field values, but provide a default for each of them, the user will still be able to save its JSON without having to manually enter all the 20 values. Then, as a user, if a non required field as a default value, I expect it to stable after I save my JSON. If for some reason the schema is modified later with a different value, my JSON should still be considered to use the previous one, which I accepted.

Side topic: placeholder HTML5 attribute usage is not recommended as it makes the UI more difficult to use.

I belive the default value is useless for a validator

Ajv uses default:

With option useDefaults Ajv will assign values from default keyword in the schemas of properties and items (when it is the array of schemas) to the missing properties and items.

I think this feature can be useful for slapos .

About inline comments:

If they cared about schema authors (which could be another reason to have a "default"), they would have allowed some form of inline comments, explicitly to be ignored by any code parsing the schemas, as json does not allow inline comments.

In draft7 $comment is added, it allows inline comments.

@romain

If a schema requires 20 field values, but provide a default for each of them, the user will still be able to save its JSON without having to manually enter all the 20 values.

TL;DR: A schema requiring 20 values but still providing defaults for each is nonsense. It means it accepts 20 values. So this example is too artificial to be relevant, and drawing conclusions from it is dangerous.

In more details:

Please do not discard my advice lightly: I would like to remind you that I designed at least 80% of the ERP5 SR schema, which is used by 100% of Nexedi. I have spent non-negligible time thinking about how an SR should be parametrised, and how to express this parametrisation in the schema. I also believe the result is very usable (partly because there are extremely few required parameters), so I think I have tangible results to back my words.

[EDIT]: Also, I am the one who initiated the use of schemas in slapos (here is the first commit introducing jsonc schema in slapos repository), especially jsonschema. I remember discussing it with @rafael , which is when we came up with Slapos Instance Descriptors (schema, documentation) (which was introduced about one year later). Note that the documentation abstract explicitly says schemas are a documentation format for UI generation:

The structure of these values is constrained by how the Software Release was implemented, and must be documented so it can be used. Instance descriptors are intended to provide such documentation in a form allowing automated generation of a user interface to consult and provide parameters, and to consult published results.

(emphasis added)

It does not say everything it is not about (because it fundamentaly cannot), but I can tell you it is not about modifying user input.

So I think I know what I'm talking about at the level of the original intent, and not just about the implementation.

If a schema requires 20 values, then it requires the user to make 20 conscious decisions. This is what the schema expresses. Taking these decisions instead of the user is in direct contradiction with what the schema is expressing. While I agree that it would make the UI hard to use, the issue lies in why these 20 conscious decisions are needed, not in how they are presented to the user. So taking these decisions is definitely not the job of a UI.

If a schema accepts 20 value and handles a default fallback internally, then these are not required, the user it not required to take decision, and displaying a hint is sufficient (again, in whatever way, I don't care if it's a placeholder, a tooltip, a side label or an holographic schmobulator).

Also, as I proposed above, if an integrator really, really wants to go out of its way to dumb things down so users "just have to press a button", then it is free to implement its own input preprocessor. This is completely unrelated to UI generation.

I'm going further from the topic, but slapos master does not help getting things right by cummulating independent roles:

• "master" proper (transmit and remember parameters and published values, allocate partitions, invoice resource usage). This is a zero-UI job, only about implementing slapos protocol and whatever is needed to get invoices to customers and paid. Pure ERP5 job. Its job is to not get in the way. An example of where it fails to stay out of the way it the forced XML serialisation.
• Software Release instance provider. This is about which software releases can be "sold" to users, maybe providing a set of machines to instanciate them on, preventing users from using some parts of software releases. Here can live a fancy UI and user hand-holding. Here we could forbid users from using any other UI if desired (ex: slapconsole) to maybe force all parameter validation. Each feature here gets in the way of other features (ex: if parameter schema validation is forced here, then we cannot get end-to-end ecryption), but that's a policy decision and users may decide to use a competitor (which would still be offering slapos-based service, just with different policy).

So if we provide a jsonschema UI generator by only putting ourselves in the state of mind of "using it on vifib.net", we are completely missing the goal of "getting it listed on a general list of jsonschema UI generators".

So which one do we really want ? I seriously hope it is the latter, because what I have seen in Boris' work is very worthy of being used beyond just vifib.net .

[EDIT]: forgot to split the second half, which is about something else

Then, as a user, if a non required field as a default value, I expect it to stable after I save my JSON. If for some reason the schema is modified later with a different value, my JSON should still be considered to use the previous one, which I accepted.

This means you also accept having to review the change when upgrading.

To give a tangible example: ERP5 getting ipython notebook (whatever its name). Putting it in the software release means it is considered an essential component of the ERP5 software release, so that business templates may expect its presence. If its configuration is controled by parameters, what value needs to be set if defaults cannot be applied (because parameters were cast in stone in a previous version) ?

This basically means there is no way to add any feature which needs configuration without breaking backward compatibility.

[EDIT]: Side note: in ERP5, the rule is "store user input verbatim". This is the kind of principle I would happily reproduce in slapos, but applying all defaults on first edit directly contradicts this principle.

@bk

I think this feature can be useful for slapos .

No, value fallback in slapos belongs to the Software Release, not to the schema. (and I think this should be the general rule)

The state of mind of handling any fallback at schema level is dangerous, because the next step is to think the schema will protect against all malformed inputs. It will never be able to validate complex arguments (ex: an apache software release accepting bits of apache configuration as parameters, where user must not be able to make apache listen to an arbitrary IP/port but only on the one provided by the software release - I do not see a way to validate this in a jsonschema), or ensure any amount of referential integrity (ex: one part of arguments references another part, like how SLAs constrain subpartition allocation in the ERP5 SR parameters).

So I would argue that a schema validator which alters its input is a misfeature, which will result in a broken/vulnerable integration (schema + whatever uses the value the schema validates) by putting developers in a wrong state of mind: they will try to put as much validation in the schema and then neglect it in the code. Eventually, something will not be protected correctly, and a security hole will be present which will be harder to spot because one has to consider both the schema and the code at the same time, instead of just the code.

Don't get me wrong: I am annoyed by the duplication caused by having "stuff in the code" and "same-ish stuff in the schema", because duplicated stuff eventualy gets inconsistent. But fundamentally, a schema summarises what code produces and/or expects, so a schema is already 100% duplicating the code. I think a better way to think of a schema is documentation: documentation is also duplicating code and gets inconsistent, and it is generally considered acceptable.

@romain told me to remove using of schema.default. i have done. bk/rjs_json_form@186c60f4

resolved all discussions

To me, this is really good, is there anything preventing from merging at this point ?

mentioned in merge request bk/rjs_json_form!3

I was checking this, and I found something inconsistent:

The editor allow me to save an invalid value, so I think the schema is never used to validate the input.

A after another quick read.

The way it is implemented it don't looks like compliant with officejs (which I'm not specialist so maybe @vincentB and @romain can say), so soon or later, someone will complain about it too.

For example, portal_skins/erp5_json_form/jsonform seems missplaced, it should be moved into web_page_module.

Another example, the draft json schemas should be placed on document_module or I would suggest a kind of "JSON Portal Type" on web_page_module (as we have Web Style, Web Script, I would say Web JSON ?)

I think this is a bit copy and paste, not sure it is needed.

i fix it in 90fe5df1

Where is this portal type, it seems not commited on this MR. Does it exist already?

answer

I think this come from copy and paste

i fix it in 90fe5df1

Seems copy and paste?

i fix it in 90fe5df1

Where is JSON Schema portal type on ERP5 and schema_module?

answer

I'm not sure @romain would agree with this change of API.

This is not a change of API, just renaming this local function as the name clashed with the declared method. It was discussed a bit in !761 (comment 67138)

Maybe the local function should be named _updateHeader ? l prefix means local / internal if I understood well )

I agree with @jerome , that it's not a change of API. @romain , should i rename the local function? to _updateHeader ?

Hello @rafael, thank you for your comments.

I was checking this, and I found something inconsistent: (screenshot)

... I think the schema is never used to validate the input.

As i see you:

1. had added new empty schema and fill the $schema property field with https://lab.nexedi.com/nexedi/slapos/raw/master/software/kvm/instance-kvm-input-schema.json .
2. saved the schema and reloaded the web page. As a result you got Schema form not Document form. The meta schema of this new Schema is https://lab.nexedi.com/nexedi/slapos/raw/master/software/kvm/instance-kvm-input-schema.json .
3. filled the field ram‑size with 22 and the field ram‑hotplug‑slot‑size 6144.

I didn't think this result (Schema) was the result that you wanted to get, but i supposed i repeated your way. But i've got the error under the ram-size field Value 22 is not a multiple of 512 .

Could you please tell where i was wrong in my understanding what you did?

If you wanted to edit document based on https://lab.nexedi.com/nexedi/slapos/raw/master/software/kvm/instance-kvm-input-schema.json schema, then you have to

1. upload the schema (click Upoad in menu and fill field Fetch and Upload files and Zip archive containing files with schema url and save
2. new schema appears in menu - only if its Title is not empty
3. click on "Schemas" in menu and click on the schema in list to edit it
4. to add the document based on the schema - click the schema name in menu and click "Add"

Here is described how to work with schema which has references to other schemes.

The editor allow me to save an invalid value

Yes, editor allows to save an invalid value, because json document can be not finished and user wants to finish it latter.

For example, portal_skins/erp5_json_form/jsonform seems missplaced, it should be moved into web_page_module.

Portal_skins/erp5_json_form is not officejs related gadget. It is used in officejs' schema editor application, but it's not officejs component. Erp5_json_form can be used in erp5_graph_editor or slapos which are not officejs applications. But if @romain tells me to move - i will move.

Another example, the draft json schemas should be placed on document_module or I would suggest a kind of "JSON Portal Type" on web_page_module (as we have Web Style, Web Script, I would say Web JSON ?)

To make this the following issue should be solved before:

In schema editor application the relation is not between the documents, but between the json document and it's schema. Now i use schema Id, because a reference (in which the schema relative path is saved) can be changed by developer. In officejs applications the id of a document is generated as Uid. After synchronization it is changed for Id generated by Erp5 instance. So the the relation link will be broken. That's why there is no synchronization between erp5 and local jio storage used in schema editor application, so currently there is no sense to create Portal Type on erp5 side.

May be you have a solution or an idea on synchronization issue?

@vpelletier wrote:

would gladly edit these using your UI (and actually did it initially), but could not find a way to get the modified schema file out of it.

I added this possibility on base of @jerome application: schema, document and form for json document can be edit in one browser tab. The url of downloaded schema situated on the top left can be changed. While the json schema (in Monaco editor) is edited the form (in json form generator) is automatically changed. While the fields in form are filled the json document (in Monaco editor) based on the schema is automatically changed. If the json document is changed manually in Monaco editor, then fields in the form automatically filled. This demo can be tested on https://d1.erp5.ru/nexedi/rjs_json_form/demo/with_monaco/ . The sources of it - https://lab.nexedi.com/bk/rjs_json_form/tree/master/demo/with_monaco .

While the json schema (in Monaco editor) is edited the form (in json form generator) is automatically changed.

Looks very nice !

I tried to reproduce my original scenario, and my browser tab died. I could reproduce the issue, and here are the steps:

1. in schema URL, input https://json-schema.org/draft-04/schema (note the "https" scheme so chrome accepts loading)
2. check that the schema is actually loaded and the UI is generated. A keypress event may be needed to trigger the load, which may not happen on its own if you paste using a middle-click.
3. open the before-fix ERP5 SR schema, copy its content, and paste it in the bottom-left pane in Monaco editor
4. wait, notice large CPU usage and memory usage rise. You may get chrome's prompt to wait or kill the script, choose "wait". Then the tab dies when its memory usage gets close to 2GB (x64 arch with several GBs of free memory, and binary does not seem to be x32, so it should not be an actual segfault/out-of-virtual-memory/OOM but likely some voluntary limit chrome imposes on itself)

ERP5 SR schema is large, so I consider this a stress-test rather than realistic usage. I'm not reporting it to request a fix (the issue in that schema was fixed weeks ago anyway), but rather to give you a heads-up that maybe there is something that can be fixed. Could be Monaco (so nothing to fix ?), could be RenderJS (then probably @romain would want to know), could be your code, I have no idea. And it can be low priority.

Hello @bk

Portal_skins/erp5_json_form is not officejs related gadget. It is used in officejs' schema editor application, but it's not officejs component. Erp5_json_form can be used in erp5_graph_editor or slapos which are not officejs applications. But if @romain tells me to move - i will move.

Another example, the draft json schemas should be placed on document_module or I would suggest a kind of "JSON Portal Type" on web_page_module (as we have Web Style, Web Script, I would say Web JSON ?)

Almost all officejs use code locate in portal_skins, cause we use gadget_editor.html defined in erp5_core, In that way, adding an officejs application in this way is "closed" to adding a feature in erp5.

Officejs require only to make a zip from your application, which mean only a cache manifest with all files regardless of location on server, please have a look to gadget_officejs_page_export.html and officejs_export Web Site. Feel Free to also add a progressive web app manifest, this will be needed for next officejs appstore release ( you should add a logo for store and mobile app ). See this commit : vincentB/erp5@b330d1a9 Except these stuff, from officejs view it looks great !

In your case you could add in future a well defined portal type JSON and be able to edit it from erp5 renderjs_runner easily.

Thanks for the explanations @bk, I still didn't got how to use (but it is probably I have lack of time to play with it).

What I write below is unrelated to the APP, but the reusability of the components of the app for other applications.

Considering using it for slapos master parameter editor, it would be required to make things more modular, because we have significant diferences (at first look).

There is a layer on top of rendering (to read software.cfg.json) and after that we work exclusively over the querying the URL (so no zip at all). I wonder if you could split the pure rendering gadget from the load of schemas, and construct the real form.

Maybe, I would suggest I would move rendering of field and form for example to another gadget, this was my indention on slapos master, but I really didn't do (as it was a PoC done in 2 weeks time, not an final app like this one).

I also feels that render a field is a duplication with the ones on renderjs (to render erp5 form), so I was wondering, if we could re-use the engine or the approach already present on the erp5js. (Not sure if @romain would like this or not).

On slapos master, we are based on ERP5JS and not on OfficeJS, so for example, we don't rely on local jio storage, so all JSON Document/Schema is something incompatible with SlapOS. If this part can be splited, it would help me to review a possible integration.

Thanks for the explanations @bk, I still didn't got how to use (but it is probably I have lack of time to play with it).

There's a doc at https://lab.nexedi.com/bk/rjs_json_form/blob/master/README.md .

There is a layer on top of rendering (to read software.cfg.json) and after that we work exclusively over the querying the URL (so no zip at all). I wonder if you could split the pure rendering gadget from the load of schemas, and construct the real form.

This seems to be what schema_url parameter of render is for.

The goal of this merge request is not to provide any reusable JSON editor component for now. Which means, the current code can not be used on SlapOS or the graph editor as is, and @boris should not try to reach such milestone for now. So people, please do not try to reuse erp5_json_form for now, even if merged, because it will be incompatible in the coming weeks.

As the application is useful to @vpelletier and @jerome , let's try to merge it with it's current functionalities. Please:

• write an officejs zelenium test showing that open the app, create a document with a specific schema and save some modification (no need to test all field interactions).
• move the CSS into the officejs app
• revert all changes done in erp5_web_renderjs_ui. First because I have many ongoing changes which will conflict next week (if I succeed finishing them...). Then because I believe they are not needed.
• update the OfficeJS zip exporter to support your app (no need to update the OfficeJS frontpage for now).

If done quickly, I'll merge the application and it will be published into OfficeJS next week.

Tests can be seen here.

I tried to reproduce my original scenario, and my browser tab died.

Thank you @vpelletier, for reporting i fix it

Thank you @vpelletier, for reporting i fix it

I confirm, now the form loads after a few seconds. Nice !

mentioned in merge request slapos!549

@jerome wondered why this is not merged yet. Maybe the tests failing is a reason why.

@bk do you know if the tests ever worked ?

It would be great to have this merged...