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

Printing options #5786

Merged
merged 19 commits into from
Nov 1, 2023
Merged

Conversation

wolflu05
Copy link
Contributor

@wolflu05 wolflu05 commented Oct 25, 2023

This PR should implement printing options to allow printing plugins to expose some options that can be set on each printing job from within the printing dialog, that get passed to the printing function on the plugin to dynamically control e.g. orientation, black/white mode, ... I think this gets also pretty interesting in combination with #4805 (@martonmiklos) to allow to skip certain grid positions to reuse a not fully used A4 label sheet on a regular printer.

image

Initial concept idea summary

This could look something like this, but keep in mind, this is just a mockup, that is not yet implemented, as the CUI code is not that easy. I described it a bit below.
image

This is not yet ready. I created this PR as a discussion base. Basically I see two options.

  1. Leave the API like it is and just add a POST handler to expose the plugin serializer option fields via the OPTIONS request (This is what currently is implemented in this PR on the backend site).
    • ✅ No big, breaking backend changes required
    • ❌ Not so easy to integrate in the frontend. Each client (CUI, PUI, app, python interface, ...) needs to implement the fetching of printing plugins, templates, ... on them self, and add the required OPTIONS fetching logic if the plugin select input changes to dynamically adjust the printing options to the selected plugin
  2. Refactor the API, so that label template, plugin, parts are not sent as query/path parameters but in the body. That way, I can use my dependent field (see explanation below) implementation to add a custom serializer to the plugin. (This is what I currently have more or less on my machine implemented, but not pushed because its not ready, because I didn't created a 4th view yet)
    • ✅ Easy integration into all clients once the dependent field is supported, which I need anyway later for the machine integration
    • ❌ This would require a big API change, because the template pk was previously a path parameter, but if I move this into the body, this cannot be a path parameter anymore, so a new 4th API few per label type would be required with duplicate code for the two printing views. (If I only move the ?plugin=xxx parameter into the body, clients still have the same amount of work, because the template fetching needs to be implemented still.) I can also utilize my DependentField here for the fetching of the available templates, like so:
    class LabelPrintSerializer(serializers.Serializer):
        items = serializers.ListField(child=serializers.IntegerField())
        template = DependentField(depends_on=["items"], field_serializer="get_template_serializer")
        printing_options = DependentField(depends_on=["plugin"], field_serializer="get_printing_options_serializer")
    
        def __init__(self, *args, **kwargs):
            super().__init__(*args, **kwargs)
    
            # add plugins field to serializer
            plugins = registry.with_mixin("labels", active=True)
            plugin_choices = [(plg.slug, f"{plg.name} - {plg.human_name}") for plg in plugins]
            self.fields["plugin"] = serializers.ChoiceField(choices=plugin_choices)
    
        def get_template_serializer(self, data):
            return self.context["get_template_serializer"](data)
    
        def get_printing_options_serializer(self, data):
            return self.context["get_printing_options_serializer"](data)
    
    # context function implementation in label/api.py:LabelPrintMixin
    def get_template_serializer(self, data):
         templates = label.models.get_templates_for_items(
             self.ITEM_MODEL,
             self.ITEM_MODEL.objects.filter(pk__in=data.get("items", [])),
             self.queryset.all(),
         )
    
         return serializers.ChoiceField(choices=[(t.pk, f"{t.name} - {t.description}") for t in templates])
    
     def get_printing_options_serializer(self, data):
         plugin = self.get_plugin(data.get("plugin", None))
         return plugin.get_printing_options_serializer(self.request)



Dependent field

What I already implemented is a new serializer field called DependentField. The following example should illustrate the functionality. Basically the client now needs to send an OPTIONS request with the fields that are already filled out e.g. {"a": "Test"}, and then the DependentField called the defined field_serializer method with the body, which then could resolve in a serializer or None which the frontend should then just update on his displayed field list. This should also work nested. Im pretty sure this will be easy to implement in PUI, but I want to still support CUI. (If we don't go with the second approach, I will still need this DependetField for the machine integration, as the machines should be able to expose options too. But there I first need to select the plugin, then the machine, and then I could resolve the correct options from the machine driver definition via a DependentField.)

class SubTestSerializer(serializers.Serializer):
    c = serializers.CharField()
    cc = DependentField(depends_on=["c"], field_serializer="get_cc_field")

    def get_cc_field(self, fields):
        if fields["c"] == "1":
            return serializers.IntegerField()
        if fields["c"] == "2":
            return serializers.CharField()

class TestSerializer(serializers.Serializer):
    def d_field_serializer(self, fields):
        return SubTestSerializer()

    a = serializers.CharField()
    b = serializers.CharField()
    d = DependentField(depends_on=["a"], field_serializer="d_field_serializer")

Please let me know your thought and if something is unclear, I tried to explain it a bit better here with some examples.

I collapsed the initial design concept options above as they are not relevant anymore. I now decided to go with my option 1 and maybe later if the label app gets refactored, some time (so that it only has one label table to simplify everything that has to do with labels), option 2 can be build. That's also why I added the printing_options as a kwarg, so that we don't have to break the plugin API later if we want to send the printing options not directly in the request body, but using a dependent field or something else.

This PR is ready for review now. I added a section in the documentation for plugin developers about how to use the printing options. Implementing this in PUI is another task, as there is basically nothing regarding printing already.

@netlify
Copy link

netlify bot commented Oct 25, 2023

Deploy Preview for inventree-web-pui-preview ready!

Name Link
🔨 Latest commit 4c98fb4
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/654221b6786cac00086b168e
😎 Deploy Preview https://deploy-preview-5786--inventree-web-pui-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 100 (no change from production)
Accessibility: 86 (no change from production)
Best Practices: 92 (no change from production)
SEO: 70 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@wolflu05 wolflu05 added enhancement This is an suggested enhancement or new feature plugin Plugin ecosystem api Relates to the API labels Oct 25, 2023
@wolflu05 wolflu05 mentioned this pull request Oct 25, 2023
@matmair
Copy link
Member

matmair commented Oct 25, 2023

I would prefer option 1 but if the solution works I would not object to either. My opinion regarding new features in CUI is probably known.

@wolflu05
Copy link
Contributor Author

Maybe I just implement option 1 and sometime later if we can refactor the label app to only have one table, better api endpoints, ... and then implement the printing options like in option 2.

Or what do you think @SchrodingersGat especially with the app support, do you want to just implement the dependent field in the app form framework and then everything works, or do you think its better to implement the dynamic options fetching depending on the selected plugin in all clients (options 1)?

@wolflu05 wolflu05 added this to the 0.13.0 milestone Oct 27, 2023
@wolflu05 wolflu05 marked this pull request as ready for review October 27, 2023 13:30
@wolflu05
Copy link
Contributor Author

@SchrodingersGat @matmair This would now be ready for review. I updated the initial PR description and added some docs + testing.

Copy link
Member

@matmair matmair left a comment

Choose a reason for hiding this comment

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

Nice and simple changeset; must have been a good amount of work to get it this simple; Looks great to me;
Please address (at least) the blocking remarks, nothing major in there though.

InvenTree/InvenTree/metadata.py Show resolved Hide resolved
InvenTree/label/api.py Show resolved Hide resolved
InvenTree/label/api.py Outdated Show resolved Hide resolved
InvenTree/label/api.py Show resolved Hide resolved
InvenTree/templates/js/translated/forms.js Outdated Show resolved Hide resolved
docs/docs/extend/plugins/label.md Show resolved Hide resolved
docs/docs/extend/plugins/label.md Show resolved Hide resolved
@SchrodingersGat
Copy link
Member

@wolflu05 thanks for this, looks like a very clean implementation. Once @matmair is happy with your changes I'll do another review and see how it fits in with the app :)

@SchrodingersGat
Copy link
Member

@wolflu05 API version needs fixing again! Ping me when that's updated and I'll review again for you

@wolflu05
Copy link
Contributor Author

wolflu05 commented Nov 1, 2023

@SchrodingersGat done.

One side question, do I need to update the date in the api_version.py so that it is in sequence with the version numbers or can I leave the date like matmair did recently?

@SchrodingersGat
Copy link
Member

I don't think it overly matters if the dates are out of sync a bit

@SchrodingersGat
Copy link
Member

Screenshot 2023-11-01 at 10 09 45 pm

Using this as an opportunity to implement inventree/inventree-brother-plugin#30

Copy link
Member

@SchrodingersGat SchrodingersGat left a comment

Choose a reason for hiding this comment

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

LGTM and works well on my end

@SchrodingersGat
Copy link
Member

@wolflu05 happy to merge?

@wolflu05
Copy link
Contributor Author

wolflu05 commented Nov 1, 2023

Yes, happy to merge now. I'm currently preparing another pr related to #4824 (hopefully the last so that everything for the machine registry is in place) which will add some tiny non breaking adjustments to the printing options.

@SchrodingersGat SchrodingersGat merged commit a114183 into inventree:master Nov 1, 2023
24 of 25 checks passed
@SchrodingersGat
Copy link
Member

Thanks for all your work here @wolflu05 :)

@wolflu05 wolflu05 deleted the feature/printing-options branch November 2, 2023 12:30
@martonmiklos
Copy link
Contributor

Hi folks!

I am very glad that it got implemented!
Shall I update the #4805 to utilize this for printing with label skipping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Relates to the API enhancement This is an suggested enhancement or new feature plugin Plugin ecosystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants