-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add printing options framework #31
Conversation
Ok you are freakin awsome :P |
@wolflu05 has done most of the heavy lifting here (and this won't work until the linked PR is merged into InvenTree master) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation LGTM. But overall this would mean every plugin needs to implement such options.
inventree_brother/brother_plugin.py
Outdated
# Printing options requires a modern-ish InvenTree backend, | ||
# which supports the 'printing_options' keyword argument | ||
options = kwargs.get('printing_options', {}) | ||
n_copies = min(1, options.get('copies', 1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this min function? Doesn't that mean if I set copies to 10, I still get 1? I think the min/max could be generally left out, because you already set the default at the .get function.
@@ -57,6 +80,8 @@ class BrotherLabelPlugin(LabelPrintingMixin, SettingsMixin, InvenTreePlugin): | |||
SLUG = "brother" | |||
TITLE = "Brother Label Printer" | |||
|
|||
PrintingOptionsSerializer = BrotherLabelSerializer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment: Yep, that's another way of defining the options I haven't thought of, but Program wise they are exactly considered the same.
inventree_brother/brother_plugin.py
Outdated
Used to specify printing parameters at runtime | ||
""" | ||
|
||
class Meta: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment: the serializer also worked without a meta class in my tests, I guess it is only necessary for model serializers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I saw your example now, I might simplify it
For now, this only adds in a "number of copies" option. But, other options could be added too. For example if multiple printers are available on the network, select which one to use