search

virtudraft / CrossContextsSettings

A MODX's custom plugin to manage cross contexts' settings
6 stars 4 forks source link

Context setting getting deleted #8

Closed minagerges closed 9 years ago

minagerges commented 9 years ago

If a setting was not created for one of the contexts, updating the setting value causes the setting to be deleted for all contexts.

Reproduction steps: 1-Create a new setting for 1 context 2-Open CrossContextSettings CMP 3-Change the value of the created setting for another context.

Result: the setting is totally deleted.

Expectations: the setting should have been created for the other context with the entered value

goldsky commented 9 years ago

tested it on MODX 2.3.3-pl, and it seemed working to me?

minagerges commented 9 years ago

Interesting it always deleted the context setting for me on 3 different installs, but on 2.3.1, you think it's modx?

Kind regards, Mina Gerges

Sent from mobile device, apolgies for any mistyping. On Jul 25, 2015 4:59 AM, "rico" notifications@github.com wrote:

tested it on MODX 2.3.3-pl, and it seemed working to me?

— Reply to this email directly or view it on GitHub https://github.com/virtudraft/CrossContextsSettings/issues/8#issuecomment-124787081 .

goldsky commented 9 years ago

yes, I'm proposing update to MODX. https://github.com/modxcms/revolution/pull/12525

Mark-H commented 9 years ago

Sounds like this core fix that landed in 2.3.3 might be relevant: https://github.com/modxcms/revolution/pull/12099

goldsky commented 9 years ago

hmm... I think we are talking about different issue here. It should be https://github.com/virtudraft/CrossContextsSettings/issues/7

minagerges commented 9 years ago

Still reproducible after upgrade to Modx 2.3.5 Once attempting to add a value to the setting on another context, the setting gets deleted. Setting is boolean, Key "session_enabled"

1- Create the setting for one context. 2- Open CrossContextSettings CMP and put a value for another context. Refresh and the setting will be totally deleted, confirm that setting is deleted on the context setting page.

NB, updating the same context setting value first time worked, an second time deleted the setting.

minagerges commented 9 years ago

Have a look on "updatefromgrid.class.php" starting from line 84 ~ 87 Why attempting to delete the setting ??

goldsky commented 9 years ago

If you set empty value, the setting must be removed from the context's scope to allow general (System) setting to set the default value. Otherwise, your context will still set that property, but with empty value. Usually, FATAL ERROR occurs.

But I'll investigate again your case.

minagerges commented 9 years ago

I found the cause, sending a pull in few minutes

Kind regards, Mina Gerges

On Sun, Jul 26, 2015 at 3:26 PM, rico notifications@github.com wrote:

If you set empty value, the setting must be removed from the context's scope to allow general (System) setting to set the default value. Otherwise, your context will still set that property, but with empty value. Usually, FATAL ERROR occurs.

But I'll investigate again your case.

— Reply to this email directly or view it on GitHub https://github.com/virtudraft/CrossContextsSettings/issues/8#issuecomment-124976658 .

minagerges commented 9 years ago

Tested that and it works better now for both changes in "updatefromgrid.class.php": 1- 0 is treated as empty, causing setting to be deleted (Line 60) 2- No need to save a setting with same value again (no changes.)

goldsky commented 9 years ago

oh, 0! Ok, thanks for point that out.! For 2, thanks for reminding me this. I fixed this slightly different. https://github.com/virtudraft/CrossContextsSettings/commit/df6a5fa7f830bebe3bf7eca5a5c4229beabfcae7