Form Builder - Move backend to HCMS

[neatbyte-vnobis]

Changes

Form Methods moved to HCMS:

• [x] createForm
• [x] getForm
• [x] listFormRevisions
• [x] listForms
• [x] createFormFrom
• [x] deleteForm
• [x] deleteFormRevision
• [x] publishForm
• [x] unpublishForm
• [x] updateForm

Submission Methods moved to HCMS:

• [x] listSubmissions
• [x] createSubmission
• [x] updateSubmission

Skipped Submission Methods: deleteSubmission, getSubmission. They were skipped because they are not used anywhere.

How Has This Been Tested?

Manually.

Documentation

Form Builder - Move backend to HCMS

[neatbyte-ivelychko]

@adrians5j, @SvenAlHamad what should we do about these bugs?

While working on this feature we have found bug that exists for a long time.

1. When we have a form that has ONLY one revision and after deleting that revision we would see that the form that should have been deleted still being displayed in UI in the list of the forms. But after reloading the page it would not be rendered in the list of forms. I think that happens becuase of caching. Furthemore this bug doesn not appear if we delete a form instead of revision.

https://github.com/webiny/webiny-js/assets/129154672/1f0a2d5e-e62f-4e6d-98b1-df3d60713a28

2. When have created a form submission, opened Submissions tab in Form Builder and clicked on reload submissions we would see that the list of submissions has been updated. But Overview stats on the Submissions tab are still the same, so they has not been updated. To see a most recent data for Overview we would need to reload the page.

3. When we choose a form revision that we had chosen previously then that revision won't be re-rendered in the Page Builder Editor Form and it would render the latest chosen revision.

https://github.com/webiny/webiny-js/assets/129154672/061493ce-7b39-4f45-9828-c49c7b6f4f40

[neatbyte-ivelychko]

@leopuleo I have fixed issue with settings zod model

[neatbyte-vnobis]

Also, I noticed some of the GraphQL schemas generated have been changed.

@leopuleo I've investigated this a little bit and found that a similar issue exists in FileManager too. Fields marked as required in the file model are not required in the schema. The problem lies in the renderFields function. This function utilizes createTypeField, which is not influenced by the required value of a field.

[leopuleo]

Also, I noticed some of the GraphQL schemas generated have been changed.

@leopuleo I've investigated this a little bit and found that a similar issue exists in FileManager too. Fields marked as required in the file model are not required in the schema. The problem lies in the renderFields function. This function utilizes createTypeField, which is not influenced by the required value of a field.

@neatbyte-vnobis thanks for investigating. Actually, marking a field as required does not change the generated GraphQL schema: this is happening also for UI-managed content models.

Similar behaviour happens for predefined values. So, skip my comments mentioning Should be required and Should be an enum.

Could you please have a look at the other comments?

[neatbyte-vnobis]

Also, I noticed some of the GraphQL schemas generated have been changed.

@leopuleo I've investigated this a little bit and found that a similar issue exists in FileManager too. Fields marked as required in the file model are not required in the schema. The problem lies in the renderFields function. This function utilizes createTypeField, which is not influenced by the required value of a field.

@neatbyte-vnobis thanks for investigating. Actually, marking a field as required does not change the generated GraphQL schema: this is happening also for UI-managed content models.

Similar behaviour happens for predefined values. So, skip my comments mentioning Should be required and Should be an enum.

Could you please have a look at the other comments?

@leopuleo regarding the layout property in the form steps, which was originally of type [[String]], the current implementation of createTypeField does not support two-dimensional arrays.

The possible solutions I see are: adding a separate type for this value, modifying createTypeField to support two-dimensional arrays, or use the [JSON] type as it is currently implemented (which can be simplified to just JSON).

[leopuleo]

Also, I noticed some of the GraphQL schemas generated have been changed.

@leopuleo I've investigated this a little bit and found that a similar issue exists in FileManager too. Fields marked as required in the file model are not required in the schema. The problem lies in the renderFields function. This function utilizes createTypeField, which is not influenced by the required value of a field.

@neatbyte-vnobis thanks for investigating. Actually, marking a field as required does not change the generated GraphQL schema: this is happening also for UI-managed content models. Similar behaviour happens for predefined values. So, skip my comments mentioning Should be required and Should be an enum. Could you please have a look at the other comments?

@leopuleo regarding the layout property in the form steps, which was originally of type [[String]], the current implementation of createTypeField does not support two-dimensional arrays.

The possible solutions I see are: adding a separate type for this value, modifying createTypeField to support two-dimensional arrays, or use the [JSON] type as it is currently implemented (which can be simplified to just JSON).

@neatbyte-vnobis thanks for the explanation. Let's stick to the current implementation [JSON]

[neatbyte-vnobis]

@leopuleo Is this PR ready to be merged?

[neatbyte-vnobis]

@adrians5j @leopuleo As we're removing the revision selection for forms in the Page Builder, I assume we should now only display forms with a published revision in the form select. Should we create a new listPublishedForms API endpoint for this, or modify the existing listForms with an additional parameter? I think a separate endpoint would be preferable, as it would utilize listPublishedEntries instead of listLatestEntries.

Additionally, considering we now have only one published revision, should we rename getLatestPublishedFormRevision to getPublishedFormRevision?

[neatbyte-vnobis]

@adrians5j @leopuleo As we're removing the revision selection for forms in the Page Builder, I assume we should now only display forms with a published revision in the form select. Should we create a new listPublishedForms API endpoint for this, or modify the existing listForms with an additional parameter? I think a separate endpoint would be preferable, as it would utilize listPublishedEntries instead of listLatestEntries.

Additionally, considering we now have only one published revision, should we rename getLatestPublishedFormRevision to getPublishedFormRevision?

@adrians5j @leopuleo Any thoughts on this?

[neatbyte-vnobis]

@adrians5j While updating tests following the recent status updates, I noticed that this test fails. The test involves publishing a form revision and then incrementing its views. However, with our current use of published HCMS entries, we cannot update them. This is because the current implementation of HCMS includes a check to see if an entry is locked before updating, and it is set to true when an entry is published. So we cannot update already published form - which is a breaking change compared to old API and FB logic.

What should our approach be in this case? Could we consider making some changes to HCMS to address this issue? (option to override the locked status or something like this)

[neatbyte-vnobis]

@adrians5j While updating tests following the recent status updates, I noticed that this test fails. The test involves publishing a form revision and then incrementing its views. However, with our current use of published HCMS entries, we cannot update them. This is because the current implementation of HCMS includes a check to see if an entry is locked before updating, and it is set to true when an entry is published. So we cannot update already published form - which is a breaking change compared to old API and FB logic.

What should our approach be in this case? Could we consider making some changes to HCMS to address this issue? (option to override the locked status or something like this)

@adrians5j @Pavel910 Any thoughts on this?

[adrians5j]

We're still discussing this internally. We'll get back to you re this soon.