Skip to content

Conversation

webdevinition
Copy link

Hello @jbtronics,

In relation to #1051 the feature for part IPN suggest with category prefixes.
Also mentioned in the discussion under #1041, section 2.

I am grateful for integration!

Best regards,
Marcel

Marcel Diegelmann added 8 commits September 25, 2025 10:31
@jbtronics
Copy link
Member

Some preliminary remarks:

  • Why do you drop the unique constraint on the IPN? I think it is important that the IPNs are truly unique, otherwise they are not useable as identifiers. And as far as I understand the IPN_ENABLE_UNIQUE_CHECK options documentation, an additional suffix is added anyway.
  • Im not sure if the name "enableUniqueCheck" is maybe a bit misleading. Maybe something like "automatically attach suffix or something" might be better.
  • It might be a good addition to have the option to define a global regular expression, that IPNs have to fullfill. that way you can enforce certain formats.
  • The repo method, should not require get a base64 decoded string. That decoding should happen in the controller that handles the request.
  • Is it really useful to generate the IPN based on the part description? Does this gives much advantages about just using the part category?
  • There should be test ensuring the correct behavior of the IPN suggestions and the autocomplete suggestion endpoint.

@jbtronics
Copy link
Member

Also you should fix the complains of phpstan.

Copy link

codecov bot commented Sep 26, 2025

Codecov Report

❌ Patch coverage is 45.12821% with 107 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.65%. Comparing base (e1418df) to head (a2f5329).

Files with missing lines Patch % Lines
src/Repository/PartRepository.php 62.85% 39 Missing ⚠️
...tSubscriber/UserSystem/PartUniqueIpnSubscriber.php 0.00% 31 Missing ⚠️
src/Controller/TypeaheadController.php 7.14% 13 Missing ⚠️
...c/Validator/Constraints/UniquePartIpnValidator.php 33.33% 12 Missing ⚠️
src/Form/AdminPages/CategoryAdminForm.php 0.00% 10 Missing ⚠️
src/Entity/Parts/Category.php 50.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1054      +/-   ##
============================================
- Coverage     57.77%   57.65%   -0.13%     
- Complexity     7043     7106      +63     
============================================
  Files           565      568       +3     
  Lines         23028    23222     +194     
============================================
+ Hits          13305    13389      +84     
- Misses         9723     9833     +110     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@webdevinition
Copy link
Author

@jbtronics, please accept my apologies for not having contacted you about this matter yet.
I haven't forgotten about it, and I will definitely get back to you.

I have also worked on some of the points you mentioned so that I can get as close as possible to what you requested.

@jbtronics
Copy link
Member

No worries, take your time ;)
I will be in vacation next week and won't be able to do anything then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants