Codecov Report

❌ Patch coverage is 73.55649% with 316 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.35%. Comparing base (8b417d6) to head (66f7629).
⚠️ Report is 7 commits behind head on master.

@@             Coverage Diff              @@
##             master     #982      +/-   ##
============================================
+ Coverage     59.83%   61.35%   +1.51%     
- Complexity     6050     6326     +276     
============================================
  Files           529      542      +13     
  Lines         20625    21730    +1105     
============================================
+ Hits          12341    13332     +991     
- Misses         8284     8398     +114     

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

Here is description of excel functionality pull report

• Allow using xlsx and xls to import parts
• Update example csv to better show what is possible
• Add required tests (PASS)
• Fix parts merger when using update from info supplier to consider parts that already have a spn populated a duplicate

Added a tab for filtering by job statuses and such

Do not merge yet, there is a bug when user doesn't check prefetch and it still fetches info, causing timeout on larger sets.

Add priority support in mapping table

Hey @jbtronics any clue when this could be merged/rejected? Otherwise we will need to continue on our own fork, not providing any contributions here anymore.

I will try to release the V2 version in the next few days, it's basically only missing some documentation. Then I can review and merge the PRs.

As some base stuff changed, it will probably be easier to merge the PRs into the V2 changes than the other way around....

I have started reviewing the PR. I have rebased it on the master branch with v2. As i wasn't sure if I am allowed to force push and overwrite your branch (or if you are using it for something else), there is now a branch with the same name in the main repo. If you have no problem with the force push, just say it, and i will do it (makes some PR features a bit easier i think).

I really like the feature, its nicely done in the WebUI.
I have already changed some few things related to the v2 changes so far and some smaller things related to the excel exports.

I am not finished with the review, but some remarks so far:

• The BulkInfoProviderImportController is really long and complex. I think functionality like the serialization/deserialization can be moved to the entities. Maybe introducing some DTOs instead of the complex arrays and move some of the logic to a service which is independent of the form handling would help too.
• I don't really like the special handling for the batch handling of LCSC. It is not really intrinsically unique to LCSC, and should be more universally available. I already created a new BatchInfoProvider interface, that if implemented make the searchKeywordsBatch method implementable by all info providers. The decision whether to use the batch or individual import should be based on this interface. Not hardcoded for LCSC
• The inline scripts in the templates will most likely not work in productive use, as then the Content-security-policy is active that will block this. Either you need to put it into a js file that will get included, a stimulus controller, or you need to add a nonce to the inline script tag. But i haven't tried out this so far and i don't know how good this work with turbo.

You may force push, I have my fork just to contribute, I have no plans to have own features on it.

• Do you want me to refactor BulkInfoProvider or will you do it in V2?
• Regarding hardcoded LCSC supplier number import I agree, but will BatchInfoProvider interface allow to search by supplier numbers? I haven't really tested with other than LCSC and Mouser as I don't regularly use others, but on them at least searching by MPN on batch input returns a very large number of results, which overloads API or in case of LCSC temporarily bans your ip, because it tries to then fetch datasheets and so on.

Regarding hardcoded LCSC supplier number import I agree, but will BatchInfoProvider interface allow to search by supplier numbers?

I would say that the batch search will just work like the single searches and search for a keyword. What the provider does with that is, is the choice of the provider (and will be limited by the API). Maybe one can pass an optional argument to demark what kind of information this is, to give the provider a hint. That could also be useful to have on the single search

I haven't really tested with other than LCSC and Mouser as I don't regularly use others, but on them at least searching by MPN on batch input returns a very large number of results

Thats why I would implement it as on seperate interface. If it can be useful done for an provider (and somebody wants to implement it for an interface), then the batch methods will be used, otherwise not (and they do not exist on the provider then).

It would probably be the easiest if you could rewrite the controller to the more generic system of bulk info providers. I tried it yesterday and saw no easy way to do so, and would probably need more time to understand how the current code works...