Refactor review forms feature to reusable custom forms feature

[ViRuSTriNiTy]

Hi there,

I need to use the features of the review forms code (e.g. create form, add elements, ...) in my own code that is not related to review forms.

To avoid recreating the wheel I refactored the existing code in such a way that it is reusable hence the name custom forms. I have almost finished the work on it and I would like to get some input whether this could be integrated into pkp-lib. This would avoid the need to merge constantly between possible review forms changes and my own custom forms code and I could give something back to the community.

What do you think?

So lonG

[NateWr]

Can you describe in more detail your particular use case?

[asmecher]

(I'm not sure if you've explored the assoc_type / assoc_id mechanism we've used elsewhere to attach this sort of general-use feature to various entities in the system -- it's not always the cleanest or most intuitive mechanism, but it's one that's already in broad use in the codebase.)

[ViRuSTriNiTy]

@NateWr

I need to extend the whole submission process with a lot of additional fields. But later on I surely need these features again somewhere else, forms are everywhere :). At first I thought of using the additional meta data stuff but this is somewhat limited. The review form handling on the other hand did show huge potential due to stuff like the assoc_type / assoc_id as @asmecher has already pointed out, the existing UI to configure forms and fields, the field variants etc. All I had to do is separating the review forms code from some dependencies as these dependencies would not make much sense in a custom form scope.

So just to name a few changes I made to give some insight:

• introduced a context class that holds all data that is shared among multiple custom form classes (like what is the name of the custom forms table or what is the name of a specific DAO class)
• some DB changes like removing replacing columns that are targeting a specific dependency that makes no sense in a custom forms scope, e.g. review_form_responses.review_id becomes review_form_responses.assoc_type -> ASSOC_TYPE_REVIEW_ASSIGNMENT in review form scope review_form_responses.assoc_id -> review ID in review form scope

