Skip to content

Conversation

@laurajaime
Copy link
Contributor

@laurajaime laurajaime commented Mar 20, 2025

There is an error when the form builder is used in /system when you edit an organization.

imagen

@laurajaime
Copy link
Contributor Author

@microstudi we found this error, but I don't know if this solution is the best approach... can you take a look and tell me? thanks!!

Comment on lines 13 to 15
def file_field(object_name, options = {})
return super(object_name, options) unless Decidim::ReportingProposals.use_camera_button
return super(object_name, options) if @object_name == "editor_image" || @object_name == "oauth_application"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def file_field(object_name, options = {})
return super(object_name, options) unless Decidim::ReportingProposals.use_camera_button
return super(object_name, options) if @object_name == "editor_image" || @object_name == "oauth_application"
alias_method :original_file_field, :file_field
def file_field(object_name, options = {})
return super(object_name, options) unless Decidim::ReportingProposals.use_camera_button
return original_file_field unlesss @template.respond_to?(:snippets)

Hi @laurajaime, I think the best approach would just do something like this

@laurajaime laurajaime force-pushed the fix/error_with_form_builder_override branch from c1bee01 to cd3e8be Compare April 1, 2025 14:37
@codecov
Copy link

codecov bot commented Apr 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.43%. Comparing base (782784c) to head (cd3e8be).

❗ There is a different number of reports uploaded between BASE (782784c) and HEAD (cd3e8be). Click for more details.

HEAD has 3 uploads less than BASE
Flag BASE (782784c) HEAD (cd3e8be)
4 1
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #104      +/-   ##
==========================================
- Coverage   89.71%   80.43%   -9.29%     
==========================================
  Files          72       66       -6     
  Lines        1507     1436      -71     
==========================================
- Hits         1352     1155     -197     
- Misses        155      281     +126     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@laurajaime laurajaime marked this pull request as ready for review April 1, 2025 14:50
@laurajaime laurajaime requested a review from microstudi April 1, 2025 14:50
Copy link
Member

@microstudi microstudi left a comment

Choose a reason for hiding this comment

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

Thanks @laurajaime

Please check the comment and we are good to go!

Comment on lines 16 to 17
return super(object_name, options) unless Decidim::ReportingProposals.use_camera_button
return original_file_field unless @template.respond_to?(:snippets)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return super(object_name, options) unless Decidim::ReportingProposals.use_camera_button
return original_file_field unless @template.respond_to?(:snippets)
return original_file_field(object_name, options) unless Decidim::ReportingProposals.use_camera_button
return original_file_field(object_name, options) unless @template.respond_to?(:snippets)

Now that I see thid, maybe for consistency we should do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tramuntanal
Copy link

@microstudi ready to review again

Copy link
Member

@microstudi microstudi left a comment

Choose a reason for hiding this comment

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

Thanks!

@microstudi microstudi merged commit c1b0695 into openpoke:main May 6, 2025
6 checks passed
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.

3 participants