Add global parameter definitions with configurable choice inputs - #1492
Open
LMatt08 wants to merge 6 commits into
Open
Add global parameter definitions with configurable choice inputs#1492LMatt08 wants to merge 6 commits into
LMatt08 wants to merge 6 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds reusable global parameter definitions to Part-DB.
A parameter definition can define:
TextorChoice)ChoiceparametersDefinitions are global and are not dependant of categories.
Parts can link their parameters to a global definition while still keeping
the existing parameter fields as snapshots for compatibility and history.
Legacy / ad-hoc parameters are still supported.
Example:
A part parameter linked to this definition is displayed using the current
definition metadata and use a dropdown for its value.
Main behavior
Global parameter definitions
A new
ParameterDefinitionentity and administration page are available under:Definitions have case-insensitive unique names.
Choicedefinitions store an ordered list of canonical values.Textdefinitions continue to use the existing free-text parameter editor.The goal is to have one global definition for a parameter instead of redefining
again and again the same name/unit/symbol in different places.
Part parameter editor
Existing parameters stay compatible.
When the user selects a known global parameter definition:
definition_idChoicedefinitions use a dropdownTextdefinitions use the existing text inputUnknown parameter names stay normal ad-hoc parameters and do not automatically
create a global definition.
A user allowed to manage parameter definitions can explicitely add a new choice
directly from the Part editor.
The new value is only kept as a pending choice during the form editing.
The global definition is modified only after the complete Part form is valid
and saved.
Server-side validation recheck:
No DB write is done directly from Javascript.
Compatibility and history
Existing
name,symbolandunitfields stay stored on parameters assnapshots.
Current entities use linked definition metadata when available.
Historical / TimeTravel views keep the snapshots and safely fallback to them
if the definition cannot be resolved anymore.
Deleting a definition currently used by parameters is blocked.
This was important to not break the existing data model too much and to keep
compatibility with old parameters.
Advanced parameter search
The Part search can now filter directly using global parameter definitions.
Example:
Multiple parameter filters are combined with AND semantic.
For linked parameters, matching primarily use
definition_id.For old/ad-hoc data, a compatibility fallback match by parameter name only
when
definition IS NULL.This avoid a parameter linked to another definition to accidentally match
through an old snapshot name.
Choice parameters use exact canonical values.
Existing text operators continue to work for Text definitions.
Numeric comparison of global parameters is volontary not included in this PR.
I prefered to keep this PR focused before adding unit-aware numeric search.
Changes by commit
6c03f9f1— Add global parameter definitions with choice inputsIntroduced the first global parameter definition architecture:
ParameterDefinitionentitydefinition_idrelationThe main idea here was to introduce the new model without replacing all the
existing parameter system.
2fc432ac— Improve global choice parameter editingThis commit mainly harden the Part editor and the Choice workflow:
null-> empty string)new_choice_valuepending statePendingParameterChoiceApplierAn important point is that Javascript never modify directly the global
definition. It only prepare the user intention, and the server apply it after
validation.
5dd48ab2— Add global parameter filters to part searchAdded definition-aware advanced Part filtering.
Main additions:
definition_idas the primary identitydefinition IS NULLExample:
Both filters must match the Part.
For Choice definitions the equality is strict/canonical.
For Text definitions the previous text operators are still usable.
b08fa3b6— Fix permission schema upgrade during API requestsThis commit fix an upgrade-time regression exposed by the new permission
schema v5.
Existing Part-DB installations can contain users/groups still using permission
schema v4.
The automatic permission upgrade subscriber was running at
kernel.requestpriority0.At this priority, API Platform already deserialised the new API entity.
The permission subscriber then called:
while the new entity was not yet persisted by API Platform.
This could produce errors like:
The real listener order was:
The subscriber is now running at priority
6:So the subscriber still have access to the authenticated user, but execute
before API Platform starts loading/deserializing request entities.
No Doctrine cascade was added and API Platform persistence behavior was not
changed.
A regression test now explicitely:
parameter_definitionspermissionsPOST /api/categoriesparameter_definitionspermission values were correctly migratedThis reproduce the real case of an existing Part-DB installation upgraded
from permission schema v4 to v5.
Permission schema v5
The new
parameter_definitionspermission set is migrated this way:Management operations:
are inherited from:
So users already allowed to read parts can read the global definitions.
Users who could modify system configuration keep the rights needed to manage
them.
Validation
A lot of automated tests were added or extended during the different
checkpoints.
The tests cover in particular:
After the permission-upgrade fix:
On a clean SQLite test DB:
The complete local PHPUnit suite was also ran:
The remaining local failures are only OAuth tests because the local Windows /
OpenSSL setup is currently not able to generate the RSA OAuth test keys.
CI generates these keys before executing the tests, so these failures are not
related to this PR.
PHPStan targeted checks are clean.
git diff --checkis clean.Frontend assets builds correctly.
Backward compatibility
The implementation try to keep the old parameter behavior as much as possible.
v4 to v5
The
definition_idrelation is nullable so old data stay valid.Out of scope
Some things are intentionally not included in this PR to keep the scope
reasonable:
Numeric filtering is probably the next logical evolution, but I prefered to
not mix it with this first global parameter definition implementation.