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

Feature/refactor crud2 #835

Merged
merged 63 commits into from
Nov 23, 2019
Merged

Feature/refactor crud2 #835

merged 63 commits into from
Nov 23, 2019

Conversation

romaninsh
Copy link
Member

Refactor CRUD to rely on new actions, clean up older implementation(s)

@romaninsh romaninsh requested a review from ibelar November 7, 2019 20:28
@codecov
Copy link

codecov bot commented Nov 11, 2019

Codecov Report

Merging #835 into develop will decrease coverage by 4.7%.
The diff coverage is 21.26%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #835      +/-   ##
=============================================
- Coverage      73.35%   68.65%   -4.71%     
- Complexity      2514     2578      +64     
=============================================
  Files            125      126       +1     
  Lines           5919     6049     +130     
=============================================
- Hits            4342     4153     -189     
- Misses          1577     1896     +319
Impacted Files Coverage Δ Complexity Δ
src/TableColumn/ActionMenu.php 37.73% <ø> (-50.95%) 19 <0> (ø)
src/jsToast.php 100% <ø> (ø) 5 <0> (ø) ⬇️
src/ActionExecutor/UserAction.php 0% <0%> (-27.28%) 97 <2> (+1)
src/ActionExecutor/UserConfirmation.php 0% <0%> (ø) 21 <21> (?)
src/Text.php 50% <0%> (-16.67%) 5 <1> (+1)
src/Grid.php 55.15% <100%> (-18.56%) 79 <0> (ø)
src/FormField/Input.php 91.13% <12.5%> (-8.87%) 34 <0> (+2)
src/CRUD.php 41.89% <37.68%> (-6.11%) 43 <36> (-11)
src/View.php 81.79% <40%> (-5.5%) 162 <0> (+3)
src/Menu.php 93.22% <50%> (-1.52%) 26 <0> (+1)
... and 17 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 5661df4...0dd35ed. Read the comment docs.

$single_record_action->ui['executor'] = $executor;
$executor->addHook('afterExecute', function ($x, $m, $id) {
return $m->loaded() ? $this->jsSave($this->notifyDefault) : $this->jsDelete();
});
Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@acicovic
Copy link
Collaborator

While refactoring this I think it would be worth it to also fix #837

@romaninsh
Copy link
Member Author

romaninsh commented Nov 23, 2019

@ibelar still breaks test.. is it due to dependency on atk data branch?

@ibelar
Copy link
Contributor

ibelar commented Nov 23, 2019

@romaninsh - Look like it. Seem the error come from calling getConfirmation() on the atk4/Data/UserAction/Generic

@romaninsh
Copy link
Member Author

@ibelar let me merge and then test

@romaninsh romaninsh merged commit 4fad5d4 into develop Nov 23, 2019
@romaninsh romaninsh deleted the feature/refactor-crud2 branch November 23, 2019 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants