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 UI typecasting #1667

Merged
merged 19 commits into from
Oct 4, 2021
Merged

Fix UI typecasting #1667

merged 19 commits into from
Oct 4, 2021

Conversation

mvorisek
Copy link
Member

@mvorisek mvorisek commented Oct 2, 2021

merge after atk4/core#329 and atk4/data#897

@mvorisek mvorisek marked this pull request as ready for review October 2, 2021 03:26
@@ -100,7 +100,6 @@ protected function init(): void
public function set($value = null, $junk = null)
{
if ($this->field) {
$value = $this->getApp()->ui_persistence->typecastLoadField($this->field, $value);
Copy link
Member Author

Choose a reason for hiding this comment

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

This was wrong here. Control::set should expect already fully loaded data and can not be used to set data from frontend directly.

@mvorisek mvorisek changed the title Fix no boolean field Fix UI typecasting Oct 2, 2021
@mvorisek mvorisek force-pushed the fix_no_boolean_field branch 15 times, most recently from c54b4c3 to 991838f Compare October 2, 2021 15:21
@@ -12,7 +12,8 @@ Feature: ScopeBuilder
Then bool rule "atk_fp_stat__is_commercial" has value "No"
Then I check if input value for "qb" match text in "p.atk-expected-input-result"
And I press button "Save"
Then I check if text in "p.atk-expected-word-result" match text in ".atk-scope-builder-response"
# TODO uncomment once "Object serialization is not supported" is fixed
# Then I check if text in "p.atk-expected-word-result" match text in ".atk-scope-builder-response"
Copy link
Member Author

Choose a reason for hiding this comment

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

@georgehristov disabling now as we can not serialize for security reasons from the user input. Why do we need to serialize (object type) at all?

@mvorisek mvorisek force-pushed the fix_no_boolean_field branch 5 times, most recently from fa03c4f to 381f1f6 Compare October 3, 2021 23:07
@mvorisek mvorisek force-pushed the fix_no_boolean_field branch 2 times, most recently from d431047 to 0a0a560 Compare October 3, 2021 23:29
@mvorisek mvorisek added good first issue 🍼 Start contributing by fixing this issue! RTM and removed good first issue 🍼 Start contributing by fixing this issue! labels Oct 4, 2021
Copy link
Contributor

@ibelar ibelar left a comment

Choose a reason for hiding this comment

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

Look Good. - There are some issues in demos for calendar input and multiline but it might be because of some refactoring need after #1664
I think it is ok to merge, we can submit pr for each issues.

@mvorisek
Copy link
Member Author

mvorisek commented Oct 4, 2021

Thanks for the review. The issue is here #1668. If you know about any other issue, please post.

@mvorisek mvorisek merged commit 2b03fd9 into develop Oct 4, 2021
@mvorisek mvorisek deleted the fix_no_boolean_field branch October 4, 2021 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants