Persistence->typecastSaveRow shouldn't do validation #439
Assignees
Labels
hangout agenda 🔈
Will discuss on next hangout
Comments
|
That's not a validation, it's type conformity. It is explained in a doc, like saving string into boolean or long text into varchar10 |
DarkSide666
added a commit
to atk4/audit
that referenced
this issue
Jun 13, 2019
romaninsh
added a commit
to atk4/audit
that referenced
this issue
Jun 13, 2019
fix issue related to atk4/data#439
DarkSide666
added a commit
to atk4/audit
that referenced
this issue
Feb 11, 2020
* Update README.md * improve support for bypassing log * Improve better separation of reactive diffs * minor bugfixes * Improve handling of new records * upgrade for new agiledata * Minor bug-fixes * drop php 5.5 and update phpunit * prevent php7.1 from complaining * clanup * add licnse * Update README.md * make it work with verisons or dev version * fix license * try with dev-develop * Sample SQL for audit_log table in MySQL * typo * model_id was missing * remove database name * don't log dsql expressions. maybe also helps with #21 * Cleanup and refactor * Theoretical implementation of audit Lister view * Implement Lister and create demo * nice lister template and update lister class * work in progress * work in progress * now all works good * add pug files * address @romans comments and rename controller property to auditController * empty audit records * make demo nicer and move db migrator to separate page * more test cases for nested audit logs and undo etc * help with backward compatibility * model->no_audit=true support * fix audit for serialized fields, adds test * add release tools * added suggest * fixes * tweak release script * minor cleanups * minor fix * minor cleanups * Added release notes for 1.1.1 * minor cleanups * minor celanup * add manage of caption and reference/One values in place of ref->id * fix issue related to atk4/data#439 * Fix problem with using audit log under cli * update composer * Create release-drafter.yml * Create bundler.yml * Create unit-tests.yml * release drafter * Setting release dependencies Co-authored-by: Romans Malinovskis <me@nearly.guru> Co-authored-by: Imants Horsts <DarkSide666@users.noreply.github.com> Co-authored-by: PhilippGrashoff <33204878+PhilippGrashoff@users.noreply.github.com> Co-authored-by: Francesco Danti <fdanti@gmail.com> Co-authored-by: Svetlozar Kondakov <skondakoff@gmail.com> Co-authored-by: GitHub Web Flow <noreply@github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently
Persistence->typecastsaveRow()includes validation check for null values https://github.com/atk4/data/blob/develop/src/Persistence.php#L191).It's not that bad to validate value early, but this is not correct place to put validations in. Typecasting should only do what it have to do and nothing else.
Also it's bad for atk4\audit because atk4\audit uses this typecastSaveRow method to typecast values and store them in audit table. In case value changed from
nullto some meaningful value it still needs to typecast that (previous/old)nullvalue and if field havemandatory=>trueset then typecastSaveRow will throw exception.Workaround is to use
required=>trueinstead ofmandatory=>true, but that's not always good.Anyway the point is - typecastSaveRow method should not include any validation (validation exceptions). It should only typecast what it can and that's all.
Maybe this is for v2.0 if you think that this change can break v1.x
The text was updated successfully, but these errors were encountered: