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

Allow to get target model after deep traversal #1052

Open
mvorisek opened this issue Aug 16, 2022 · 3 comments
Open

Allow to get target model after deep traversal #1052

mvorisek opened this issue Aug 16, 2022 · 3 comments

Comments

@mvorisek
Copy link
Member

mvorisek commented Aug 16, 2022

class Order extends \Atk4\Data\Model {
 protected function init(): void
    {  
        
        parent::init();
        $this->hasMany('order_doc', ['model' => [OrderDoc::class
        ]]);
        
    }
}

class OrderDoc extends \Atk4\Data\Model { 

    protected function init(): void
    { 
        parent::init();
    
        $this->hasOne('order_id', ['model' => [Order::class]]);
        
        $this->onHook(\Atk4\Data\Model::HOOK_BEFORE_INSERT, function ($model, &$data) {
        
                // Create order if not existing
                if (!$model->get('order_id') && $model->get('customer_id')) {
                
                    $model->set('order_id', $model->refLink('order_id')->insert([...]);
                    ]));   }
        
        });
    }
}

// Use case 1: Create a parent order container, when a new child order doc is created
$newOrderDoc = (new OrderDoc($db))->createEntity();
$newOrderDoc->save([...]);

// Use case 2: Assign an order document to another order
$order = (new Order($db))->load(1);
$orderdoc = $order->ref('order_doc')->load(1);

$orderdoc->saveAndUnload(['order_id' => 2]);

When traversing on null value, we may allow the ID to change (to newly inserted). I will check what can be done and if it will not break other tests.

use case 2 - how should this work? Previously, the save conditions were relaxed, to me, incorrectly. What is the reasoning to relax save conditions when the entity cannot be loaded after?

A solution I can think of would be to implement Model::getModelWithoutRefConditions(). Currently, we do not store traversed reference info, so in $orderdoc, we have no info which condition to remove. Let's discuss this feature request later. If you have (or can put) your project into Github, I can code some helper methods specific to your projects.

@mkrecek234
Copy link
Contributor

mkrecek234 commented Aug 16, 2022

@mvorisek Just some further food for discussion to make uses cases a bit clearer:

  1. Let's say a user has restricted access to only some customer entities - e.g., he is an account manager for business and not end customers:
    $customerModel->addCondition('customer_group_id', 1); // may not access any other group

  2. He now filters his customer list showing customers which he flagged as important:

$customerModel->addCondition('customer_group_id', 1); // may not access any other group`
$grid->setModel($customerModel)
$grid->model->addCondition('flagged', true);
  1. In this $grid, the customer decides to remove the flag 'important' for a specific customer - naturally not having scope limitations in mind, you would just do:
$grid->addActionButton(['icon'=>'flag'], function ($v, $id) use ($grid) {
                    $messagemodel = (clone $grid->model);
                    $entity = $messagemodel->load($id);
                    if ($entity->get('flagged')) {
                        $entity->set('flagged', 0);
                    } else {
                        $entity->set('flagged', 1);
                    }
                    $entity->save();
                    return new \Atk4\Ui\JsReload($grid);              
                });

This would now fire an Exception, as removing the flag would move it out of the model scope of $messagemodel.

Now here is the idea:

  • There are conditions which are critical that the user stays inside these conditions (here the first condition customer_group =1

  • There are other conditions which are non critical, but are used in coding for filtering down views further (like the flagged=true condition).

If Atk4/Data knew which conditions might be released on scope validation checks, it could then be more flexible. So in this case only the customer_group condition would be checked to always be valid.

OPTIONAL: Nevertheless, I believe there should be an easy way to also release those critical conditions: Take the use case that the account manager has the right to change a customer to be an end customer: the customers thus should be handled by end customer sales team. $entity->dangerouslySaveWithoutScope(['customer_group', 2) should be allowed, if intentionally decided by the developer.

So concrete proposal:

  1. We add an optional parameter for addCondition called e.g., $weak defaulting to false which - if set to true marks to non-checked conditions on validation checks.
  2. OPTIONAL: We add another function called Model::dangerouslySaveWithoutScope which saves in a "fresh" unconditioned model / without scope.

Naturally speaking all those saves would mean saveAndUnload as after save the entity is no longer in the model scope.

@mvorisek
Copy link
Member Author

mvorisek commented Aug 17, 2022

skipping (allowing to skip) whole scope is wrong and should not be part of the framework, so no Model::dangerouslySaveWithoutScope, this is not possible also because there are more save methods, and we would need to implement all methods for such feature

however, your can achive that by creating a new model (and loading it) like you posted in the chat or maybe even easier by:

⚠️ this following example is wrong, entity clone would not clone parent model, thus the model scope is not cloned!

$entity = clone $entity;
$entity->scope()->clear();
$entity->set('xxx', 'value_out_of_original_scope');
$entity->save();

the following should work

$model = clone $entity->getModel();
$model->scope()->clear();
$entity = $model->load($entity->getId());
$entity->set('xxx', 'value_out_of_original_scope');
$entity->save();

I can think of some method like Model::cloneEntityModel() which will clone the current entity (/wo reload), clone parent model and replace the entity model, so scope could be cleared/changed /wo affecting the original model + /wo the need to reload. Example:

$entity = $entity->cloneEntityModel();
$entity->scope()->clear(); // same as $entity->getModel()->scope()->clear()
$entity->set('xxx', 'value_out_of_original_scope');
$entity->save();

@mkrecek234
Copy link
Contributor

mkrecek234 commented Aug 17, 2022

@mvorisek That sounds practical to me and very logical. Any other thoughts from the community @DarkSide666 eventually?
What about the idea of weak conditions (so we would not completely clear all conditions, but safely only the ones which are not critical)? So not all scope is cleared?

We should definitely then store those four lines of code above in a separate saveWeakly or saveOutOfScope function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants