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

EZP-31558: Updated loadByIdentifier to use escapeForCacheKey function before get value from cache #3018

Merged
merged 6 commits into from Apr 20, 2020

Conversation

mateuszdebinski
Copy link
Contributor

@mateuszdebinski mateuszdebinski commented Apr 14, 2020

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

TODO:

  • Implement feature / fix a bug.
  • Implement tests.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@mateuszdebinski mateuszdebinski changed the title [WIP]EZP-31558: Updated loadByIdentifier to use escapeForCacheKey function before get value from cache EZP-31558: Updated loadByIdentifier to use escapeForCacheKey function before get value from cache Apr 14, 2020
Copy link
Member

@konradoboza konradoboza left a comment

Choose a reason for hiding this comment

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

It would be good to verify, whether escaping is necessary in some other methods, e.g. loadGroupByIdentifier and handlers (if needed).

@mateuszdebinski
Copy link
Contributor Author

It would be good to verify, whether escaping is necessary in some other methods, e.g. loadGroupByIdentifier and handlers (if needed).

I will check and if necessary I make changes, for now, I change to WIP

@mateuszdebinski mateuszdebinski changed the title EZP-31558: Updated loadByIdentifier to use escapeForCacheKey function before get value from cache [WIP]EZP-31558: Updated loadByIdentifier to use escapeForCacheKey function before get value from cache Apr 14, 2020
@andrerom
Copy link
Contributor

andrerom commented Apr 14, 2020

@mateuszdebinski You may go ahead an add this everywhere where we load by identifier or remote id I think, if you are up for it, bette be safe then sorry.

@mateuszdebinski mateuszdebinski changed the title [WIP]EZP-31558: Updated loadByIdentifier to use escapeForCacheKey function before get value from cache EZP-31558: Updated loadByIdentifier to use escapeForCacheKey function before get value from cache Apr 15, 2020
Copy link
Member

@konradoboza konradoboza left a comment

Choose a reason for hiding this comment

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

After internal sync, it looks good to me.

@tomaszszopinski tomaszszopinski self-assigned this Apr 20, 2020
Copy link

@tomaszszopinski tomaszszopinski left a comment

Choose a reason for hiding this comment

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

QA approved on eZPlatform 2.5.9 with diff.

@lserwatka lserwatka merged commit bfacdc3 into 7.5 Apr 20, 2020
@lserwatka lserwatka deleted the EZP-31558 branch April 20, 2020 10:26
@lserwatka
Copy link
Member

You can merge it up to ezplatform-kernel/1.0 and master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants