Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix form builder settings after switching to new locale #3546

Open
wants to merge 9 commits into
base: next
Choose a base branch
from

Conversation

neatbyte-vnobis
Copy link
Collaborator

Changes

Resolve issue: Unable to submit a form in newly created locale.

Form submission was throwing "Form Builder" settings not found! error, since there were no Form Builder settings created for the new locale. This also caused the same issue when saving Form Builder settings form.
Issue was resolved by triggering Form Builder settings creation, when a new locale is created.

While fixing this issue I've found that the same is happening for the Website settings form, since Website settings are also not created for new locale. If this fix is ok, then something similar can be done inside api-page-builder to create settings after a new locale is created.

Also while testing this fix I've noticed that after switching to a new locale user is unable to create any pages until a page category with static slug is created. Is this expected behavior?

@adrians5j @Pavel910 What do you think?

How Has This Been Tested?

Manual

Documentation

None

Copy link
Member

@adrians5j adrians5j left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor stuff.

I agree with PB-related things you mentioned. We can do them all within this PR or via separate PRs, your choice. 🙂

import { ContextPlugin } from "@webiny/api";
import { FormBuilderContext } from "~/types";

export const createSettingsForNewLocale = new ContextPlugin<FormBuilderContext>(context => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once a locale has been deleted, let's also delete settings.

context.i18n.locales.onLocaleAfterCreate.subscribe(async ({ locale }) => {
const existingSettings = await context.formBuilder.getSettings({
auth: false,
locale: locale.code
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need locale here? Wouldn't the current locale automatically be pulled above?

If there's no really a need to expose locale param, let's maybe not do it.

@neatbyte-vnobis
Copy link
Collaborator Author

@adrians5j While adding similar changes to PB I've noticed that there is no delete option in PB settings crud (FB settings crud has it). Should we add one? Then we will be able to delete settings after the locale is deleted (just like we do in FB now).

@adrians5j
Copy link
Member

Yes, let's do it @neatbyte-vnobis . Thanks!

@neatbyte-vnobis
Copy link
Collaborator Author

@adrians5j Currently, the FB/HCMS PR is focused on migrating only forms and submissions. Changes in this PR are related to FB settings (not sure what is current plan for FB settings migration to HCMS).

The FB-related changes can be included in the FB/HCMS PR, and, as you've suggested, the PB part can be moved to a separate PR.

@neatbyte-vnobis
Copy link
Collaborator Author

@adrians5j Any updates on this one? Should we move PB changes to a separate PR or leave it as it is?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants