InvenTree icon indicating copy to clipboard operation
InvenTree copied to clipboard

Report printing refactor

Open SchrodingersGat opened this issue 1 year ago • 25 comments

This PR presents a major refactoring of how we handle report templates. In particular, a lot of model duplication and API endpoint duplication is removed. This is a precursor to supporting reports in the new interface, and potentially expanding the functionality of the reporting framework into the future.

Note that this is a breaking change as we are removing a number of models, and also deleting a number of existing API endpoints

Related Issues

  • Closes https://github.com/inventree/InvenTree/issues/5882
  • Closes https://github.com/inventree/InvenTree/issues/5841
  • Related to https://github.com/inventree/InvenTree/issues/6417

Features / Updates

  • Create new generic ReportTemplate database model
  • Migrate all existing report template tables into this new one
  • Reduce many report API endpoints to a single generic set

TODO

  • [x] Refactor label printing models / API in a similar fashion
  • [x] Commonize the "report context" data across labels / reports
  • [x] Allow template table(s) to be filtered by "model type"
  • [ ] Update unit testing
  • [x] Add playwright testing for report and label printing via web interface
  • [ ] Add unit test for data migration
  • [ ] Update documentation
  • [ ] Pull "context" data directly into the documentation from the codebase
  • [x] Test that all CUI report printing functionality continues to function correctly
  • [x] Test that all CUI label printing functionality continues to function correctly
  • [x] Implement report printing in new PUI interface
  • [x] Implement label printing in new PUI interface
  • [x] refactor template editor in PUI to use new API
  • [x] Update python bindings to support new API endpoints - https://github.com/inventree/inventree-python/pull/226
  • [x] Update python bindings again now that we have changed the plugin API - https://github.com/inventree/inventree-python/pull/229
  • [x] Update app to support new API endpoints - https://github.com/inventree/inventree-app/pull/489
  • [x] Update app again now that we have changed the plugin API - https://github.com/inventree/inventree-app/pull/491
  • [x] Refactor "include_installed" option for stock report
  • [x] Refactor generate_filename method - ensure it works for reports and labels
  • [x] Get label printing working again (label size is not observed)
  • [x] Automatically add "default" label templates on launch
  • [x] Automatically add "detault" report templates on launch
  • [x] Ensure that both builtin "label printing" plugins still work
  • [x] Reimplement / refactor API endpoints for "printing" labels and reports with proper serializers
  • [ ] Ensure backwards compatibility with existing label printer plugins
  • [ ] Ensure that the "machines" backend works
  • [ ] ~~Profile existing label printing code - where are the bottlenecks?~~
  • [x] Perform full migration run to ensure old labels and templates are migrated correctly
  • [x] Move templating db model mixin into report.mixins

SchrodingersGat avatar Apr 21 '24 14:04 SchrodingersGat

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

Name Link
Latest commit 07ca9a189da7f9444028652cbb51b0b0b306cee1
Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/664c91cbc22a470008493ca6
Deploy Preview https://deploy-preview-7074--inventree-web-pui-preview.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

