-
Notifications
You must be signed in to change notification settings - Fork 65
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
RFC: Use setting to get submit_info form #4393
base: master
Are you sure you want to change the base?
RFC: Use setting to get submit_info form #4393
Conversation
@yakky just an FYI I've added this to our planning call for discussion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @yakky ,
Sorry it has taken some time to get to this draft. We like the idea of being able to easily override the form class, but there are some concerns about being able to load arbitrary code using the Janeway settings interface - more comments inline.
As for overriding the templates, there is a feature (well, more of a happy accident) where you can use the Django template engine and settings to load custom backoffice templates. It can be helpful when you also need to alter how the form is laid out, since Django's formfield templates will not allow you to control the outer HTML.
def get_submit_info_form(request): | ||
if request.user.is_editor(request): | ||
custom_form = setting_handler.get_setting( | ||
"general", "submit_info_form_editor_version", request.journal | ||
) | ||
else: | ||
custom_form = setting_handler.get_setting( | ||
"general", "submit_info_form_general_version", request.journal | ||
) | ||
form_path = custom_form.processed_value | ||
return import_string(form_path) | ||
|
||
|
||
|
||
def get_submit_info_edit_form(request): | ||
custom_form = setting_handler.get_setting( | ||
"general", "submit_info_form_general_version", request.journal | ||
) | ||
form_path = custom_form.processed_value | ||
return import_string(form_path) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is neat idea. One thing to consider is that the settings have been configured to be accessible by both editors and journal managers; Since all it takes is a path to a python module, it opens up a window for arbitrary code execution.
It would be safer to implement a registry of allowed forms (perhaps controlled in settings.py
so that it can be expanded on) and then only exposing editors to a choice field.
The implementation could be similar to other registry-based loaders (e.g. template tag/filter registry). We could assist with that if needed.
I would like to be able to customize submission forms (and templates as a consequence) to modify the widgets and the behaviour of the form, without having to override the views or doing something tricky as that.
Replacing direct import of forms with import_string function + settings would make the overriding effortless, with any significant complexity in the janeway code and in a completely backward compatible code.
I attached an example about an implementation for ArticleInfoSubmit to get your feedback about the general idea