Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make FieldDefinitionIdentifier matcher actually test the field identifier #42

Merged
merged 1 commit into from Jun 18, 2020

Conversation

bdunogier
Copy link
Member

Question Answer
JIRA issue EZP-31677
Type bug
Target eZ Platform version v2.5, v3.0, v3.1
BC breaks no
Tests pass yes
Doc needed yes

Changes FieldDefinitionIdentifier matcher to actually test that the matched view is for the configured field definition identifier. To make it possible, the view must include a fieldIdentifier parameter set to the field being rendered.

Checklist:

  • PR description is updated.
  • Added code follows Coding Standards (use $ composer fix-cs).
  • PR is ready for a review.

@bdunogier bdunogier requested a review from adamwojs June 8, 2020 14:43
@bdunogier bdunogier changed the title Fix EZP-31677: make FieldDefinitionIdentifier matcher test the field identifier Make FieldDefinitionIdentifier matcher actually test the field identifier Jun 9, 2020
@bdunogier bdunogier force-pushed the ezp31677-fix_field_definition_identifier_matcher branch from df2fb41 to 8422492 Compare June 9, 2020 07:22
@bdunogier bdunogier requested a review from alongosz June 9, 2020 12:10
@bdunogier
Copy link
Member Author

Ping @adamwojs & @alongosz. Fairly simple but important one.

Andrew, we still don't have a way to use a string literal (e.g. Identifier\FieldDefinition) instead of the service's id, do we ?

@alongosz
Copy link
Member

Andrew, we still don't have a way to use a string literal (e.g. Identifier\FieldDefinition) instead of the service's id, do we ?

Nope, EZP-31568 is still to be scheduled.

@bdunogier
Copy link
Member Author

Okay thank you. Not critical, but it would still be a very nice DX improvement, especially with that field...

Or I can reintroduce that hack that modifies the view config to replace it with the service's id 😈

spec/eZ/ContentView/QueryResultsInjectorSpec.php Outdated Show resolved Hide resolved
src/eZ/ContentView/FieldDefinitionIdentifierMatcher.php Outdated Show resolved Hide resolved
src/eZ/ContentView/QueryResultsInjector.php Outdated Show resolved Hide resolved
@bdunogier bdunogier force-pushed the ezp31677-fix_field_definition_identifier_matcher branch from 868558d to 6bd0c8f Compare June 18, 2020 09:19
@bdunogier
Copy link
Member Author

Rebased, should be ready to merge.

@bdunogier bdunogier force-pushed the ezp31677-fix_field_definition_identifier_matcher branch from 445bfdb to 2f8848b Compare June 18, 2020 10:15
@lserwatka lserwatka merged commit 456bae4 into 1.0 Jun 18, 2020
@lserwatka lserwatka deleted the ezp31677-fix_field_definition_identifier_matcher branch June 18, 2020 12:36
@lserwatka
Copy link
Member

@bdunogier you can merge it up. We will do QA as part of RC1

@bdunogier
Copy link
Member Author

Yes, I'm on it.

@bdunogier
Copy link
Member Author

Merged to 2.0, 2.1 and master.

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