Skip to content

Fix UI typecasting#1667

Merged
mvorisek merged 19 commits intodevelopfrom
fix_no_boolean_field
Oct 4, 2021
Merged

Fix UI typecasting#1667
mvorisek merged 19 commits intodevelopfrom
fix_no_boolean_field

Conversation

@mvorisek
Copy link
Copy Markdown
Member

@mvorisek mvorisek commented Oct 2, 2021 •
edited
Loading

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

@mvorisek mvorisek force-pushed the fix_no_boolean_field branch from 251d28f to cf05f1e Compare October 2, 2021 03:26
@mvorisek mvorisek marked this pull request as ready for review October 2, 2021 03:26
mvorisek
mvorisek commented Oct 2, 2021
View reviewed changes
src/Form/Control.php
public function set($value = null, $junk = null)
{
if ($this->field) {
$value = $this->getApp()->ui_persistence->typecastLoadField($this->field, $value);
Copy link
Copy Markdown
Member Author

@mvorisek mvorisek Oct 2, 2021

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 requested a review from DarkSide666 October 2, 2021 11:11
@mvorisek mvorisek force-pushed the fix_no_boolean_field branch 15 times, most recently from c54b4c3 to 991838f Compare October 2, 2021 15:21
mvorisek
mvorisek commented Oct 2, 2021
View reviewed changes
tests-behat/scopebuilder.feature
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
Copy Markdown
Member Author

@mvorisek mvorisek Oct 2, 2021

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 added the RTM label Oct 2, 2021
@mvorisek mvorisek force-pushed the fix_no_boolean_field branch from d074d37 to 8036c5e Compare October 2, 2021 16:02
@mvorisek mvorisek requested a review from ibelar October 2, 2021 16:08
@mvorisek mvorisek force-pushed the fix_no_boolean_field branch from 8889d04 to b7a0c0c Compare October 2, 2021 16:15
@mvorisek mvorisek force-pushed the fix_no_boolean_field branch from 894cd19 to 4745ed0 Compare October 3, 2021 16:31
@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 force-pushed the fix_no_boolean_field branch from 1cfc31d to 629e44d Compare October 4, 2021 00:03
@mvorisek mvorisek force-pushed the fix_no_boolean_field branch from 629e44d to 066a8fd Compare October 4, 2021 00:10
@mvorisek mvorisek force-pushed the fix_no_boolean_field branch from 8a0ef30 to 066a8fd Compare October 4, 2021 10:56
@mvorisek mvorisek force-pushed the fix_no_boolean_field branch from 58bf729 to 21ce1aa Compare October 4, 2021 11:16
@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
ibelar
ibelar approved these changes Oct 4, 2021
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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
@mvorisek mvorisek mentioned this pull request Oct 21, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ibelar ibelar ibelar approved these changes

@DarkSide666 DarkSide666 Awaiting requested review from DarkSide666

@georgehristov georgehristov Awaiting requested review from georgehristov

Assignees

No one assigned

Labels

bug 🪲 RTM

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mvorisek @ibelar