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

Form improvements #5837

Merged
merged 25 commits into from
Nov 13, 2023
Merged

Conversation

wolflu05
Copy link
Contributor

@wolflu05 wolflu05 commented Nov 2, 2023

This PR adds a few things which improve the form framework. This is hopefully the last PR that is needed to continue #4824.

Specifically the following is changed:

  1. A new serializer field named DependentField is added. To test this new serializer field, the printing plugin code at the end can be used. This is actually a sample implementation of how I imagine the machine printing to work.
  2. CUI now fully supports nested object serializer fields in the form framework.
  3. CUI now supports the new DependentField
Bildschirmaufnahme.2023-11-02.um.13.23.18.mov

I actually tried to implement the nested serializer field in PUI, but I sadly have not found my way around the PUI form framework code, so this would be another task.

Please let me know what you think of that implementation and if there's anything I should change.


Sample printing plugin code for testing
from django.utils.translation import ugettext_lazy as _
from InvenTree.serializers import DependentField
from label.models import LabelTemplate
from rest_framework import serializers

# InvenTree plugin libs
from plugin import InvenTreePlugin
from plugin.mixins import LabelPrintingMixin


class SettingsTestPlugin(LabelPrintingMixin, InvenTreePlugin):
    AUTHOR = "wolflu05"
    DESCRIPTION = "Label printing test plg"
    VERSION = "0.1.1"

    NAME = "LabelPrintingTest"
    SLUG = "labelprintingtest"
    TITLE = "Label printing test PLG"

    def print_label(self, **kwargs):
        return super().print_label(**kwargs)

    def print_labels(self, label: LabelTemplate, items: list, request, **kwargs):
        return super().print_labels(label, items, request, **kwargs)

    class PrintingOptionsSerializer(serializers.Serializer):
        machine = serializers.ChoiceField(choices=[
            ("a", "Machine A"),
            ("b", "Machine B"),
            ("c", "Machine C"),
        ])

        def get_machine_options(self, data):
            if data["machine"] == "b":
                return BMachineOptionsSerializer()

        machine_options = DependentField(depends_on=["machine"], field_serializer="get_machine_options")


class BMachineOptionsSerializer(serializers.Serializer):
    select_orientation_type = serializers.CharField()
    orientation = DependentField(depends_on=["select_orientation_type"], field_serializer="get_orientation_field")

    def get_orientation_field(self, fields):
        if fields["select_orientation_type"] == "1":
            return serializers.IntegerField()
        if fields["select_orientation_type"] == "2":
            return serializers.CharField()
        if fields["select_orientation_type"] == "3":
            return serializers.ChoiceField(choices=[
                ("landscape", "Landscape"),
                ("portrait", "Portrait"),
            ])

Copy link

netlify bot commented Nov 2, 2023

Deploy Preview for inventree-web-pui-preview canceled.

Name Link
🔨 Latest commit e33f3b4
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/6548bd53938245000845d131

@wolflu05 wolflu05 mentioned this pull request Nov 2, 2023
@wolflu05 wolflu05 added enhancement This is an suggested enhancement or new feature user interface User interface labels Nov 2, 2023
@wolflu05 wolflu05 marked this pull request as ready for review November 2, 2023 12:26
@wolflu05
Copy link
Contributor Author

wolflu05 commented Nov 9, 2023

@SchrodingersGat , @matmair this would be ready to review. I know you have a lot of other things todo with PUI, but this would be the last improvement to CUI to raise the printing options to a next level.

@SchrodingersGat
Copy link
Member

@wolflu05 an interesting approach! Does this mean that you have to query the server on each field change, to request the new sub-field information?

@wolflu05
Copy link
Contributor Author

Exactly @SchrodingersGat , an OPTIONS request is sent on any field change of a field that is included in the depends_on list. For text fields, a change is you edit and then click outside of the field (I used the onEdit handler for that). That way we can easily create different forms based on other values.

@SchrodingersGat
Copy link
Member

Ok, that seems reasonable. Will you integrate support for this into PUI too?

@wolflu05
Copy link
Contributor Author

I actually tried, but didn't got my way around. Now after I refactored the forms in #5897 I may understand the logic. If you really wish this should be done now I can see what I can do, otherwise the plan was to finish the machine integration first for what I would need this PR. But that's up to you.

@SchrodingersGat
Copy link
Member

I'm not expecting the PUI implement straight away - but it should be kept in mind...

@SchrodingersGat
Copy link
Member

I'm happy to merge this in for now though, thanks!

@SchrodingersGat SchrodingersGat added this to the 0.13.0 milestone Nov 13, 2023
@SchrodingersGat SchrodingersGat merged commit 0d193d8 into inventree:master Nov 13, 2023
24 checks passed
const processField = (name, field, optionsField) => {
if (field.type === "nested object") {
for (const [k, v] of Object.entries(field.children)) {
processField(`${name}__${k}`, v, optionsField.children[k]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wolflu05 this has broken existing form handling. e.g. try to edit an existing purchase order:

image

Please look into this one ASAP

@SchrodingersGat
Copy link
Member

@wolflu05 please see comment above - this PR has broken some forms

SchrodingersGat added a commit that referenced this pull request Nov 14, 2023
@SchrodingersGat
Copy link
Member

Looks like there are some other on-flow issues too. For now I will revert - #5913 - and we will need to re-asssess

SchrodingersGat added a commit that referenced this pull request Nov 14, 2023
@wolflu05
Copy link
Contributor Author

Sorry that this introduced some errors, I really thought I tested with the part form and it worked. I'll definitely take a look and fix that. Good that you reverted the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an suggested enhancement or new feature user interface User interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants