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

3768 - Fix error saving field-based page #4123

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

WillNigel23
Copy link
Collaborator

No description provided.

@WillNigel23 WillNigel23 linked an issue May 23, 2024 that may be closed by this pull request
@WillNigel23 WillNigel23 force-pushed the 3768-error-saving-field-based-page-fix branch from a829d89 to f280c3a Compare May 23, 2024 20:15
validates :percentage, numericality: { allow_nil: true, greater_than: 0, less_than_or_equal_to: 100 }
validates :page_number, numericality: { allow_nil: true, greater_than: 0, less_than_or_equal_to: 1000 }

validates :label, format: { without: /[\[\]\{\}]/, message: "cannot contain '{', '}', '[' or ']'" }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Validation for checking characters that might be interpreted as hash like '{}' or '[]'

Copy link
Owner

Choose a reason for hiding this comment

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

Let's remove the {} validation -- only the hash element accessor [] is used by Rails to create structured parameters, {} isn't a problem. (I just tested that on development and {} works.

div.character-count
-else
=text_area_tag "fields[#{field.id}][#{field.label}]", content, class: 'field-input'
=generate_field_input(field, cell)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved some of the logic here to the helper function.

Copy link
Owner

Choose a reason for hiding this comment

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

Great!

def generate_field_input(field, cell)
input_name = formatted_field_name(field)

label = label_tag(field.label.parameterize, field.label.titleize)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I titleize the label for the field inputs. Let me know if it should not be like this after all

Copy link
Owner

Choose a reason for hiding this comment

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

Let's not titleize the label -- many of our customers have strong opinions about this.

@WillNigel23
Copy link
Collaborator Author

Moving back to in progress. seems there are a lot of specs failing due to changed name.

@WillNigel23 WillNigel23 force-pushed the 3768-error-saving-field-based-page-fix branch from bb64adf to 9e6ded4 Compare May 23, 2024 22:01
@benwbrum benwbrum self-assigned this May 24, 2024
Copy link
Owner

@benwbrum benwbrum left a comment

Choose a reason for hiding this comment

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

Remove titleize and permit curly-braces.

@benwbrum benwbrum assigned WillNigel23 and unassigned benwbrum May 24, 2024
@WillNigel23 WillNigel23 requested a review from benwbrum June 3, 2024 21:10
@benwbrum benwbrum merged commit ac0b3c3 into development Jun 3, 2024
@benwbrum benwbrum deleted the 3768-error-saving-field-based-page-fix branch June 3, 2024 23:44
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.

500 error saving field-based page
2 participants