Purchase order approvals and "ready" state

[LavissaWoW]

Adds a simple implementation of approvals to Purchase Orders.

Also introduces a "ready" status and ability to limit order issuance to members of a group.

Closes #3501

The following has been added:

4 new settings:

• ENABLE_PURCHASE_ORDER_APPROVAL - Boolean
• PURCHASE_ORDER_APPROVE_ALL_GROUP - Group name
• ENABLE_PURCHASE_ORDER_READY_STATUS - Boolean
• PURCHASE_ORDER_PURCHASER_GROUP - Group name

Purchase Order default state transitions:

Todo-list:

• Implement all notifications
• Verify test coverage
• Finish CUI implementation
• PUI?

[netlify]

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

Name	Link
🔨 Latest commit	d962448
🔍 Latest deploy log	https://app.netlify.com/sites/inventree-web-pui-preview/deploys/66a42b1091ab850008d61175
😎 Deploy Preview	https://deploy-preview-6989--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: 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.

[LavissaWoW]

[LavissaWoW]

@SchrodingersGat Have a look at the comments here.
Any input?

[codecov]

Codecov Report

Attention: Patch coverage is 93.35219% with 47 lines in your changes missing coverage. Please review.

Project coverage is 82.28%. Comparing base (f92d734) to head (d962448).
Report is 225 commits behind head on master.

Files with missing lines	Patch %	Lines
...nd/src/components/buttons/OrderStateTransition.tsx	80.99%	21 Missing and 2 partials ⚠️
src/backend/InvenTree/order/models.py	86.23%	19 Missing ⚠️
src/backend/InvenTree/order/serializers.py	90.69%	4 Missing ⚠️
...venTree/InvenTree/templatetags/inventree_extras.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6989      +/-   ##
==========================================
- Coverage   83.63%   82.28%   -1.35%     
==========================================
  Files        1118     1007     -111     
  Lines       49549    46936    -2613     
  Branches     1625     1040     -585     
==========================================
- Hits        41441    38623    -2818     
- Misses       7680     8163     +483     
+ Partials      428      150     -278     

Flag	Coverage Δ
backend	85.48% <95.84%> (+0.13%)	⬆️
pui	36.12% <82.17%> (-28.35%)	⬇️

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.

[SchrodingersGat]

@LavissaWoW thanks for submitting. @matmair I'd be interested in your thoughts on this, obviously you have put a lot of work into a very similar plugin.

I'd like to see this integrated into the new interface before merging. This will require the "purchase order actions" (submit / cancel / approve / etc) to be added to the PurcahseOrderDetail page.

[LavissaWoW]

@SchrodingersGat working on it 👌

[matmair]

I have a conflict of interest and will not comment or review anything relating to this.

[LavissaWoW]

Some obvious Mantine v7 changes needed. I'm going to be mostly off the next 10 days or so, but I'll try to patch this up.

[LavissaWoW]

Using the PO model property is_complete in a serializer results in an O(n) increase in queries made.
Just if anyone was wondering :>

[LavissaWoW]

@SchrodingersGat Playwright tests seem to change status from run to run, and it's getting to become quite frustrating...
The new tests pass locally, and they passed the QC check on the previous commit.

[LavissaWoW]

Note: It's impossible with our current playwright config to run "global setting"-altering tests.
Even with test.describe.configure({ mode: 'serial' }), Chromium and Firefox tests will run in parallell.
In my ridiculous amount of variations of tests, it's clear that not even global file-scope is shared between workers.
There is literally no way to isolate Firefox and Chromium runs to prevent them from changing settings and messing up each others runtime.

The only option I see here is making them "fight" over setting state, halting test operation until the worker is allowed to reload the page with the correct settings enabled. There is a risk of deadlock with this, but I'm out of options.

[LavissaWoW]

@SchrodingersGat I'm just gonna be blunt: The tests you want require me to write tests that do this:

while(setting_not_set(setting)) {
  sleep(1000)
  try_to_set_setting(setting)
}

await check_some_thing_depending_on_the_setting_hoping_that_the_setting_wasnt_changed_by_the_other_browser()

Unless I'm missing some huge element of Playwright, I'm putting my hair pulling frustrations on hold until I get some feedback.

[SchrodingersGat]

@wolflu05 any thoughts on the tests that @LavissaWoW wants to run here? Maybe there's a way to reset states between test runs?

@LavissaWoW the difference between the frontend tests and backend (django) tests is that the underlying database is reset to a known state before every test run. I'm not sure how we could accomplish a similar thing here

[LavissaWoW]

Running 8 tests using 2 workers

     1 [Chromium] PUI - Pages - Purchasing - Pending transitions
     2 [Firefox] PUI - Pages - Purchasing - Pending transitions

So it's not even considered "between" runs. They run in parallell, and read from the same test DB, at the same time.
I'm at a loss here.

[wolflu05]

Sorry, I have really not much experience with frontend testing. I do understand the problem, but the only thing I could think of is to spin up multiple inventree backend servers on different ports which are used for the different browsers. That way when each browser runs the tests sequentially, there is no race condition. And regarding the data reset after each test, maybe there is some possibility to reset the database to the migrated state after each test by using transactions or save points somehow that are send to the database.

[SchrodingersGat]

@LavissaWoW perhaps if there is sufficient API testing via python unit tests, you can simplify the tests for the frontend, and do a "read only" set of playwright tests?

[LavissaWoW]

@SchrodingersGat That would exclude the setting-based states.
One suggestion I see thrown around online is to split the Playwright testing into two steps where you run non-destructive tests in parallell, then afterwards run destructive tests with only 1 worker. This will force the problem away, but will mean modifying the QC workflow and file structure of the tests.

Basically this:

npx nyc playwright test tests/parallell
npx nyc playwright test tests/sequential --workers=1

It is something to consider doing. Some other tests start failing after I've run the PO tests too many times, as I keep making new POs for every test. So tests looking at the PO index and looking for "Molex Connectors" for example, will fail if enough new POs are created and that PO is rolled to page 2 of the table.

[LavissaWoW]

@SchrodingersGat Aight, here we go. PUI tests are working.

I've done a few things:

1. I've made a new directory src/frontend/tests/destructive.
2. The default playwright.config.ts file now ignores the above directory
3. I made a new playwright-destructive.config.ts that only runs the tests in the directory in pt. 1. These tests are only run with 1 worker. This forces the tests to run one browser at a time.
4. I modified the QC workflow to run the default config first, then run the destructive config.
5. We have a lot of flakyness in our tests, so I increased the retry count in the playwright configs to 2.

Note: Codecov seems to be referencing a whole lot of stuff not related to this PR

[LavissaWoW]

With working tests, I'm considering this ready for a final review

[SchrodingersGat]

@LavissaWoW thanks, I'll hope to assign some time to review this in the next couple of days

[github-actions]

This PR seems stale. Please react to show this is still important.

[LavissaWoW]

@SchrodingersGat Never heard back from you here.
Seems like some parts need a rewrite now tho.