Skip to content
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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

yakky
Copy link
Collaborator

@yakky yakky commented Sep 9, 2024

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

@ajrbyers
Copy link
Member

ajrbyers commented Sep 11, 2024

@yakky just an FYI I've added this to our planning call for discussion.

Copy link
Member

@mauromsl mauromsl left a 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.

Comment on lines +26 to +46
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)

Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants