Extending core model and form (WTForm)

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?

Many thanks.

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.

Thank you for the quick reply, but i cannot patch a function at module level.
The plugin is executed too late to patch what is already imported.

you can patch it in the module that imported it

It’s imported by a core class before the plugin is imported.

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.

The import is in indico.modules.events.management.controllers.settings, so patching that module would work.

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.

You meant to patch the request handler instead, and injecting the update_event “replacement” into the patched request handler?

Ok, this is looks very dirty, but working.

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.

No, the import is just a normal import in the module. So you just need to do something like this:

from indico.modules.events.management.controllers import settings as event_mgmt_settings_controllers
orig_update_event = event_mgmt_settings_controllers.update_event
event_mgmt_settings_controllers.update_event = your_update_event

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.