inventree / InvenTree

Open Source Inventory Management System
https://docs.inventree.org
MIT License
4.34k stars 786 forks source link

Add PO wildcard default setting #8532

Closed jacobfelknor closed 12 hours ago

jacobfelknor commented 1 day ago

This PR is definitely a work in progress, but I wanted to float this idea because it addresses one of the complaints I get about my InvenTree instance.

At my company, purchase orders may be prepended with a P or a T depending on the situation. For example, a reference may be P-12345 or T-12345. As a result, my PO reference pattern is ?-{ref:05d}.

However, a majority of POs get the P version of the reference, and the create PO form suggests the literal ? from the reference, like ?-12345. This means users must replace the ? with a P in a majority of cases, which is annoying. If I change the reference to P-{ref:05d}, the T version is no longer accepted (which is correct and expected).

I propose adding a setting PURCHASEORDER_REFERENCE_PATTERN_WILDCARD_DEFAULT (and similar for other types of references) which suggests in the form a default value for the wildcard ?, but continues to accept other characters in this position. If the setting is set to None, no replacement happens and the form continues to suggest ?.

As I mentioned, this PR is incomplete in its current state and does not consider tests or other objects that use reference patterns. I also only implemented setting the value in the CUI because I have no familiarity with developing in the PUI yet (but will if this becomes real). This is more to gather feedback on the idea before I commit more time to it,

netlify[bot] commented 1 day ago

Deploy Preview for inventree-web-pui-preview canceled.

Name Link
Latest commit b1fc8dcd5b7306eacb8ccd679f7aab7bfef4d63d
Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/673f97dedc0bbd0008c540aa
wolflu05 commented 1 day ago

Just an idea/question for someone who is more familiar with the reference syntax. I guess if this is a custom implementation, maybe we could add some kind of ?{default:P}-{ref:05d} syntax @SchrodingersGat ?

jacobfelknor commented 1 day ago

Just an idea/question for someone who is more familiar with the reference syntax. I guess if this is a custom implementation, maybe we could add some kind of ?{default:P}-{ref:05d} syntax @SchrodingersGat ?

That does feel cleaner... especially because it probably means we can do it all in one place instead of a setting per object type

SchrodingersGat commented 1 day ago

I like the suggestion by @wolflu05 too. @jacobfelknor do you think you can rework the PR to use that approach? This way is generic and automatically rolls out against all other reference types

wolflu05 commented 1 day ago

I mean the syntax is still TBD. I'd really like to hear your ideas here, as I'm not so familiar with the ref syntax, never did something there.

jacobfelknor commented 1 day ago

Yes, I will explore it

jacobfelknor commented 16 hours ago

@wolflu05 @SchrodingersGat

This ended up being quite elegant in my opinion. Using the reference pattern {?:P}-{ref:05d} I'm able to accomplish what I wanted for POs, and this syntax works for other reference patterns as well

I still need to update tests/docs

codecov[bot] commented 15 hours ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 84.24%. Comparing base (13440a6) to head (b1fc8dc). Report is 3 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #8532 +/- ## ========================================== - Coverage 84.61% 84.24% -0.38% ========================================== Files 1178 1178 Lines 53584 54252 +668 Branches 2026 2026 ========================================== + Hits 45340 45704 +364 - Misses 7722 8026 +304 Partials 522 522 ``` | [Flag](https://app.codecov.io/gh/inventree/InvenTree/pull/8532/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inventree) | Coverage Ξ” | | |---|---|---| | [backend](https://app.codecov.io/gh/inventree/InvenTree/pull/8532/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inventree) | `85.95% <100.00%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inventree#carryforward-flags-in-the-pull-request-comment) to find out more.

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


🚨 Try these New Features:

jacobfelknor commented 14 hours ago

Ok, added to docs/tests. I wasn't sure exactly where to put the test, but I put it where it made most sense to me. Let me know if there's anything else I should do

SchrodingersGat commented 12 hours ago

@jacobfelknor nice work, this is very clean indeed. The tests look appropriate, too.

Thanks for the great contribution!