Skip to content

Supplier Mixin #9761

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Supplier Mixin #9761

wants to merge 9 commits into from

Conversation

wolflu05
Copy link
Member

@wolflu05 wolflu05 commented Jun 11, 2025

This Adds a supplier mixin so that plugin can provide functions to import data from supplier APIs.

TODO

  • initial import wizard
  • make sure that it works again after closing
  • initial stock creation
  • allow importing only MP and SP
  • check todo comments
  • sample plugin
  • docs
  • tests

Future Ideas

Copy link

netlify bot commented Jun 11, 2025

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

Name Link
🔨 Latest commit 8627e4e
🔍 Latest deploy log https://app.netlify.com/projects/inventree-web-pui-preview/deploys/684b493ff58696000832627a

@wolflu05 wolflu05 added plugin Plugin ecosystem User Interface Related to the frontend / User Interface labels Jun 11, 2025
@wolflu05 wolflu05 added this to the 1.0.0 milestone Jun 11, 2025
@wolflu05 wolflu05 changed the title Suplier Mixin Supplier Mixin Jun 11, 2025
Copy link

codecov bot commented Jun 11, 2025

Codecov Report

Attention: Patch coverage is 87.29282% with 46 lines in your changes missing coverage. Please review.

Project coverage is 86.36%. Comparing base (c705830) to head (8627e4e).
Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
...c/backend/InvenTree/plugin/base/supplier/mixins.py 80.64% 18 Missing ⚠️
src/backend/InvenTree/InvenTree/serializers.py 34.78% 15 Missing ⚠️
src/backend/InvenTree/plugin/base/supplier/api.py 86.04% 12 Missing ⚠️
src/backend/InvenTree/part/api.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9761      +/-   ##
==========================================
+ Coverage   86.33%   86.36%   +0.03%     
==========================================
  Files        1234     1239       +5     
  Lines       54276    54624     +348     
  Branches     2242     2238       -4     
==========================================
+ Hits        46860    47177     +317     
- Misses       6847     6878      +31     
  Partials      569      569              
Flag Coverage Δ
backend 88.25% <87.29%> (-0.01%) ⬇️

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines +114 to +127
// Update pricing when quantity changes
useEffect(() => {
if (quantity === null || quantity === undefined || !pricing) return;

// Find the highest price break that is less than or equal to the quantity
const priceBreak = Object.entries(pricing)
.sort(([a], [b]) => Number.parseInt(b) - Number.parseInt(a))
.find(([br]) => quantity >= Number.parseInt(br));

if (priceBreak) {
setPurchasePrice(priceBreak[1][0]);
setPurchasePriceCurrency(priceBreak[1][1]);
}
}, [pricing, quantity]);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SchrodingersGat do I understand this wrong how this form framework works? I want to hook up the provided price breaks to the purchase price input field. Meaning if I change the quantity, it should auto fill the purchase price from the provided price breaks. I have debugged this issue like 2h now and still cannot figure out why the price does not get updated in the field, even now the field definition updates the value correctly (after the change in ApiForm.tsx). Do you have any idea here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can test this now with the integrated sample plugin very easily. Just go to a part table and open the new import wizard there next to the create new part button. Then search for e.g. M5 and import any bolt. When you are at the stock step, click the plus icon and update the quantity and see that the pricing does not change. However when logging the stockItemFields to the console, the value actually updates.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wolflu05 I'm not setup to test this right now but the code you have written here looks like it should work. This is the approach we use elsewhere...

@wolflu05
Copy link
Member Author

wolflu05 commented Jun 12, 2025

And do you have any idea, why the PUI tests fail, I havent touched them in a while and I do net get anything from the error. There seems to be tests failing where I havent changed anything... I would really appriciate some pointers on how to fix them.

Otherwise this PR is ready for a first review.

@wolflu05 wolflu05 marked this pull request as ready for review June 12, 2025 21:53
@matmair
Copy link
Contributor

matmair commented Jun 12, 2025

without looking at it deeply - there is a lot of coverage on the backend missing

Copy link
Contributor

@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.

Very good quality, as always. A few questions / nitpicks; nothing really blocking.
I think a few lines of user docs on this would be good too - if you have time.

This is a great start, there are a few things that I think could be more sketched out (in future PRs, by anyone):

  • Caching
  • Handling of rate limits
  • oAuth (digikey and a few other have that)
  • parameter normalizing among suppliers
  • data structures to handle multiple supplier accounts

Comment on lines +50 to +80
@dataclass
class SearchResult:
"""Data class to represent a search result from a supplier."""

sku: str
name: str
exact: bool
description: Optional[str] = None
price: Optional[str] = None
link: Optional[str] = None
image_url: Optional[str] = None
id: Optional[str] = None
existing_part: Optional[part_models.Part] = None

def __post_init__(self):
"""Post-initialization to set the ID if not provided."""
if not self.id:
self.id = self.sku

@dataclass
class ImportParameter:
"""Data class to represent a parameter for a part during import."""

name: str
value: str
on_category: Optional[bool] = False
parameter_template: Optional[part_models.PartParameterTemplate] = None

def __post_init__(self):
"""Post-initialization to fetch the parameter template if not provided."""
if not self.parameter_template:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Why are these in the mixin itself and not module context? Is there a need to have separate instances per plugin?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As python classes inside classes, are nothing more special than just having the abillity to call them by SupplierMixin.ImportParameter, I have thought this is just an easy way to expose them via the plugin interface without needing to export a bunch of things. If you dont like that approach, please show me in detail, where I should export them, and make sure, that this export does not break in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way, we have multiples of each of these classes in the server process when more than 1 supplier plugin is used (which I think is the whole idea behind the mixin). Seeing how the plugin mechanism works that might be a problem and make interactions with classes unreliable as any plugin might override them and lead to hard to debug bugs.

I would move them into the module context of this file and export them in the plugin.mixin space, just like BulkNotificationMethod.

make sure, that this export does not break in the future.

I am not the one introducing unstable interfaces that are relied upon like they are stable. If this way breaks on the regular, I know who designed them

Comment on lines +162 to +217
def download_image(self, img_url: str):
"""Download an image from the given URL and return it as a ContentFile."""
img_r = download_image_from_url(img_url)
fmt = img_r.format or 'PNG'
buffer = io.BytesIO()
img_r.save(buffer, format=fmt)

return ContentFile(buffer.getvalue()), fmt

def get_template_part(
self, other_variants: list[part_models.Part], template_kwargs: dict[str, Any]
) -> part_models.Part:
"""Helper function to handle variant parts.

This helper function identifies all roots for the provided 'other_variants' list
- for no root => root part will be created using the 'template_kwargs'
- for one root
- root is a template => return it
- root is no template, create a new template like if there is no root
and assign it to only root that was found and return it
- for multiple roots => error raised
"""
root_set = {v.get_root() for v in other_variants}

# check how much roots for the variant parts exist to identify the parent_part
parent_part = None # part that should be used as parent_part
root_part = None # part that was discovered as root part in root_set
if len(root_set) == 1:
root_part = next(iter(root_set))
if root_part.is_template:
parent_part = root_part

if len(root_set) == 0 or (root_part and not root_part.is_template):
parent_part = part_models.Part.objects.create(**template_kwargs)

if not parent_part:
raise SupplierMixin.PartImportError(
f'A few variant parts from the supplier are already imported, but have different InvenTree variant root parts, try to merge them to the same root variant template part (parts: {", ".join(str(p.pk) for p in other_variants)}).'
)

# assign parent_part to root_part if root_part has no variant of already
if root_part and not root_part.is_template and not root_part.variant_of:
root_part.variant_of = parent_part
root_part.save()

return parent_part

def create_related_parts(
self, part: part_models.Part, related_parts: list[part_models.Part]
):
"""Create relationships between the given part and related parts."""
for p in related_parts:
try:
part_models.PartRelated.objects.create(part_1=part, part_2=p)
except ValidationError:
pass # pass, duplicate relationship detected
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q(nb): do these need to be overrideable or could they be placed in the module scope?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, just thought this might be some helpers to simplify the importing process for plugin developers. Do you have a specific place in mind where I can place them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -1700,6 +1701,23 @@ class PartParameterList(PartParameterAPIMixin, DataExportViewMixin, ListCreateAP
]