• moved reusable code to CustomForm... classes like

  class CustomFormDAO extends DAO {
      ...
  
      protected function initDataObjectFromRow($customForm, $row) {
          $customFormDAOContext = $this->getCustomFormDAOContext();
  
          $formIdTableColumnName = $customFormDAOContext->getFormIdTableColumnName();
  
          $customForm->setId($row[$formIdTableColumnName]);
          $customForm->setAssocType($row['assoc_type']);
          $customForm->setAssocId($row['assoc_id']);
          $customForm->setSequence($row['seq']);
          $customForm->setActive($row['is_active']);
  
          $this->getDataObjectSettings($customFormDAOContext->getFormSettingsTableName(),
              $formIdTableColumnName, $row[$formIdTableColumnName], $customForm);
      }
  ...
  
  class ReviewFormDAO extends CustomFormDAO {
      ...
  
      protected function initDataObjectFromRow($customForm, $row) {
          parent::initDataObjectFromRow($customForm, $row);
  
          $customForm->setCompleteCount($row['complete_count']);
          $customForm->setIncompleteCount($row['incomplete_count']);
      }
  
  ...

and much more but it would reach to far at this point.

Basically with these changes you can now add a custom forms handler wherever you need it.

In my case its the section form where i added the custom forms handler via hooks and it now holds a custom forms configuration option for submission step 3.

For example:

{url|assign:customFormsUrl
  router=$smarty.const.ROUTE_COMPONENT
  component="grid.settings.customForms.CustomFormGridHandler"
  op="fetchGrid"
  assocId=$context.SectionId
  assocType=$context.SectionAssocType escape=false}
{load_url_in_div id="customFormGridHandler" url=$customFormsUrl}

It accepts the parameters assocId and assocType. In case of my current use case the assocId is the section id and assocType is ASSOC_TYPE_SECTION because I need to create forms for the submission process of a specific section. Therefore if I now create a form it is stored together with the section information hence I am able to display the form when a submission belongs to a specific section and the submission process is in step 3.

The values submitted by the user via these forms are stored in the responses table as custom_form_responses.assoc_type -> ASSOC_TYPE_SUBMISSION custom_form_responses.assoc_id -> submission ID

I hope I covered the main changes so that it's understandable. What do you say?

[NateWr]

Thanks for that @ViRuSTriNiTy, it's very helpful. It sounds like you've already gotten most of the way, so by all means please raise a Pull Request. It will be easier to look at and discuss particulars there.

In principle, I'm in favor of this idea to generalize the form creation tool. There are a few things going on at the moment that may make this an inopportune time to implement and maintain this change. I'll describe those things and then suggest some ways forward that might make this easier to maintain over time.

At the moment, we're making some pretty big architectural changes to the editorial backend for OJS/OMP, moving towards a REST API-driven approach that communicates with a UI built in Vue.js. We're still very early in this transition, but you can see a couple examples in the submissions lists and reviewer selection list.

These changes include #3594, which will move forms over to Vue.js, where they will interact with REST API endpoints. That means that the Form class and the many classes which extend it will eventually be deprecated. Forms will submit to, and be processed by, endpoints that are attached to entities (submissions, issues, contexts, etc) rather than separate form handlers.

As you can imagine, I'm hesitant to bring in a feature built around the existing forms approach when we will be refactoring it in the medium-term future. I understand, though, that our timeline may not match yours and you may need to move to production with a working solution in the near-term.

It looks like a lot of your work has been done around the database layer, so I think it might not be too difficult to migrate some of your work over to a new API endpoint, /forms, where CRUD operations for custom forms would happen. A /forms/responses endpoint could handle CRUD operations for form responses. This would inoculate your modifications from the changes that are coming to the Form classes, hopefully smoothing out the future upgrade path. I'd be happy to help you get to grips with how to create and process API endpoints, since this architecture is not yet well documented.

Now, the kicker. With the work we're doing around the API and forms, I think (hope) that we won't have as much of a use-case for custom form handling in order to extend existing forms. It should be much easier to extend a form and it's associated entity without having to declare and handle a separate form. Here's an example plugin which extends the Settings > Journal form with a new field (only works with a development branch at the moment).

We'll need a point-and-click interface to build form extensions. Submission and review are two obvious candidates; I can also imagine things like pre-publication checklists. But we'll probably not do this with entirely separate forms submitted to separate endpoints. In most cases, if we're extending an existing activity (submission, review), we'll want our modifications to flow seamlessly in with the existing process.

That means I'm not sure where the /forms API endpoint will end up. We may find that there is a real use case for it. But it might end up out-of-scope for the project. So I'm not sure what the long-term looks like for a fork of this magnitude.

Having said all of this (I know it's been a lot, but I wanted to bring you in on all of this to give you a sense of my perspective), I definitely think we'd like to look at what you've done, since it holds a lot of potential in the near-term. If it looks like something we could use and extend to the submission process in the near-term, then I'd be in favor of building it around a /_forms endpoint, which is our way of indicating that an endpoint is not for third-party consumption and may change in the future.

So please do raise a PR and link it here. And please ask any questions you may have. I know I've dumped a lot on your here.

[ViRuSTriNiTy]

Thanks for the deep insight, much appreciated. It helps me to understand why currently there is a mix of Vue.js, Smarty, and other stuff. As I understand it's more or less still a work in progress.

I'm also open to new technologies, so adopting my code to the new REST API / Vue.js approach shouldn't be an issue in the future event if it means that my custom forms approach is not necessary anymore.

Do you have an roughly estimate when this REST API / Vue.js approach will be fully implemented and released to the public?

I will try to clean up the code and raise a PR when i'm ready.

[NateWr]

Do you have an roughly estimate when this REST API / Vue.js approach will be fully implemented and released to the public?

Because of the scale of the application and the limited number of developers, there won't be a single release where this is "done". It will be rolled out in stages as we refactor parts of the system.

For example, we've got /submissions, /issues and /users endpoints in place at the moment. But we're only using them for the Submissions lists and the Reviewer selection process. When #2860 goes in, we'll make more heavy use of the /users endpoint. The first forms scheduled for refactor in #3594 involve context settings, so there will be a /context endpoint.

We'll roll out this refactored code as we replace and enhance certain feature sets. It could be years before the full refactor is complete. But we'll focus our energy on key parts of the platform so in theory we should make rapid progress over the next 6-12 months, even if we won't be able to say we're "done".

I hope to make better documentation of all of this available within 6 months, once we've successfully merged some of the big changes we're exploring in #3594 . Feel free to hold me to that schedule. If you don't see anything available in 6 months ask what the hold-up is.

[ViRuSTriNiTy]

@NateWr Offtopic, but would you please have a look at https://forum.pkp.sfu.ca/t/refactor-submission-checklist-feature-to-reusable-custom-checklist-feature/50105 ? Similar request but for submission checklists.

[NateWr]

Closing this in favor of #7191 which is a major overhaul to the submission wizard. It doesn't fully implement this request, but it brings the submission wizard onto the new forms system. It should be easier to add fields and sections to the wizard.