Fixes regression for not working embedded widgets #8330
Conversation
sgiehl
added
Major
sgiehl
force-pushed
the
8321_embedded_widgets
branch
from
c4835f0
to
7c99419
Compare
| @@ -323,6 +323,10 @@ private static function forceReloadAuthUsingTokenAuth($tokenAuth) | |||
|
|
|||
| private static function shouldReloadAuthUsingTokenAuth($request) | |||
| { | |||
| if (is_null($request)) { | |||
There was a problem hiding this comment.
Problem is, that $request is null. So the Auth was never reloaded.
I had searched for a better solution. But I didn't find another without changing the current logic.
There seems to be no way to determine whether a parameter is set in the request but empty. Common::getRequestVar() returns the default (or throws exception) if the parameter is missing or empty. Imho returning the empty value would be correct here, but I guess that might break other stuff in piwik to change that behavior.
If you think it would be correct behavior, maybe it's worth trying the correct fix and see if all our tests would still pass? it may be a bit risky but in general it's worthwile trying the "Correct fix approach" in case it may just work :-) And kuddos for this PR! 👍 |
|
Maybe we should not try to do that for a minor fix version.... But I could
try to change that after 2.14.1 was released.
|
|
Makes sense to limit risk for 2.14.1 👍
|
Fixes regression for not working embedded widgets
fixes #8321