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: Incorrect type for empty extraSettings in frontend #1730

Merged
merged 1 commit into from Oct 6, 2023

Conversation

Chartman123
Copy link
Collaborator

This fixes an error introduced with the XML fix in #1705

For questions where extraSettings weren't set we got a type mismatch in the extraSettings prop (Object expected, array provided)

@Chartman123 Chartman123 added javascript Javascript related ticket 3. to review Waiting for reviews regression Regression of a previous working feature labels Sep 29, 2023
@Chartman123 Chartman123 added this to the 3.4 milestone Sep 29, 2023
@Chartman123 Chartman123 self-assigned this Sep 29, 2023
@Chartman123 Chartman123 changed the title (fix): Incorrect type for empty extraSettings in frontend fix: Incorrect type for empty extraSettings in frontend Sep 29, 2023
Signed-off-by: Christian Hartmann <chris-hartmann@gmx.de>
@Chartman123 Chartman123 force-pushed the fix/type-extraSettings-frontend branch from 76244f2 to 4d36422 Compare October 1, 2023 20:20
@codecov
Copy link

codecov bot commented Oct 1, 2023

Codecov Report

Merging #1730 (4d36422) into main (d2ba5b5) will increase coverage by 42.12%.
The diff coverage is n/a.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1730       +/-   ##
===========================================
+ Coverage        0   42.12%   +42.12%     
- Complexity      0      570      +570     
===========================================
  Files           0       55       +55     
  Lines           0     2381     +2381     
===========================================
+ Hits            0     1003     +1003     
- Misses          0     1378     +1378     

@susnux
Copy link
Collaborator

susnux commented Oct 6, 2023

Not sure if this is a proper fix or just silencing this error. As the error has a reason: The given type a an array instead of the expected object.

The reason is if the extra settings are empty, php will return an empty assoc. array which will be converted to [] instead of {} for the json.

Copy link
Collaborator

@susnux susnux left a comment

Choose a reason for hiding this comment

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

But I think there is no other good solution, because either we have to fix it when the form is loaded in the frontend.

Or drop XML support (no idea if used, but we should not drop it now).

@susnux susnux merged commit 3554e47 into main Oct 6, 2023
22 checks passed
@susnux susnux deleted the fix/type-extraSettings-frontend branch October 6, 2023 16:20
@Chartman123
Copy link
Collaborator Author

But I think there is no other good solution, because either we have to fix it when the form is loaded in the frontend.

Or drop XML support (no idea if used, but we should not drop it now).

Yes, with this fix the extraSettings prop is either an array[0] or an object depending of the result from the database... Do you see any chance to fix this in the backend and always return an object in JSON?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews javascript Javascript related ticket regression Regression of a previous working feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants