I need to add an attribute to the Event class via plugin. I’m facing this issue:
I patched the EventDataForm in order to have the new field in the form, but the populate_from_dict method raises a ValueError: Event has no attribute "...". I patched the Event model class as well in order to have the attribute available in both: class and table. It worked properly and fixed the issue from the populate_from_dict.
The RHEditEventDataBase._process calls update_event, which, at the first line has an assert with an hardcoded list of updatable attributes. Obviously my attribute is not in the list.
I assume the list above is there to prevent some malicious data from the client-side.
Is there some guideline or a suggested strategy to address this scenario?
I think the easiest (yet dirty) option would be patching the function so a custom one runs first and removes & processes your custom keys, and then calls the original function forthe rest.
There is another issue related to this topic, the update_events tries also to log the action creating a user log entry. the function _log_event_update has a very similar issue. It tries to properly log the action with some constraint and rules specified in a hardcoded dictionary. I assume is for having a better natural-language logging machinery, but if something is not in the dictionary (such as in my case) the _log_event_update raises a KeyError. Why don’t default to a basic default rule such as "Field ... changed. foo -> bar.
This sounds easy to fix. If there is no side-effects at glance from you, i can create a pull-request to add this default rule.
You are right, but the core is (of course) not expecting the kind of patching you do. Anyway, if you get rid of the custom data keys before calling the original function, this KeyError won’t happen (but you also won’t get the log entry for them).
If I get rid of the custom data the _log_event_update will not log the change. If I need to choose among having a bad log format instead of no log at all, I prefer to have a badly formatted (but readable) log and I still think it’s better for you as well.
The disadvantage I see in handling unknown keys without an error is that it’ll be very easy to forget adding “pretty” logging when adding some new key in the core at some point…
But I don’t see how you would get this function to be called at all if you process your own data before getting into update_event where the assertion fails… so you could just log it as a second change.
It doesn’t work due to the loading order, when the plugin is imported that module is already imported. what you suggested has no impact to the final result. if you execute module foo.py as:
from indico.modules.events.management.controllers.settings import update_event
and after you execute
The last code will have no impact on the already referenced variables in the module foo so foo.update_event will points still to the same old function and in my case, in a plugin, the plugin is loaded after the core.