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

UI plugins custom features #8137

Merged
merged 24 commits into from
Sep 26, 2024

Conversation

wolflu05
Copy link
Contributor

@wolflu05 wolflu05 commented Sep 17, 2024

This PR enhances the UI mixin (which currently only can provide panels) with the capability to also integrate in lots of different other parts of the UI via a generic system. E.g. table actions, row actions, custom navigation entries, ... Currently only custom template editors and previews can be provided.

I'm currently working on this label template drag and drop editor plugin: https://github.com/wolflu05/inventree-template-editor-plugin and have released the first version now. Feel free to test it.

TODO

  • refactor user interface related API and serializer code into separate files
  • basic plugin ui feature implementation
  • integrate template editors into plugin system
  • integrate template previews into plugin system
  • provide exportable types
  • add code editor sample to ui sample plugin to use in tests
  • refactor ref out of the getFeature functions as parameter and instead provided them if necessary via the renderContext, and also have the ability to specify the return type to make the system more generic for other usecases (e.g. only return a list of navigation entries, ...)
  • allow to specify the function name via the source file string divided by a colon
  • bump API version
  • add tests
  • documentation

Copy link

netlify bot commented Sep 17, 2024

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

Name Link
🔨 Latest commit ebe2f11
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/66f511fc8cf46f0008de95cd
😎 Deploy Preview https://deploy-preview-8137--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 plugin Plugin ecosystem Platform UI Related to the React based User Interface labels Sep 17, 2024
@wolflu05 wolflu05 added this to the 0.17.0 milestone Sep 17, 2024
Copy link

codecov bot commented Sep 17, 2024

Codecov Report

Attention: Patch coverage is 92.77108% with 18 lines in your changes missing coverage. Please review.

Project coverage is 84.44%. Comparing base (b1c1981) to head (ebe2f11).
Report is 9 commits behind head on master.

Files with missing lines Patch % Lines
...rontend/src/components/plugins/PluginUIFeature.tsx 83.33% 5 Missing and 2 partials ⚠️
src/frontend/src/hooks/UsePluginUIFeature.tsx 82.60% 4 Missing ⚠️
src/frontend/src/tables/settings/TemplateTable.tsx 78.94% 2 Missing and 2 partials ⚠️
src/backend/InvenTree/plugin/base/ui/api.py 95.23% 2 Missing ⚠️
src/backend/InvenTree/plugin/base/ui/mixins.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8137      +/-   ##
==========================================
+ Coverage   84.12%   84.44%   +0.31%     
==========================================
  Files        1154     1160       +6     
  Lines       51831    52087     +256     
  Branches     1851     1869      +18     
==========================================
+ Hits        43605    43984     +379     
+ Misses       8015     7885     -130     
- Partials      211      218       +7     
Flag Coverage Δ
backend 85.81% <97.95%> (+0.02%) ⬆️
pui 69.48% <85.29%> (+2.24%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@matmair
Copy link
Member

matmair commented Sep 17, 2024

Very interesting, I like the approach @wolflu05!

@wolflu05 wolflu05 marked this pull request as ready for review September 18, 2024 13:54
@wolflu05
Copy link
Contributor Author

This is ready for review now.

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.

LGTM; There seems to be no coverage on usePluginUIFeature, getPluginTemplateEditor, getPluginTemplatePreview and most changes on TemplateTable.tsx reported on codecov - is that an issue with reporting or are these changes not tested?

src/frontend/src/components/plugins/PluginContext.tsx Outdated Show resolved Hide resolved
src/frontend/tests/pui_printing.spec.ts Outdated Show resolved Hide resolved
src/frontend/tests/pui_printing.spec.ts Outdated Show resolved Hide resolved
@wolflu05
Copy link
Contributor Author

There seems to be no coverage on usePluginUIFeature, getPluginTemplateEditor, getPluginTemplatePreview and most changes on TemplateTable.tsx reported on codecov - is that an issue with reporting or are these changes not tested?

@matmair I was wondering about that too, but all of that code should normally be covered by my added playwright test. That's why I actually made the effort to activate the sample plugin and ensure the sample template editor is shown when doing template editing. And also make sure that passing the code back and forth between the editors works. But I have no idea why codecov does not display that. Maybe you can help me here?

@matmair
Copy link
Member

matmair commented Sep 19, 2024

I see the test but Istanbul is not reporting any coverage - maybe the code sections are not reached because of incremental rendering/loading?

@wolflu05
Copy link
Contributor Author

Yeah, the template editors are not rendered on the initial component render, as first the data from the server has to be loaded, but that shouldn't cause any problems, as that's how we do it everywhere? But what's interesting though is if you look at the history of the codecov comment, the previous comment actually hasn't contained the missing tsx files, only the python file.

@wolflu05
Copy link
Contributor Author

wolflu05 commented Sep 19, 2024

@matmair Maybe that's the same problem like mxschmitt/playwright-test-coverage#22, but unfortunately there is no response on that issue . I guess you have used that template to build the coverage system?
Maybe we need to try the official coverage solution: https://playwright.dev/docs/api/class-coverage Seems like that's not a real solution, just some confusing snippet.

@wolflu05
Copy link
Contributor Author

wolflu05 commented Sep 19, 2024

The code that is marked as uncovered is definitely being run:

Code marked as not covered:
image

I have added a throw new Error(...) here:
image

And in the output when running the tests, you can see that this error is raised.
(I have run the tests via: npx playwright test --headed --project chromium -g "PUI - Report Editing")

So this is definitely a problem with the coverage reporting.

@matmair
Copy link
Member

matmair commented Sep 19, 2024

This is indeed very curious; do you think it is realistic that we could fix it ourselves or do we have to just ignore it for now?
I would really like to enforce code coverage rules but that will be hard with this not working

@wolflu05
Copy link
Contributor Author

Not sure, I have no idea what's going on there, I even cannot get coverage to work locally.

@wolflu05
Copy link
Contributor Author

@matmair can we merge this pr now, as the lines are actually be covered by the tests? Then you also have something to test coverage with your new approach.

@matmair
Copy link
Member

matmair commented Sep 22, 2024

I would prefer if we figure this out before, coverage of PUI, in general, is very slacking. We can go ahead and merge but than I would expect an issue to keep track of this.

@wolflu05
Copy link
Contributor Author

As said before, I have absolutely no clue on this, why it's not counted. I have never done frontend coverage in any other project, so I'm no help here. @SchrodingersGat do you have any idea here?

@wolflu05
Copy link
Contributor Author

@SchrodingersGat would you mind taking a review at this, so we can merge this? You can activate the sample ui plugin or install the inventree-template-editor-plugin for testing.

@wolflu05
Copy link
Contributor Author

@SchrodingersGat @matmair the coverage issue has been resolved. It was caused by await page.context().close(); as the last line of the playwright test. I have no idea why this was added. I have just extended the test and didn't keep care what this is doing. I have removed it now, and coverage seems to be reported fine now. So there is nothing in the way anymore.

@SchrodingersGat
Copy link
Member

@wolflu05 thanks for your work here, it is looking very promising! Hopefully we can have some fully integrated plugins working soon to test it out with

@SchrodingersGat SchrodingersGat merged commit 3536234 into inventree:master Sep 26, 2024
26 checks passed
@wolflu05 wolflu05 deleted the pui-feature-plugin branch September 26, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform UI Related to the React based User Interface plugin Plugin ecosystem
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants