[!!!][FEATURE] Use DTO for record representation and identity#77
Merged
[!!!][FEATURE] Use DTO for record representation and identity#77
Conversation
Adds two classes, changes params used in *RecordOperation and *RecordRequestHandler, but just sets same properties in RecordOperation classes as before but based on RecordRepresentation and RecordInstanceIdentifier for now.
The functionality had disappeared during refactoring.
It's already set in the fixtures
A condition was missing a bracket, which led to the condition being interpreted with wrong precedence, resulting in PID being set to zero at the wrong time.
Was only accepting `internal_type` as "db", but `allowed` can also indicate a database relation.
mabolek
changed the title
This reverts commit ae68229.
To fix problem with tests in TYPO3 v9.
Contributor
MattiasNilsson
left a comment
MattiasNilsson
left a comment
There was a problem hiding this comment.
Really nice one, make the data structure more readable! I have some small comments.
| $settings['persistence.']['storagePid.'] ?? [] | ||
| ); | ||
|
|
||
| if ($pid === '') { |
Contributor
There was a problem hiding this comment.
Should this also cover the null check not only empty string?
Contributor
Author
There was a problem hiding this comment.
Exchanging this condition with empty($pid).
| $this->table = strtolower($table); | ||
| $this->remoteId = $remoteId; | ||
| $this->workspace = $workspace; | ||
| $this->language = $this->resolveLanguage((string)$language); |
Contributor
There was a problem hiding this comment.
Is there a need to typecast this as it already is a string?
Contributor
Author
There was a problem hiding this comment.
It's a string or null, so by casting null to string we get an empty string. The question would be if this conversion should happen earlier.
Contributor
Author
There was a problem hiding this comment.
I have changed this now.
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
3 participants
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.
This internal change does not affect any external APIs, but improves the way record data and identities are handled. This fixes and issue that made multilingual operations impossible, as well as making it easier to work with record data and identities.
\Pixelant\Interest\Domain\Model\Dto\RecordRepresentationrepresents a record state, and includes data as well as aRecordInstanceIdentifierobject.\Pixelant\Interest\Domain\Model\Dto\RecordInstanceIdentifierrepresents a record's identity: table, UID, language, workspace, and remote ID. It can output both the remote ID as it is seen from the outside (which is only unique when combined with language and workspace) and the internal, so-called "aspected", remote ID, which is unique for each language and workspace combination.