Lighthouse
1 paths audited
Performance: 99 (🔴 down 1 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.

netlify[bot] avatar Apr 21 '24 14:04 netlify[bot]

Looks promising! I am still convinced that reports and labels should be the same thing but we have to start somewhere.

matmair avatar Apr 21 '24 14:04 matmair

Codecov Report

Attention: Patch coverage is 78.76712% with 217 lines in your changes missing coverage. Please review.

Project coverage is 83.21%. Comparing base (921bf91) to head (07ca9a1). Report is 320 commits behind head on master.

Files Patch % Lines
src/backend/InvenTree/report/api.py 76.56% 45 Missing :warning:
...nTree/report/migrations/0026_auto_20240422_1301.py 50.87% 28 Missing :warning:
...nTree/report/migrations/0023_auto_20240421_0455.py 44.44% 20 Missing :warning:
...rontend/src/components/buttons/PrintingActions.tsx 58.69% 18 Missing and 1 partial :warning:
src/backend/InvenTree/report/models.py 88.81% 16 Missing :warning:
src/backend/InvenTree/report/apps.py 75.00% 12 Missing :warning:
...mponents/editors/TemplateEditor/TemplateEditor.tsx 0.00% 11 Missing :warning:
...s/editors/TemplateEditor/PdfPreview/PdfPreview.tsx 0.00% 8 Missing :warning:
src/frontend/src/tables/settings/TemplateTable.tsx 63.63% 7 Missing and 1 partial :warning:
src/backend/InvenTree/stock/models.py 68.42% 6 Missing :warning:
... and 24 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7074      +/-   ##
==========================================
- Coverage   84.07%   83.21%   -0.87%     
==========================================
  Files        1056     1069      +13     
  Lines       46440    46854     +414     
  Branches     1334     1376      +42     
==========================================
- Hits        39046    38989      -57     
- Misses       7055     7508     +453     
- Partials      339      357      +18     
Flag Coverage Δ
backend 85.18% <82.10%> (-0.25%) :arrow_down:
pui 65.00% <60.25%> (-0.04%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 21 '24 14:04 codecov[bot]

Looks promising! I am still convinced that reports and labels should be the same thing but we have to start somewhere.

I somehow have the same feeling. Especially if we encourage users to use report assets / snippets for labels.

And I have a few questions regarding this refactor:

  1. what impact has this change on the PUI template editor?
  2. are we still gonna save templates as files and not storing the content in the db?
  3. is there an option to render a template without saving it first, but provide the template data in the request body?

wolflu05 avatar Apr 21 '24 15:04 wolflu05

Looks promising! I am still convinced that reports and labels should be the same thing but we have to start somewhere.

I somehow have the same feeling. Especially if we encourage users to use report assets / snippets for labels.

And I have a few questions regarding this refactor:

  1. what impact has this change on the PUI template editor?
  2. are we still gonna save templates as files and not storing the content in the db?
  3. is there an option to render a template without saving it first, but provide the template data in the request body?
  1. The PUI template editor will still function - I'll just have to tweak the URLs a bit
  2. No, at least not yet. We will have to consider the implementation and security implications, and I do not believe the library that @matmair linked is ready for django 4.2 yet?
  3. No, but I guess we could make an API endpoint for previewing a template? Now that all templates are "the same" this would be a much simpler activity. I'd leave that for a separate PR though.

SchrodingersGat avatar Apr 21 '24 22:04 SchrodingersGat

I think we will have to create our own db-based template renderer; I can not find something that fits our thread model

matmair avatar Apr 22 '24 10:04 matmair

Just a thought I already have since a while: maybe we can build the new api in a way that we can also expose the current available context and their sub-/sub-/... attributes via an api endpoint which we can later use in the PUI template editor to provide auto completion with some description to the properties.

wolflu05 avatar Apr 22 '24 15:04 wolflu05

Not sure of what the scope of this PR is, but maybe we can somehow get the request object out of the template rendering functions when doing this refactor. This would simplify things with workers much, as Pickeling the request object does not work. And IMHO having the request while rendering a template is not really necessary. The only thing I can think of is having the user that triggered the printing job, but we should rather pass that as a separate context argument. For more info see #6772

wolflu05 avatar Apr 29 '24 06:04 wolflu05

Not sure of what the scope of this PR is, but maybe we can somehow get the request object out of the template rendering functions when doing this refactor. This would simplify things with workers much, as Pickeling the request object does not work. And IMHO having the request while rendering a template is not really necessary. The only thing I can think of is having the user that triggered the printing job, but we should rather pass that as a separate context argument. For more info see #6772

@wolflu05 I am trying to limit scope creep (otherwise this will never get done). But, I have been looking right now at the "request" object due to similar issues so I might be able to implement this.

SchrodingersGat avatar Apr 29 '24 06:04 SchrodingersGat

May I ask you, if you could please add a quick explanation of how the new system works now compared to the old? Like how are the templates stored now, what endpoints are available and how the templates interfere with plugins?

wolflu05 avatar May 09 '24 10:05 wolflu05

May I ask you, if you could please add a quick explanation of how the new system works now compared to the old? Like how are the templates stored now, what endpoints are available and how the templates interfere with plugins?

Certainly! Let's consider "labels" below ("reports" are handled in a very similar fashion).

Label Template Model

There is now a single database table where all "label templates" are stored:

https://github.com/SchrodingersGat/InvenTree/blob/f02c2b6dad0c39e8168719936025fb391ba9f86b/src/backend/InvenTree/report/models.py#L346

Instead of multiple label db models (e.g. "part label" / "location label") the new table has a column "model_type" which refers to the type of model referenced by the particular label template.

This new template model is made available to the API at /api/label/template/. You can filter by model type e.g. /api/label/template/?model_type=part

The label template table in the admin interface shows this as follows:

image

And you can filter here by model type:

image

image

Migrations

The "label" app is now removed entirely - previous labels are migrated across to the "report" app in a data migration.

Printing

There is a single printing endpoint: /api/label/print/. You need to pass:

  • A label template to against
  • A plugin to use
  • A list of items (primary keys) of the items to print

This API endpoint is now much better implemented and self-described via DRF:

image

Label Selection

To properly integrate with the REST API selection and validation, the "plugin" needs to be a PluginConfig object - which references a concrete item in the database.

Editing

The template editor has been refactored (and simplified due to the single endpoint). Still works as expected:

image

Some things to note here:

  • POST operation to the /api/label/print/ endpoint
  • Responds with a JSON object, with a link to a generated file
  • This change will allow us to (in the future) run printing actions in the background worker thread and provide progress updates to the user

User Interface

User interface code has thusly been greatly simplified as there are now only a small number of API endpoints to consider.


I hope this overview helps. Let me know if there are any other aspects you want more details on!

SchrodingersGat avatar May 09 '24 11:05 SchrodingersGat

Thank you very much for your explanation to get a overview of this change. It really helped to understand the fundamentals.

And you can filter here by model type:

  1. Is that normal that in your screenshot the names are sometimes lower and sometimes uppercased?

  2. I'm not sure if it's just me, but this filtering looks like bad UX to me, as:

  • a lot of clicks are required to filter for a specific template
  • by default all templates are shown
  • the filters are not persisted in the url query params, so sharing a link with a particular template selected is not possible Don't get me wrong, I don't say the previous method of filtering was the best, but it was really easy to navigate between the different template types.

Do I understand this right:

  1. an API endpoint to post template code against for previewing is up for further work to enable a more "live" preview feeling?
  2. labels can also be rendered as a pdf via the /label/print api endpoint now without using a plugin. And to that point labels are now completely similar to reports. The only difference between reports and labels is that reports don't have the plugin print integration?

wolflu05 avatar May 09 '24 13:05 wolflu05

  1. Is that normal that in your screenshot the names are sometimes lower and sometimes uppercased?

That's due to the internal "verbose name" values of the database tables - I'll look into that:

Edit: Fixed now

  1. I'm not sure if it's just me, but this filtering looks like bad UX to me, as:

This is how table filtering works in PUI. I agree that it could be improved. Happy to review a PR :)

an API endpoint to post template code against for previewing is up for further work to enable a more "live" preview feeling?

Yep, could now quite easily support a "preview" endpoint. Not in the scope of this refactor though.

labels can also be rendered as a pdf via the /label/print api endpoint now without using a plugin. And to that point labels are now completely similar to reports. The only difference between reports and labels is that reports don't have the plugin print integration?

Correct.

SchrodingersGat avatar May 10 '24 04:05 SchrodingersGat

So is there something we can help with to get this over the finish line?

matmair avatar May 10 '24 06:05 matmair

@matmair getting the unit tests updated to address the new API is the big blocker currently. Most of the other things are done!

SchrodingersGat avatar May 10 '24 06:05 SchrodingersGat

@SchrodingersGat schould we split that up? One does inventree-python, one inventree for example?

matmair avatar May 10 '24 06:05 matmair

@SchrodingersGat schould we split that up? One does inventree-python, one inventree for example?

I'd appreciate that! I've started work on the inventree-python tests - https://github.com/inventree/inventree-python/pull/226 - perhaps you want to tackle the outstanding printing unit tests here? :)

SchrodingersGat avatar May 10 '24 07:05 SchrodingersGat

@SchrodingersGat I am on it!

matmair avatar May 10 '24 14:05 matmair

One small question, what are revisions in context of these template models? How do they affect business logic, and how do they get bumped? For every typo fix in a template? Or is there a way to say when it should be a new revision?

wolflu05 avatar May 12 '24 11:05 wolflu05

@wolflu05 good question. As originally implemented the "revision" number increments on every save. However this is perhaps not the best approach? Open to suggestions, now would be the time to change if we wanted to.

SchrodingersGat avatar May 12 '24 11:05 SchrodingersGat

My idea would be to implement a preview button as default action to let the user try out the template without unnecessarily bumping the revision each time. This does just render a pdf without modifying the template on the server.

Then we could add a save template action which opens a modal with a checkbox labeled like "create new revision", which is default selected. And now this modal contains the actual save button. We can later extend this to also let the user specify a note about the change, and even save the different versions of templates. But that would only work easily if we save the Django templates in the database.

For example grafana or some workflow automation tools have this concept of a version history too.

wolflu05 avatar May 13 '24 09:05 wolflu05

Sounds like a lot of feature creep. The existing changeset is already very wide and a lot of new surfaces, I would not change anything more here.

matmair avatar May 13 '24 10:05 matmair

Yes this sounds like it would be good for a future PR.

SchrodingersGat avatar May 13 '24 23:05 SchrodingersGat

@matmair @wolflu05 regarding the requests to retain plugin lookup by "slug" (rather than "pk"): I am happy to do this, but I would suggest that we move all the plugin lookup in the API to use slug rather than PK.

e.g. /api/plugin/inventreebarcode/ - rather than /api/plugin/17/

The reason for this is to allow the printing endpoint to use the plugin slug for the foreign key field. For consistency I'd just roll that out across all the plugin endpoints.

If you are both OK with that, I'd submit that as a separate PR, and wait for that to be merged before continuing with this.

SchrodingersGat avatar May 14 '24 09:05 SchrodingersGat

I am bumping this to the next release cycle, as there is still significant work to do here.

SchrodingersGat avatar May 14 '24 09:05 SchrodingersGat

@wolflu05 @matmair last remaining hurdle is the failing playwright test. The test runs successfully but gets stuck on "teardown". Any ideas? Really struggling on this one.

SchrodingersGat avatar May 21 '24 06:05 SchrodingersGat

@matmair @wolflu05 thanks for your help on this one. Merging in now, we have some time before 0.16.0 to work out any issues which may rise from such a significant diff.

SchrodingersGat avatar May 22 '24 00:05 SchrodingersGat

Should we maybe open a follow up issue with all the future ideas for this area that were voiced here?

matmair avatar May 22 '24 05:05 matmair

Should we maybe open a follow up issue with all the future ideas for this area that were voiced here?

Yes I think that's a great idea

SchrodingersGat avatar May 22 '24 06:05 SchrodingersGat