Report merge

[tristanle22]

Allow generation of a signle report PDF when multiple items are selected.

Closes #9423

[netlify[bot]]

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

Name	Link
🔨 Latest commit	b8b9bf28736698de6e2dd95403524fc664dde93d
🔍 Latest deploy log	https://app.netlify.com/projects/inventree-web-pui-preview/deploys/6854c75fb5b6cb00087a7d79
😎 Deploy Preview	https://deploy-preview-9532--inventree-web-pui-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 96 (🔴 down 1 from production) Accessibility: 81 (no change from production) Best Practices: 100 (no change from production) SEO: 78 (no change from production) PWA: - View the detailed breakdown and full score reports

---

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

[matmair]

As the API schema is changed you will also need to change the API version - see https://docs.inventree.org/en/stable/develop/contributing/#api-versioning

Let me know if you want more detailed pointers

[tristanle22]

As the API schema is changed you will also need to change the API version - see https://docs.inventree.org/en/stable/develop/contributing/#api-versioning

Let me know if you want more detailed pointers

Is it just simply updating "src/backend/InvenTree/InvenTree/api_version.py" by bumping up the version and add a new line to INVENTREE_API_TEXT?

[matmair]

Yes; Just copy the latest entry and adjust to fit this PR (especially the url)

[SchrodingersGat]

@tristanle22 a great start here :) Please begin by addressing the issues raised by @matmair

[matmair]

Looks pretty good now; it mainly is missing the migration I mentioned above and a bit of unit-testing to ensure your changes continue to work in the future; Do you want pointers to those topics?

[tristanle22]

@matmair For migration, there are some new Python files generated in src/backend/InvenTree/report/migrations/ after running invoke migrate, I guess they need to be included in this PR? For the testing part, I'm guessing the report unit tests live here src/backend/InvenTree/report/tests.py?

[matmair]

Both correct @tristanle22

[tristanle22]

Is there a way to run src/backend/InvenTree/report/tests.py with VSCode debugger?

[SchrodingersGat]

https://docs.inventree.org/en/latest/develop/contributing/?h=dev.test#run-tests-locally

In your case

invoke dev.test -r report.tests

[codecov[bot]]

Codecov Report

:x: Patch coverage is 88.00000% with 9 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 86.57%. Comparing base (45daef8) to head (b8b9bf2). :warning: Report is 319 commits behind head on master.

Files with missing lines	Patch %	Lines
src/backend/InvenTree/report/models.py	83.63%	9 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9532      +/-   ##
==========================================
+ Coverage   86.56%   86.57%   +0.01%     
==========================================
  Files        1242     1243       +1     
  Lines       54698    54752      +54     
  Branches     2234     2234              
==========================================
+ Hits        47348    47402      +54     
  Misses       6782     6782              
  Partials      568      568              

Flag	Coverage Δ
backend	88.52% <88.00%> (+0.01%)	:arrow_up:

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.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[SchrodingersGat]

@tristanle22 some further things I would like to see here:

Migrations

You have a two migrations here, 0030 and 0031.

• 0030 shadows an existing migration
• 0031 simply combines the two "0030" migrations

Can you please delete 0031, and rename 0030 to 0031.

Unit Testing

Your unit testing does not actually test for multiple items being combined into a single report as you only send it a single item.

I would suggest that your unit test needs to:

• Supply multiple items
• Ensure that the report output generates a single report, not multiple reports stacked together
• You can utilize the "DEBUG MODE" for reports here to return HTML and then you can read the HTML to ensure it is formatting properly

Documentation

Please add some documentation (in ./docs/) for this new feature:

• What the "merge" feature does, and how to use it
• How to specify which report templates get merged

[tristanle22]

https://docs.inventree.org/en/latest/develop/contributing/?h=dev.test#run-tests-locally

In your case

invoke dev.test -r report.tests

I setup this command in launch.json, but it doesn't stop at any breakpoints. I want to place breakpoints to play around with the variables to see how the pieces work together. Do you have any suggestions?

[matmair]

There is a pre-defined launch definition for this provided in the code base, check out the launch.json file

[matmair]

@tristanle22 looking good; could you resolve the merge conflict so we can do a final review round?

[wolflu05]

Do I understand it correct, that now we have another "dimension" of possible context generation in a matrix.

So now we have different contexts for:

• type: label / report
• component: part, stocklocation, ...
• merged report: true, false

=> 2 * (~9) * 2 = ~36 different context combinations

Meaning if merge is set, all instance specific context is available in instances.x.<name> instead of <name>, but e.g. width (for labels) is still available as width and not instances.x.width ?

Just asking, because I have to adjust my LSP plugin accoringly now.

[tristanle22]

Do I understand it correct, that now we have another "dimension" of possible context generation in a matrix.

So now we have different contexts for:

• type: label / report
• component: part, stocklocation, ...
• merged report: true, false

=> 2 * (~9) * 2 = ~36 different context combinations

Meaning if merge is set, all instance specific context is available in instances.x.<name> instead of <name>, but e.g. width (for labels) is still available as width and not instances.x.width ?

Just asking, because I have to adjust my LSP plugin accoringly now.

Yes both correct

[tristanle22]

@SchrodingersGat @matmair I think it's ready for a final round of review. Let me know if there's anything else you'd like to add/updated!

[SchrodingersGat]

@tristanle22 I'm back from vacation now and ready to review. Can you please look at the conflicts here and I can review again :)

[matmair]

@SchrodingersGat I have resolved the conflicts

[SchrodingersGat]

Documentation

The "merge" feature needs some clearer documentation in the "report" page (./docs/docs/report/report.md).

In particular, the difference in terminology in the report template, e.g. use instance.<name> instead of <name>

Functionality

On testing, it appears to work as expected, nice!

[tristanle22]

Documentation

The "merge" feature needs some clearer documentation in the "report" page (./docs/docs/report/report.md).

In particular, the difference in terminology in the report template, e.g. use instance.<name> instead of <name>

Functionality

On testing, it appears to work as expected, nice!

I have a section in .docs/docs/report/context_variables.md that explains what you mentioned. I can make it clearer there, instead of in ./docs/docs/report/report.md

[SchrodingersGat]

I would still like to see some more obvious callout from the main report docs page :)

[SchrodingersGat]

@tristanle22 if you can address the documentation, resolve conflicts and look at the failing tests, I'd love to see this merged!

[tristanle22]

Sorry, I've been occupied lately. Will try to resolve the last two items.

[tristanle22]

All failing tests are because of the same error: FileNotFoundError: [Errno 2] No such file or directory: '/home/runner/work/InvenTree/media/data_output/output.html' This is how I'm grabbing the output html. It worked locally: report_path = os.path.join( settings.MEDIA_ROOT.parent, response.data['output'].lstrip('/') )

@SchrodingersGat Do you have any suggestion?

[SchrodingersGat]

All failing tests are because of the same error: FileNotFoundError: [Errno 2] No such file or directory: '/home/runner/work/InvenTree/media/data_output/output.html' This is how I'm grabbing the output html. It worked locally: report_path = os.path.join( settings.MEDIA_ROOT.parent, response.data['output'].lstrip('/') )

@SchrodingersGat Do you have any suggestion?

It should not be MEDIA_ROOT.parent - this is up one level from the media dir. I think it should be settings.MEDIA_ROOT instead

[tristanle22]

All failing tests are because of the same error: FileNotFoundError: [Errno 2] No such file or directory: '/home/runner/work/InvenTree/media/data_output/output.html' This is how I'm grabbing the output html. It worked locally: report_path = os.path.join( settings.MEDIA_ROOT.parent, response.data['output'].lstrip('/') ) @SchrodingersGat Do you have any suggestion?

It should not be MEDIA_ROOT.parent - this is up one level from the media dir. I think it should be settings.MEDIA_ROOT instead

It has to be MEDIA_ROOT.parent because:

print(response.data['output']
> /media/data_output/output_qXuMPAB.html

[SchrodingersGat]

Ah, ok so the "response" object provides the path to the file on the web server - NOT where the file resides on disk.

So, the inventree server provides the output at /media/data_output/some_file.pdf' - which is a path relative to the host.

On disk, it will be somewhere like /my/path/to/inventree-media/data_output/some_file.pdf/

So, take the response path, strip the /media/ off the start, and then join that the MEDIA_ROOT

[SchrodingersGat]

@tristanle22 thanks for implementing this, it's a great addition! Very nice work :)