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.

Sign up for GitHub

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

Jump to bottom

EZP-28848: Add relation cache tags to default relation fields #2263

Merged
merged 3 commits into from Mar 6, 2018
Merged

EZP-28848: Add relation cache tags to default relation fields #2263

merged 3 commits into from Mar 6, 2018

Conversation

andrerom
Copy link
Contributor

@andrerom andrerom commented Mar 1, 2018 •
edited

Question Answer
JIRA issue EZP-28848
Bug/Improvement yes
New feature no
Target version 6.13
BC breaks no
Tests pass yes
Doc needed no

Testing note:

  • With ezplatform-http-cache: This will only effectively work with v0.4.2 and higher, on lower versions it will not be part of xkey headers.
  • Without ezplatform-http-cache: This will have no effect, and realtions cache claring is already cleared as part of the deprecated "Smart HTTP cache clearing" system, so no change there.

Review note:

  • This implies a dependency on foshttpcache package until these field types are moved out to own bundles. However it will at least be version independent so we can still contain to move forward on removing http cache implematation from kernel.

TODO:

  • Implement feature / fix a bug.
  • Implement tests.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.
  • Should ideally do this on RichText to, which templates are affected?

@andrerom andrerom changed the base branch from master to 6.13 March 1, 2018 09:11
@andrerom
EZP-28848: Add relation cache tags to default relation fields
85aa894 
Note:
- With ezplatform-http-cache: This will only effectivly work with v0.4.2 and higher, on lower versions it will not be part of xkey headers.
- Without ezplatform-http-cache: This will have no effect, and realtions cache claring is already cleared as part of the deprecated "Smart HTTP cache clearing" system.
@adamwojs
Copy link
Member

adamwojs commented Mar 1, 2018

Should ideally do this on RichText to, which templates are affected?

@andrerom You mean templates for the embedded content/locations fromeZ/Bundle/EzPublishCoreBundle/Resources/views/FieldType/RichText/embed/*.html.twig

alongosz
alongosz approved these changes Mar 2, 2018
View reviewed changes
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andrerom
Copy link
Contributor Author

andrerom commented Mar 2, 2018

@adamwojs looks better now? ;)

@andrerom andrerom requested a review from mnocon March 4, 2018 18:52
@andrerom
Copy link
Contributor Author

andrerom commented Mar 4, 2018

@mnocon Unsure if you tested it yet, but this is expansion on the same patch I shared with you for testing EZP-28848.

@mnocon
Copy link
Member

mnocon commented Mar 5, 2018

@andrerom I've looked at it for 1.13.1 with the patch you provided - if I understand correctly this adds the "default" relations to twigs, so that the patch you provided earlier should no longer be needed, but it should work as previously?

@andrerom
Copy link
Contributor Author

andrerom commented Mar 5, 2018

so that the patch you provided earlier should no longer be needed, but it should work as previously?

yes no longer needed, it's included here. And yes it should work as before, adding relation-<id> tags to the response which in cases like publishing the relation with changes should cause the cache of the relation view to get updated.

andrerom added a commit that referenced this pull request Mar 5, 2018
@andrerom
Revert "[Soft BC] Change SiteAccessMatchListener to make fos_http_cac…
ca7e4ef 
…he.user_context.request_matcher optional"

This reverts commit dd914b6.

Reason: Until field types are moved out of kernel, any of them dealing with relations
will as of #2263 depend on either 1.x or 2,x being installed, so change to composer
is adjusted for this to not remove, but allow usage of 2.x as well.
@andrerom andrerom mentioned this pull request Mar 5, 2018
Merged
6 tasks
mnocon
mnocon approved these changes Mar 6, 2018
View reviewed changes
Copy link
Member

@mnocon mnocon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@andrerom andrerom merged commit 02337fb into 6.13 Mar 6, 2018
@andrerom andrerom deleted the EZP-28848_realtion_tags branch March 6, 2018 14:58
lserwatka pushed a commit that referenced this pull request Mar 7, 2018
@andrerom
[7.1] Remove Foshttpcache code, leaving that to ezplatform-http-cache (…
fa8ea75 
…#2194)

* [BC] Remove HttpCache implementation in Core

* [Soft BC] Change SiteAccessMatchListener to make fos_http_cache.user_context.request_matcher optional

* Revert "[Soft BC] Change SiteAccessMatchListener to make fos_http_cache.user_context.request_matcher optional"

This reverts commit dd914b6.

Reason: Until field types are moved out of kernel, any of them dealing with relations
will as of #2263 depend on either 1.x or 2,x being installed, so change to composer
is adjusted for this to not remove, but allow usage of 2.x as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamwojs adamwojs adamwojs approved these changes

@alongosz alongosz alongosz approved these changes

@mnocon mnocon mnocon approved these changes

Assignees
No one assigned
Labels
Ready for QA
Milestone
No milestone
4 participants
@andrerom @adamwojs @mnocon @alongosz