-
Notifications
You must be signed in to change notification settings - Fork 617
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
base: next
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 => { |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
@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). |
Yes, let's do it @neatbyte-vnobis . Thanks! |
@adrians5j Currently, the FB/HCMS PR is focused on migrating only 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. |
@adrians5j Any updates on this one? Should we move PB changes to a separate PR or leave it as it is? |
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 noForm Builder
settings created for the new locale. This also caused the same issue when savingForm Builder
settings form.Issue was resolved by triggering
Form Builder
settings creation, when a newlocale
is created.While fixing this issue I've found that the same is happening for the
Website
settings form, sinceWebsite
settings are also not created for newlocale
. If this fix is ok, then something similar can be done insideapi-page-builder
to create settings after a newlocale
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 withstatic
slug is created. Is this expected behavior?@adrians5j @Pavel910 What do you think?
How Has This Been Tested?
Manual
Documentation
None