Closed foundkey closed 6 months ago
I think this makes sense, but I'm not 100% sure if you're just fixing an issue with a missing variable or if the functionality of loading panel contents with RENDER_PANELS = True
and older requests was broken previously.
If it's the former I'm good with the PR, but if it's the later we'd really want a test to make sure we're not regressing here.
I think this makes sense, but I'm not 100% sure if you're just fixing an issue with a missing variable or if the functionality of loading panel contents with
RENDER_PANELS = True
and older requests was broken previously.If it's the former I'm good with the PR, but if it's the later we'd really want a test to make sure we're not regressing here.
My local test was fine. After modification, no exception logs can be seen in the same scenario. However, I only tested the scenarios I encountered, and I did not conduct a comprehensive test.
Thanks for identifying a solution! Would you be able to include a test that confirms that this is fixed? It's hard to accept a change without being able to see what it's fixing.
Sorry, this test doesn't look easy to add.
According to the Django Template rendering process, the exception django.template.base.VariableDoesNotExist
is handled gracefully. This exception is not thrown and is replaced with a default value, which is an empty string if not configured.
This results in an empty string being obtained at the exception and going to the false branch, which is in line with the expected logic.
According to debug-toolbar
's code:
if panel_content.html
is rendered under a non-ajax request, there will be a toolbar variable in the context, which is normal.
If panel_content.html
is rendered under an ajax request, the toolbar variable is missing from the context, eventually leading to the false branch. In ajax request, toolbar.should_render_panels
value is always false.
It seems that this exception does not trigger any problems. Only in debug level logs, you will see relevant error logs. To sum up, it won't hurt if this anomaly is not fixed.
After turning on Debug level logging, VariableDoesNotExist errors appear frequently during use. But it does not affect the use. I think my PR is redundant. It can be closed.
Just curious why you closed this? I don't think your PR was redundant at all. Error should not be thrown unless there is an error.
I just spent half my day trying to figure out this issue, luckily I stumbled on your PR.
@Riskcomplexx would you be able to open a PR with the fix and a test to prevent a regression?
Just curious why you closed this? I don't think your PR was redundant at all. Error should not be thrown unless there is an error.
I just spent half my day trying to figure out this issue, luckily I stumbled on your PR.
@Riskcomplexx
During the subsequent debugging process, I found that this exception did not affect the use. And there is already code to handle this exception. You can refer to the analysis in my comment above.
In addition, the test for this PR is not easy to add. I don’t have much time to complete this test, so I can only close this PR.
Description
When sending a request using ajax, toolbar.js will send a request at the same time:
GET /__debug__/history_sidebar/?store_id=<id> HTTP/1.1
This request will throw the following exception when rendering the template. It seems because toolbar is not passed in panel_context.
Fixes # (#1905 )
toolbar
intopanel_context
.Checklist:
docs/changes.rst
.