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

Add support for more fields #375

Closed
wants to merge 48 commits into from
Closed

Add support for more fields #375

wants to merge 48 commits into from

Conversation

romaninsh
Copy link
Member

@romaninsh romaninsh commented Jan 7, 2019

implement types using DBAL first - #727

Implements separate classes for Number, Percent and Rating. Also includes Money for a basic implementation.

@romaninsh romaninsh changed the base branch from develop to feature/epic-field-refactor January 7, 2019 01:02
@codecov
Copy link

codecov bot commented Jan 7, 2019

Codecov Report

Merging #375 into develop will decrease coverage by 1.66%.
The diff coverage is 65.63%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #375      +/-   ##
=============================================
- Coverage      86.70%   85.04%   -1.67%     
- Complexity      1232     1287      +55     
=============================================
  Files             28       37       +9     
  Lines           2730     2788      +58     
=============================================
+ Hits            2367     2371       +4     
- Misses           363      417      +54     
Impacted Files Coverage Δ Complexity Δ
src/Field/Callback.php 38.46% <0.00%> (-61.54%) 6.00 <4.00> (+4.00) ⬇️
src/Field/Money.php 0.00% <0.00%> (ø) 4.00 <4.00> (?)
src/Field_SQL_Expression.php 94.73% <ø> (+0.29%) 9.00 <0.00> (ø)
src/Field/Email.php 68.96% <11.11%> (-26.28%) 19.00 <7.00> (+6.00) ⬇️
src/Field/Integer.php 27.27% <27.27%> (ø) 6.00 <6.00> (?)
src/Field/Line.php 33.33% <33.33%> (ø) 5.00 <5.00> (?)
src/Field/Text.php 40.90% <40.90%> (ø) 13.00 <13.00> (?)
src/Field/Numeric.php 56.25% <56.25%> (ø) 21.00 <21.00> (?)
src/Field.php 63.82% <67.18%> (-14.70%) 57.00 <17.00> (-38.00)
src/Field/DateTime.php 80.00% <80.00%> (ø) 18.00 <18.00> (?)
... and 45 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1230c3b...1230c3b. Read the comment docs.

@romaninsh romaninsh added the Saasty.io 🏗️ Sponsored by SaaSsy.io - Online ATK app builder label Jan 8, 2019
@romaninsh romaninsh changed the base branch from feature/epic-field-refactor to develop July 24, 2019 21:22
Copy link
Member Author

@romaninsh romaninsh left a comment

Choose a reason for hiding this comment

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

WIP..

src/Field.php Outdated
throw new ValidationException([$this->name => 'Must not be empty']);
}
break;
throw new Exception(['Use Field\Text for type=text', 'this'=>$this]);
Copy link
Member Author

Choose a reason for hiding this comment

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

didnt' we agree on the meeting to keep backwards compatibility ?

if we accept that, it will break code in a very "python" way (no steady update path)

Copy link
Member

Choose a reason for hiding this comment

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

It should be BC so far. Derived field classes don't call parent::normalize() so this switch is not used at all. Theoretically all this normalize method in Field class can be removed, but not 100% yet. That's why I left it there for cases when Field->type === null (some custom fields).

src/Field.php Outdated
throw new ValidationException([$this->name => 'Must use scalar value']);
// only string type fields can use empty string as legit value, for all
// other field types empty value is the same as no-value, nothing or null
if ($f->type && $f->type != 'string' && $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.

probably incorrect.......

Copy link
Member

Choose a reason for hiding this comment

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

what exactly?

if (!$this->owner->strict_types) {
return $value;
if ($value === null) {
if ($this->required) {
Copy link
Member Author

Choose a reason for hiding this comment

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

rename into not_null

src/Field.php Outdated
} catch (Exception $e) {
$e->addMoreInfo('field', $this);
// validate scalar values
if (in_array($f->type, ['string', 'text', 'integer', 'money', 'float']) && !is_scalar($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.

we don't care about type, remove.

src/Field.php Outdated
throw $e;
// normalize
// @TODO remove this block in future - it's useless
switch ($f->type) {
Copy link
Member Author

Choose a reason for hiding this comment

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

switch not needed. remove.

*/
public function canBeNull() : bool
{
return $this->mandatory === false;
Copy link
Member Author

Choose a reason for hiding this comment

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

return !$this->not_null

}

if (is_string($value) && $this->owner && $this->owner->persistence) {
$value = $this->owner->persistence->jsonDecode($this, $value, false);
Copy link
Member Author

Choose a reason for hiding this comment

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

is that needed ?

*
* @return string
*/
public function toString($value = null) : ?string
Copy link
Member Author

Choose a reason for hiding this comment

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

use native serialization

Copy link
Member

Choose a reason for hiding this comment

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

why? jsonEncode in persistence class is native, but better method because it treats exception cases better.

src/Model.php Outdated
protected $typeToFieldSeed = [
'boolean' => ['Boolean'],
'boolean' => ['Boolean'],
'float' => ['Numeric'],
Copy link
Member Author

Choose a reason for hiding this comment

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

'float' => ['LegacyField', 'type'=>'float']

etc

Copy link
Collaborator

@abbadon1334 abbadon1334 left a comment

Choose a reason for hiding this comment

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

LGTM, i agree to hardcode in some way the notNull exception

georgehristov and others added 4 commits February 24, 2020 22:20
* badge

* Feature/set custom edit exec button (#490)

* fix one test

* Setting current dependencies

* use custom exec button on edit action

* Update Model.php

* remove class, use seed

Co-authored-by: Imants Horsts <[email protected]>

* model default add field property (#454)

* use the object default add field property

* no need for double brackets.

* force tests

Co-authored-by: Romans Malinovskis <[email protected]>
Co-authored-by: Imants Horsts <[email protected]>

* Fix empty array condition (#498)

* more tests

* Apply fixes from StyleCI

* better implementation for addFields() and test cases (#499)

* better implementation for addFields() and test cases

* Apply fixes from StyleCI

* use shorthand method

* simplify code

* implement `exprNow()` method. Rely on dsql PR.

* Apply fixes from StyleCI

* pass

* add docs

Co-authored-by: Romans Malinovskis <[email protected]>

* Fix/ Small Action related fix (#502)

* Fix/ Small Action related fix

- Set Edit action exec button labe lto 'Save' by default
 - Set Edit action exec button colot to blue by default
- Set default description for Add action in model
- Add method to retreive model from Action

* Apply fixes from StyleCI

* fix comment

Co-authored-by: Imants Horsts <[email protected]>

* use property to define field seed properties

* update field test

* introduce expandable field type registry

* update test to new properties

* revert type setting

* add shorthand int type declaration

* refactor variable names for clarity and consistency

* introduce Field::getTypecaster and Field::getSerializer methods

* introduce registering field seeds from array

* chore: refactor variable names for consistency

Co-authored-by: Imants Horsts <[email protected]>
Co-authored-by: Mimo <[email protected]>
Co-authored-by: Romans Malinovskis <[email protected]>
Co-authored-by: Alain Belair <[email protected]>
* badge

* Feature/set custom edit exec button (#490)

* fix one test

* Setting current dependencies

* use custom exec button on edit action

* Update Model.php

* remove class, use seed

Co-authored-by: Imants Horsts <[email protected]>

* model default add field property (#454)

* use the object default add field property

* no need for double brackets.

* force tests

Co-authored-by: Romans Malinovskis <[email protected]>
Co-authored-by: Imants Horsts <[email protected]>

* Fix empty array condition (#498)

* more tests

* Apply fixes from StyleCI

* better implementation for addFields() and test cases (#499)

* better implementation for addFields() and test cases

* Apply fixes from StyleCI

* use shorthand method

* simplify code

* implement `exprNow()` method. Rely on dsql PR.

* Apply fixes from StyleCI

* pass

* add docs

Co-authored-by: Romans Malinovskis <[email protected]>

* Fix/ Small Action related fix (#502)

* Fix/ Small Action related fix

- Set Edit action exec button labe lto 'Save' by default
 - Set Edit action exec button colot to blue by default
- Set default description for Add action in model
- Add method to retreive model from Action

* Apply fixes from StyleCI

* fix comment

Co-authored-by: Imants Horsts <[email protected]>

* update multiple delete example (#503)

* now supports multiple filter options in getFields()

* Apply fixes from StyleCI

* Accept any DateTimeInterface impl. for datetime (#505)

* Accept any DateTimeInterface impl. for datetime

* Fix DateTime::getTimezone method name

* Fix setTimezone for any instance of DateTimeInterface

* Add microseconds persistence support for datetime/time types + fix normalization/cloning issue (#504)

* Fix datetime normalization cloning

* Add microseconds persistence support for datetime/time types

Co-authored-by: Imants Horsts <[email protected]>

* include comment about hooks and example how to execute them (#510)

* update composer

* Update release-drafter.yml

* Update bundler.yml

* Fix hasOne relation seed processing. It should replace not merge. (#512)

* New method $model->getTitles() (#513)

* Implement `getTitles()` method

* Apply fixes from StyleCI

* Update unit-tests.yml

* migration to migrator

* rename addHook to onHook (#514)

* fix #944 (#516)

* allow dots in table names, fix #515, fix #517

* Apply fixes from StyleCI

* Do not fail-fast PHP test matrix (#522)

* Simplify code (#519)

* Update release-drafter.yml

* Update release-drafter.yml

* Fix hook trait usage (#525)

* Hook args must be an array

* Fix hook onHook() usage

* Apply fixes from StyleCI

* initialization of fields and handling of expressions

* Apply fixes from StyleCI

* introduce hasPersistence and retrieval of setting

* Apply fixes from StyleCI

Co-authored-by: Imants Horsts <[email protected]>
Co-authored-by: Mimo <[email protected]>
Co-authored-by: Romans Malinovskis <[email protected]>
Co-authored-by: Alain Belair <[email protected]>
Co-authored-by: DarkSide <[email protected]>
Co-authored-by: Michael Voříšek <[email protected]>
georgehristov and others added 2 commits April 17, 2020 23:48
# Conflicts:
#	composer.json
#	locale/en/atk.php
#	locale/it/atk.php
#	locale/ru/atk.php
#	src/Field.php
#	src/Field/Boolean.php
#	src/Field/Callback.php
#	src/Field_SQL.php
#	src/Model.php
#	src/Persistence.php
#	src/Persistence/Array_.php
#	src/Persistence/SQL.php
#	src/Reference/HasOne.php
#	src/Reference/HasOne_SQL.php
#	src/Util/DeepCopy.php
#	tests/FieldTest.php
#	tests/FieldTypesTest.php
#	tests/RandomTest.php
@mvorisek
Copy link
Member

A lot of changes are already integrated, different fields itself should be fixed in #727, closing this one.

@mvorisek mvorisek closed this Apr 19, 2021
@mvorisek mvorisek deleted the feature/number-field branch April 19, 2021 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Saasty.io 🏗️ Sponsored by SaaSsy.io - Online ATK app builder waitingForAnotherPR
Development

Successfully merging this pull request may close these issues.

5 participants