Universal Module Won't Update Active Service Fields When Their Name Change

[Rodrigo]

Hello,

 

When I was making some patches to the module, I noticed that when you have a Package with Universal Module, which have active services on clients, and you go to module configuration and change the name of a service field, the value of the active clients will disappear.

 

 

Example:

 

Before changes, looking at a client's service:

Package: Foo (using universal module)

Service Fields:

  'bar' => "barcustomvalue23443"

  'baz' => "bazcustomvalue2353454562"

 

Now, let's change the service field names at admin CP:

 'bar' gets renamed to 'bar1'

 'baz' gets renamed to 'baz1'

 

After changing service field names, looking at a client's service:

Package: Foo (using universal module)

Service Fields:

  'bar1' => ""

  'baz1' => ""

 

So, after the service field name change the values won't be preserved. This could be a problem for an installation with many field

 

Expected Behavior:

 Current services should maintain their values, but get the service name changed as well:

  'bar1' => "barcustomvalue23443"

  'baz1' => "bazcustomvalue2353454562"

 

*Using Blesta 3.2.1

[Tyson]

What is your purpose in changing the Name field?

 

The Name field acts as the key, and the expected behavior for key=>value pairs is to maintain the key's association with a variable value. You are also expecting the converse; to maintain the value's association with a variable key. Such a requirement is only logical for one-to-one relationships, which is not the case here. 

 

Additionally, if the module were to support propagating updates from the Name field with existing service fields (and I'm saying it should not), I would expect that it should also enforce the Required and Values settings under the Universal Module product. And enforcing those would be problematic as well.

 

So I don't see this as a bug, but rather that it is currently working as intended. You may want to re-evaluate your intent on modifying the Name field since it does not maintain backward compatibility with pre-existing service fields, as you've already discovered.

[Rodrigo]

I agree with your explanation, there should be no reason to change the key, but someone might do it and have bad consequences on his installation. I don't know if the data gets to a blackhole, or it just keeps saved on the database without reference in the code, but putting a real life case as example, if someone wants to change a service field key from "ip" to "host" because he wants to put FQDNs in some cases (so "ip" would not represent the field anymore), he will have a bad time with the data loss/dereference.

 

I know that in that case changing the service field key is not necessary, but I think you should protect customers from themselves (As an hosting company owner I have to deal a lot with layer 8-made issues).

 

It's not urgent, but I think it's necessary to at least include a warning to prevent users from incurring in bad practices. 

[Tyson]

The thing is, we can't protect you from yourself. It's difficult, and sometimes impossible, to provide a user with both the freedom to do what they want in the system, and protecting them from making mistakes. While documentation might help, not many people look at documentation before they perform an action anyway. And I don't think anybody wants to see notes/messages on most pages that warns them and describes what not to do, in case they perform an action whose consequences are unfavorable to their expectations.

 

While some additional documention may help, I think the best solution is to exercise additional responsibility in management of products. I imagine most people understand that changing a product's settings changes it only for future use, and leaves existing services alone.

 

That said, your example of changing the Name field retains the old service information, but it is simply not shown any longer for existing services. Maybe it should be, and maybe there should be an option to remove it for a service. Something like this might be a good feature request.

[Tyson]

Closing this thread as not a bug for aforementioned reasons.