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

Registration plugin flow could be more developer friendly #3359

Closed
sergei-maertens opened this issue Aug 11, 2023 · 0 comments · Fixed by #3933
Closed

Registration plugin flow could be more developer friendly #3359

sergei-maertens opened this issue Aug 11, 2023 · 0 comments · Fixed by #3933

Comments

@sergei-maertens
Copy link
Member

Thema / Theme

Plugins

Omschrijving / Description

The registration flow was broken up recently into more steps:

  1. pre-registration
  2. generate PDF
  3. main registration

but the base plugin classes have not been updated appropriately.

I've developed a plugin for a registration service that generates its own references. This requires me to use the hook pre_register_submission and store the reference + relevant data directly on the submission model:

        submission.registration_result = {
            "id": data["id"],
            "reference": data["reference"],
            "qrcode_data": data["qrcode_data"],
        }
        submission.public_registration_reference = data["reference"]
        submission.save()

The previous API (register_submission and get_reference_from_result) did not rely on internals - instead I could reason 100% in terms of what my registration backend returns.

To make matters worse, get_reference_from_result and register_submission no longer have any purpose - all my registration is already done in the pre-registration hook, but the base plugin class enforces me to implement these two methods.

Looking at the test suite, there are many tests that call get_reference_from_result directly, but this method isn't really called anymore during the registration flow, the expectation is that a reference is set and saved during pre-registration (as the PDF generation tries to read it, which runs after pre-registration!)

Added value / Toegevoegde waarde

  1. Better developer experience for registration plugin developers
  2. Backwards compatibility (my existing plugin was now broken after updating to latest open forms)

Aanvullende opmerkingen / Additional context

I think it'd make sense to expect pre_register_submission return a RegistrationResult dataclass, which has result: dict and reference: str fields (optional). The tasks should then extract that information and save it on the submission.

It would probably also still be nice to call the get_reference_from_result hook after running pre-registration, instead of expecting the plugin to set the field value directly on the model.

Finally, register_submission should probably receive an explicit result field, as the pre-registration task may need to store temporary data and do the expensive processing after PDF generation.

@sergei-maertens sergei-maertens added triage Issue needs to be validated. Remove this label if the issue considered valid. enhancement labels Aug 11, 2023
@SilviaAmAm SilviaAmAm self-assigned this Jan 22, 2024
@joeribekker joeribekker removed triage Issue needs to be validated. Remove this label if the issue considered valid. topic: personal pick labels Jan 22, 2024
@joeribekker joeribekker added this to the Release 2.6.0 milestone Jan 22, 2024
SilviaAmAm added a commit that referenced this issue Feb 23, 2024
The plugins no longer need to worry about the submission internals and should only return an object with the reference and any extra data
SilviaAmAm added a commit that referenced this issue Feb 26, 2024
SilviaAmAm added a commit that referenced this issue Feb 26, 2024
SilviaAmAm added a commit that referenced this issue Feb 28, 2024
The plugins no longer need to worry about the submission internals and should only return an object with the reference and any extra data
SilviaAmAm added a commit that referenced this issue Feb 28, 2024
SilviaAmAm added a commit that referenced this issue Feb 28, 2024
SilviaAmAm added a commit that referenced this issue Mar 1, 2024
The plugins no longer need to worry about the submission internals and should only return an object with the reference and any extra data
SilviaAmAm added a commit that referenced this issue Mar 1, 2024
SilviaAmAm added a commit that referenced this issue Mar 1, 2024
SilviaAmAm added a commit that referenced this issue Mar 1, 2024
sergei-maertens added a commit that referenced this issue Mar 1, 2024
…pre-registration-ref

[#3359] Refactor pre registration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants