Skip to content

Conversation

@webdevinition
Copy link
Contributor

Hello @jbtronics,

In relation to #1051 the feature for custom part states.
Also mentioned in the discussion under #1041, section 3.

I am grateful for integration!

Best regards,
Marcel

@jbtronics
Copy link
Member

My preliminary remarks from a quick look at the PR:

  • What is the advantage of having the custom states as its own entity over just putting that into a tag? (Or just an additional text field in the part)?
  • The two migrations should probably be merged. I dont see much reason to have it split in two.
  • You should add tests, to ensure the admin pages are available and everything behaves like intended
  • Something has broken the existing tests.

@webdevinition
Copy link
Contributor Author

@jbtronics, please excuse me for not getting back to you on this matter yet.
It has not been forgotten, and feedback will follow in any case.

@webdevinition webdevinition force-pushed the feature/custom-part-status branch from e88c354 to 6cea41b Compare October 15, 2025 06:42
@webdevinition
Copy link
Contributor Author

Hello @jbtronics, thank you for your patience.

Regarding your comments:

What is the advantage of having the custom states as its own entity over just putting that into a tag? (Or just an additional text field in the part)?

In everyday use, it has proven useful to be able to use functions similar to those of the other entities, such as uploading descriptive attachments or hierarchically storing desired status types. The possibility of storing notes is also not to be overlooked. This allows the administrator to specify at the time of creation exactly when the status should be used in everyday life and why it was ultimately created.

The two migrations should probably be merged. I dont see much reason to have it split in two.

That's done.

You should add tests, to ensure the admin pages are available and everything behaves like intended

Now there are also corresponding tests which I have added analogously to other entities.

Something has broken the existing tests.

I recently made an adjustment, which may have caused the problem.

Thanks for everything so far! I've pushed the additional adjustments.

@jbtronics
Copy link
Member

jbtronics commented Oct 26, 2025

I wonder if it would be useful that a part can have more than one custom state. Maybe in combination with a simple constraint that ensures that only one status out of a single parent type is selected. That way the system is a bit more universal, and would allow for different kind of states...

I have also fixed the test issues and some translation inconsistency (for every other entity the plural is used in the admin page title and tree node. I just changed it for english as in german this would lead to some weird words). However i cannot push the changes, as it tells me "Permission denied".

Besides these points, i would say the PR is ready for merge.

@webdevinition webdevinition force-pushed the feature/custom-part-status branch from 6cea41b to b4130f2 Compare October 27, 2025 07:43
@webdevinition
Copy link
Contributor Author

webdevinition commented Oct 27, 2025

@jbtronics, thank you very much for your efforts and preparation! I've checked the "Allow maintainer changes" box in the pull request.

We haven't had to set multiple statuses in practice yet, but there might be other Part-DB users who would welcome something like that.

I am very pleased about the trusting cooperation, which I enjoy, and your comments and improvements :)

Best regards,
Marcel

@codecov
Copy link

codecov bot commented Oct 27, 2025

Codecov Report

❌ Patch coverage is 63.00000% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.62%. Comparing base (600686c) to head (c8827cc).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...stem/PartKeeprImporter/PKDatastructureImporter.php 0.00% 11 Missing ⚠️
src/Repository/Parts/PartCustomStateRepository.php 37.50% 5 Missing ⚠️
...ontroller/AdminPages/PartCustomStateController.php 60.00% 4 Missing ⚠️
src/Command/Migrations/ImportPartKeeprCommand.php 0.00% 3 Missing ⚠️
src/DataTables/PartsDataTable.php 80.00% 3 Missing ⚠️
src/Entity/LogSystem/CollectionElementDeleted.php 0.00% 2 Missing ⚠️
src/Security/Voter/ParameterVoter.php 0.00% 2 Missing ⚠️
src/Services/EDA/KiCadHelper.php 33.33% 2 Missing ⚠️
...rc/Services/UserSystem/PermissionPresetsHelper.php 0.00% 2 Missing ⚠️
src/DataFixtures/DataStructureFixtures.php 0.00% 1 Missing ⚠️
... and 2 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1053      +/-   ##
============================================
+ Coverage     57.82%   58.62%   +0.79%     
- Complexity     7068     7094      +26     
============================================
  Files           568      571       +3     
  Lines         23086    22673     -413     
============================================
- Hits          13350    13291      -59     
+ Misses         9736     9382     -354     

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

@jbtronics jbtronics merged commit 14a4f1f into Part-DB:master Oct 27, 2025
16 checks passed
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