Skip to content

FIX: Process templates before previewing #33848

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Lhcfl
Copy link
Contributor

@Lhcfl Lhcfl commented Jul 25, 2025

The bare template yaml file provided by the user cannot be directly used for form templates preview, because components such as TagChooserField actually rely on a backend process step. This step exists when users actually use the template, but is missing when the admin writes and previews the template.

This commit supplements the missing process step, thus fixing the problem that tag-chooser may break ember during admin preview

Lhcfl added 2 commits July 25, 2025 10:01
The bare template yaml file provided by the user cannot be directly used
for form templates preview, because components such as `TagChooserField`
actually rely on a backend process step. This step exists when users
actually use the template, but is missing when the admin writes and
previews the template.

This commit supplements the missing process step, thus fixing the
problem that tag-chooser may break ember during admin preview
@discoursebot
Copy link

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/form-templates-preview-broken-with-tag-group/372744/3

Comment on lines +95 to +96
fab!(:tag4, :tag)
fab!(:tag5, :tag)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these records fabricated to ensure that the tag-chooser selector isn't returning all the tags? Otherwise, we don't need to fabricate these records.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, so it is using to eq instead of to include

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about it and decided to use them - because I happened to notice a bug in the initial commit, where the processed value would not be written when the attribute was empty.

@@ -139,7 +140,8 @@ export default class FormTemplateForm extends Component {
}

FormTemplate.validateTemplate(data)
.then(() => {
.then(({ form_template }) => {
this.processedTemplateContent = form_template.template;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there should also be a test for this change. Maybe a system test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. Added!

@discoursebot
Copy link

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/daily-summary-9pm-utc/291850/579

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

Successfully merging this pull request may close these issues.

3 participants