setFieldDefaults() in resource/create processor

☆ A M B ☆
91 Posts

Fi1osof Reply #1, 13 years, 2 months ago

in resource/create processor in ::setFieldDefaults() executes $this->setProperties($scriptProperties);

But this not set default properties, this SET properties and rewrite my default properties.
Is will more good if we set $this->setDefaultProperties($scriptProperties); ?

https://github.com/modxcms/revolution/blob/develop/core/model/modx/processors/resource/create.class.php#L193
https://github.com/modxcms/revolution/blob/develop/core/model/modx/processors/resource/create.class.php#L253

MODX Revolution expert
Follow ME on Twitter | Feel free ask any question on MODX-club

24,544 Posts

BobRay Reply #2, 13 years, 2 months ago

If you want to set default properties for Resource fields, you can do it with Form Customization. http://rtfm.modx.com/display/revolution20/Form+Customization+Rules

Did I help you? Buy me a beer
Get my Book: MODX:The Official Guide
MODX info for everyone: http://bobsguides.com/modx.html
My MODX Extras
Bob's Guides is now hosted at A2 MODX Hosting

☆ A M B ☆
91 Posts

Fi1osof Reply #3, 13 years, 2 months ago

Quote from: BobRay at Aug 04, 2013, 01:18 AM

If you want to set default properties for Resource fields, you can do it with Form Customization. http://rtfm.modx.com/display/revolution20/Form+Customization+Rules

Thanks, but i`m programmer, not manager. Processors have two methods - setProperties() and setDefaultProperties(). setDefaultProperties() do not overwrite existing $this->properties, but setProperties() do.
I mean, in modResourceCreateProcessor::setFieldDefaults() needs setDefaultProperties(), not setProperties(), cause if i`ll extends modResourceCreateProcessor by my own custom class and write this:

public function setFieldDefaults(){
    $this->setDefaultProperties(array(
        'createdby' => $user->id,
    ));
    return parent::setFieldDefaults();
}

parent::setFieldDefaults() will overwrite my createdby by $this->setProperties().

MODX Revolution expert
Follow ME on Twitter | Feel free ask any question on MODX-club

24,544 Posts

BobRay Reply #4, 13 years, 2 months ago

I'm not sure I understand your question. Are you suggesting that there's a bug in the resource/create processor?

I know the intention is that $scriptProperties will always take precedence over any defaults. Are you saying that's not the case with resources?

Did I help you? Buy me a beer
Get my Book: MODX:The Official Guide
MODX info for everyone: http://bobsguides.com/modx.html
My MODX Extras
Bob's Guides is now hosted at A2 MODX Hosting

☆ A M B ☆
91 Posts

Fi1osof Reply #5, 13 years, 2 months ago

Quote from: BobRay at Aug 04, 2013, 03:12 AM

I'm not sure I understand your question. Are you suggesting that there's a bug in the resource/create processor?

It`s logical error, but needs discuss (i was hope Jason reply).
I think it`s must replace with $this->setDefaultProperties() and lot of defaults declare in $this->initialize();

Quote from: BobRay at Aug 04, 2013, 03:12 AM

I know the intention is that $scriptProperties will always take precedence over any defaults.

It was before processors became class-based. Class-based processors have not $scriptProperties, they have (array)$this->properties;

MODX Revolution expert
Follow ME on Twitter | Feel free ask any question on MODX-club

24,544 Posts

BobRay Reply #6, 13 years, 2 months ago

Quote from: BobRay at Aug 04, 2013, 03:12 AM

I know the intention is that $scriptProperties will always take precedence over any defaults.

It was before processors became class-based. Class-based processors have not $scriptProperties, they have (array)$this->properties;

Yes, but the the correct way to call them is with $modx->runProcessor():

public function runProcessor($action = '',$scriptProperties = array(),$options = array()) {
    // ...
    $processor->setProperties($scriptProperties);
    // ...
}

so those properties come from any $scriptProperties that are set earlier. AFAIK, they should not be overwritten, though, so if that's happening when they are called with runProcessor() for resources, I think it's a bug.

Did I help you? Buy me a beer
Get my Book: MODX:The Official Guide
MODX info for everyone: http://bobsguides.com/modx.html
My MODX Extras
Bob's Guides is now hosted at A2 MODX Hosting

☆ A M B ☆
91 Posts

Fi1osof Reply #7, 13 years, 2 months ago

Yes, but the the correct way to call them is with $modx->runProcessor():
public function runProcessor($action = '',$scriptProperties = array(),$options = array()) {
    // ...
    $processor->setProperties($scriptProperties);
    // ...
}

Yes, correct. But this is MODx`s method. But in modProcessor class not exists $scriptProperties, exists $this->properties.
See modProcessor::__construct()

function __construct(modX & $modx,array $properties = array()) {
        $this->modx =& $modx;
        $this->properties = $properties;
}

$scriptProperties - it`s in modDeprecatedProcessor.

Yes, looks like bug. But it is not clear. Have a look: here many checking for existing data: https://github.com/modxcms/revolution/blob/develop/core/model/modx/processors/resource/create.class.php#L193
If we already check data, we can use $this->setProperties();
But some data not checking, for example: https://github.com/modxcms/revolution/blob/develop/core/model/modx/processors/resource/create.class.php#L206
But i want to set another creator (it`s user in frontend create documents. They not yet authorized and $modx->user != this user, but i know it`s him, and i want set creator).
And i must execute

public function setFieldDefaults(){
    parent::setFieldDefaults();
    $this->setDefaultProperties(array(
        'createdby' => $user->id,
    ));
    return true;
}

instead

public function setFieldDefaults(){
    $this->setDefaultProperties(array(
        'createdby' => $user->id,
    ));
    return parent::setFieldDefaults();
}

So, here needs to set $this->setDefaultProperties() OR check all overwrited properties.

MODX Revolution expert
Follow ME on Twitter | Feel free ask any question on MODX-club

24,544 Posts

BobRay Reply #8, 13 years, 1 month ago

Do you have an actual case where you set a property in the second argument to runProcessor('resource/create', $properties) and it was overwritten by a default property of the Resource?

That would prove it's a bug.

Did I help you? Buy me a beer
Get my Book: MODX:The Official Guide
MODX info for everyone: http://bobsguides.com/modx.html
My MODX Extras
Bob's Guides is now hosted at A2 MODX Hosting

☆ A M B ☆
91 Posts

Fi1osof Reply #9, 13 years, 1 month ago

Quote from: BobRay at Aug 06, 2013, 01:41 AM

Do you have an actual case where you set a property in the second argument to runProcessor('resource/create', $properties) and it was overwritten by a default property of the Resource?

That would prove it's a bug.

You may simply do this:

<?php

$data = array(
    'pagetitle' => 'New document',
    'createdby' => $userID,  // ID of another user, not your
);
$modx->runProcessor('resource/create', $data);

It`s will create new document, but createdby will by id of your user, not another.

MODX Revolution expert
Follow ME on Twitter | Feel free ask any question on MODX-club

MODX Staff
10,725 Posts

opengeek Reply #10, 13 years, 1 month ago

This looks like a bug to me. Do we have an issue in the tracker? Pull request maybe?

Jason Coward
Chief Architect @ MODX
http://www.jasoncoward.com | http://twitter.com/drumshaman | https://github.com/opengeek