class PartParameterBulkCreate(CreateAPIView):
"""Bulk create part parameters.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wolflu05 it might be worth looking into a BulkCreateMixin class here, to make this a generic approach.

I recently added in a BulkUpdateMixin - #9313 - which employs a very similar approach. The intent here is to:

  1. Reduce API calls for better throughput and atomicity
  2. Utilize the already defined serializer classes for back-end validation

So, thoughts? A BulkCreateMixin would complement the BulkUpdateMixin and BulkDeleteMixin classess nicely!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I'll see what I can do and if I need any pointers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you submit that as a separate PR first? I'd like to be able to review that separately


supplier = None
for plugin in registry.with_mixin(PluginMixinEnum.SUPPLIER):
if plugin.slug == supplier_slug:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the "supplier" supposed to be the name of the supplier (e.g. digikey) or the slug for the plugin?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was the idea, at least for the key used here in the api to determine the correct plugin. Maybe later we can display some more pretty name in the UI for it instead of just the plugin name.

color='green'
tooltip={t`Import supplier part`}
onClick={() => importPartWizard.openWizard()}
hidden={!user.hasAddRole(UserRoles.part) || !params?.part}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this button should only be visible if there is an import plugin associated with the linked supplier.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea to hide the button if there is no supplier mixin plugin available.

@wolflu05
Copy link
Member Author

Thanks for that feedback, that are some good ideas. I have collected them in the initial PR description now, but I want to make sure that I full understand them:

parameter normalizing among suppliers

What exactly do you mean by that?

data structures to handle multiple supplier accounts

I think that should be the goal of the SettingsMixin and UIMixin, also with the oauth stuff.

And still, I would really appreciate some help with the PUI tests, do you have any idea @SchrodingersGat ?

@wolflu05
Copy link
Member Author

@matmair Maybe you can add some clarification on my above questions?

@30350n
Copy link
Contributor

30350n commented Jun 24, 2025

parameter normalizing among suppliers

What exactly do you mean by that?

Different supplier parts (even from the same supplier) can feature vastly different formats for the same parameter:

image

This is a whole can of worms and could probably eat up an inifinite amount of time, so I never really looked into this much when developing inventree-part-import. Although I offer the option for users to manually implement things like this as part of their configuration, as I for example did here.

Another thing that relates to this is the Enforce Parameter Units setting. I'm not sure how it works now exactly, but back in the day this would disrupt part creation for any part that failed the "unit check". (This would already include things like the '0.125W, 1/8W' in the above screenshot).

@wolflu05
Copy link
Member Author

If I understand that correct, this is already implemented. You may can test that using the static sample plugin.

@matmair
Copy link
Contributor

matmair commented Jun 25, 2025

What exactly do you mean by that?

https://en.wikipedia.org/wiki/Canonical_form#Computing ensuring that parameters and parameter units from different suppliers are handled in a standardized form

If I understand that correct, this is already implemented. You may can test that using the static sample plugin.

I seem to have overlooked that, could you point that out?

data structures to handle multiple supplier accounts

I think that should be the goal of the SettingsMixin and UIMixin, also with the oauth stuff.

That would mean every plugin implements this itself, probably. In the future, I would prefer to have that as a core feature to ensure that the interface is the same between plugins and to make portability between plugins easier, might they become unmaintained / insecure.

@wolflu05
Copy link
Member Author

I see, have misunderstood this first, no this is not implemented, but a good idea for the future, I have added it to the list. For now, the process looks like this:

out.mp4

@matmair
Copy link
Contributor

matmair commented Jun 27, 2025

Sorry about that, I thought it was unambiguous enough.

This is already great progress as is.

@matmair matmair mentioned this pull request Jul 2, 2025
9 tasks
@SchrodingersGat
Copy link
Member

@wolflu05 sorry I have not yet had sufficient time to properly review this. I would like to push this to 1.1.0 as it requires a lot of testing and I'm keen to get our stable 1.0.0 release out soon.

@SchrodingersGat SchrodingersGat modified the milestones: 1.0.0, 1.1.0 Jul 20, 2025
@wolflu05
Copy link
Member Author

Too bad to hear; this is actually done, it just misses the bulk creation api refactor for part parameter bulk creation. I tried to look into it, but I don't think I have the deep knowledge to implement something generic here. I would be really happy if you could help me there.

And then there are the pui tests that I never really got working and know why they are failing as it's really annoying to push and wait for the ci, because I never have got them to run locally with that new auth cache system..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin Plugin ecosystem User Interface Related to the frontend / User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants