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

empty $data array() returned in wire/core/Modules.php prevents saving modules configuration #859

Closed
Da-Fecto opened this issue Jan 8, 2015 · 7 comments

Comments

@Da-Fecto
Copy link

Da-Fecto commented Jan 8, 2015

in 2.5.14 public static function getModuleConfigInputfields(array $data) get an empty array, but that shouldn't be that way.

Report here: https://processwire.com/talk/topic/7588-admin-custom-files/?p=84276
Maybe related: https://processwire.com/talk/topic/4865-custom-upload-names/page-4#entry84264

@Da-Fecto
Copy link
Author

Da-Fecto commented Jan 8, 2015

On this commit the 'bug' is introduced.

on this line the empty array is returned.

// changing this (line 1951 wire/core/Modules.php )
// if(!isset($this->configData[$id])) return array(); // module has no config data
// if(is_array($this->configData[$id])) return $this->configData[$id];

// to this solves the issue
if(is_array($this->configData[$id])) {
    if (count($this->configData[$id])) return $this->configData[$id];
    return array(); // module has no config data
}

@adrianbj
Copy link

adrianbj commented Jan 8, 2015

Thanks for taking the time to track this down Martijn.

To avoid missing index notices, I modified slightly to:

if(isset($this->configData[$id]) && is_array($this->configData[$id])) {
    if (count($this->configData[$id])) return $this->configData[$id];
    return array(); // module has no config data
}

I can confirm this also fixes the issues with Custom Upload Names to you link to above.

@Da-Fecto
Copy link
Author

Da-Fecto commented Jan 8, 2015

The isset is a weird one Adrian (I think). $this->configData[$id] is just an array at this point, so the code reads to me like: isset(array()). Is that even possible ?

@adrianbj
Copy link

adrianbj commented Jan 8, 2015

I have always used it, but that might not make it right :)

You could always use:

array_key_exists($id, $this->configData)

instead, but I don't see an issue with isset in this case. Here's an interesting read:
http://stackoverflow.com/questions/3210935/difference-between-isset-and-array-key-exists

@Da-Fecto
Copy link
Author

Da-Fecto commented Jan 8, 2015

Thanks for the info & glad it fixes the others issue.
(Don't know why I mentioned the isset thingy 👎 )

@Da-Fecto Da-Fecto changed the title Issues with $data Issues with $data (Saving configuration modules ) Jan 9, 2015
@Da-Fecto Da-Fecto changed the title Issues with $data (Saving configuration modules ) Issues with $data (saving configuration modules ) Jan 9, 2015
@Da-Fecto Da-Fecto changed the title Issues with $data (saving configuration modules ) empty $data array() returned from wire/core/Modules.php prevents saving modules configuration Jan 9, 2015
@Da-Fecto Da-Fecto changed the title empty $data array() returned from wire/core/Modules.php prevents saving modules configuration empty $data array() returned in wire/core/Modules.php prevents saving modules configuration Jan 9, 2015
@Da-Fecto
Copy link
Author

Patch fixed this issue as well.

@Da-Fecto
Copy link
Author

Latest 2.5.14 seems to fix all instances I see/find.

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

No branches or pull requests

2 participants