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

[Task]: Fix regression Newsletter news #456

Merged
merged 3 commits into from
Apr 11, 2023
Merged

[Task]: Fix regression Newsletter news #456

merged 3 commits into from
Apr 11, 2023

Conversation

kingjia90
Copy link
Contributor

@kingjia90 kingjia90 commented Apr 6, 2023

Follow-up #389

In the current PR, i am fixing the case i've encountered by reverting that change.
A TODO would be re-review #389 and see if all the other changes are affecting too

It seems that the editable would work when set to $request->query instead of $request->attributes but breaks the email (by looking at email sent) and viceversa.

image
image
image

@kingjia90 kingjia90 added this to the 11.0.0 milestone Apr 6, 2023
@kingjia90 kingjia90 added the Bug label Apr 6, 2023
@mcop1 mcop1 self-assigned this Apr 7, 2023
@mcop1 mcop1 merged commit 8b8e43c into 11.x Apr 11, 2023
@mcop1 mcop1 deleted the kingjia90-patch-1 branch April 11, 2023 06:24
@blankse
Copy link
Contributor

blankse commented Apr 11, 2023

@kingjia90 @mcop1 The $request->get() method is internal, see pimcore/pimcore#14138
Usage of News::getById() method with a numeric strings param is deprecated, see pimcore/pimcore#13985

@dvesh3
Copy link
Contributor

dvesh3 commented Apr 11, 2023

@blankse we'll cover this in #405. thanks for the notice anyway :)

@aryaantony92 aryaantony92 mentioned this pull request Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants