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

Add HistoryPanel to capture ajax requests. #1250

Merged
merged 4 commits into from
Aug 22, 2020

Conversation

tim-schilling
Copy link
Member

@tim-schilling tim-schilling commented Mar 20, 2020

This creates a new panel, HistoryPanel which makes use of the
Toolbar.store to support a history of the toolbar as requests are
made.

The interface changes as follows:
Panel.is_historical - Indicates that the panel's button and content
should be updated when switching between snapshots of the history.

Toolbar.store - Will no longer generate a new store_id when the
instance already has a value.

DEBUG_TOOLBAR_CONFIG.HISTORY_POST_TRUNC_LENGTH - Allows the request's
POST content to be truncated in the history panel's content.

LoggingPanel and StaticFilesPanel now utilize the get_stats method
to fetch panel data for nav_subtitle.

Credit to @djsutho for creating the original third party panel:
https://github.com/djsutho/django-debug-toolbar-request-history
The core concepts were derived from that package.

Closes #1066, #1032, #998.

@tim-schilling tim-schilling changed the title Support HistoryPanel to capture ajax requests. Add HistoryPanel to capture ajax requests. Mar 20, 2020
@codecov
Copy link

codecov bot commented Mar 21, 2020

Codecov Report

Merging #1250 into master will increase coverage by 0.02%.
The diff coverage is 91.48%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1250      +/-   ##
==========================================
+ Coverage   86.68%   86.71%   +0.02%     
==========================================
  Files          25       29       +4     
  Lines        1442     1528      +86     
  Branches      207      215       +8     
==========================================
+ Hits         1250     1325      +75     
- Misses        140      148       +8     
- Partials       52       55       +3     
Impacted Files Coverage Δ
debug_toolbar/toolbar.py 85.71% <0.00%> (-5.75%) ⬇️
debug_toolbar/panels/history/views.py 76.19% <76.19%> (ø)
debug_toolbar/panels/history/panel.py 97.29% <97.29%> (ø)
debug_toolbar/middleware.py 93.65% <100.00%> (+3.17%) ⬆️
debug_toolbar/panels/history/__init__.py 100.00% <100.00%> (ø)
debug_toolbar/panels/history/forms.py 100.00% <100.00%> (ø)
debug_toolbar/panels/logging.py 95.91% <100.00%> (ø)
debug_toolbar/panels/staticfiles.py 94.89% <100.00%> (+0.05%) ⬆️
debug_toolbar/panels/profiling.py 89.28% <0.00%> (-1.79%) ⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bad48b2...fa6984b. Read the comment docs.

Copy link
Member

@matthiask matthiask left a comment

Choose a reason for hiding this comment

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

I didn't try it out but went through the diff. Looks good!

debug_toolbar/panels/history/panel.py Outdated Show resolved Hide resolved
debug_toolbar/settings.py Outdated Show resolved Hide resolved
@tim-schilling
Copy link
Member Author

If we think this is going to be merged, we'll need to do a major version bump. But before we do that, I'll want to go through the third party panels identified in the docs to at least identify if any will break.

@matthiask
Copy link
Member

@tim-schilling Compatibility problems only really surface after a release, unfortunately. I think it would be alright to merge this and go ahead with it, maybe with an alpha/beta release (similar to 2.0a1) so that it is installable using pip --pre.

But I certainly wouldn't want to push this without your approval or if there is anything you'd still like to check before.

@tim-schilling
Copy link
Member Author

I have not forgotten about this, I've been busy with paid work and haven't had time for FOSS development. I will get back around to this though.

This creates a new panel, HistoryPanel which makes use of the
Toolbar.store to support a history of the toolbar as requests are
made.

The interface changes as follows:
Panel.is_historical - Indicates that the panel's button and content
should be updated when switching between snapshots of the history.

Toolbar.store - Will no longer generate a new store_id when the
instance already has a value.

DEBUG_TOOLBAR_CONFIG.HISTORY_POST_TRUNC_LENGTH - Allows the request's
POST content to be truncated in the history panel's content.

LoggingPanel and StaticFilesPanel now utilize the ``get_stats`` method
to fetch panel data for nav_subtitle.

Credit to @djsutho for creating the original third party panel:
https://github.com/djsutho/django-debug-toolbar-request-history
The core concepts were derived from that package.
@tim-schilling
Copy link
Member Author

@matthiask This requires another review due to me making a few changes:

  • Rebasing on the new main branch.
  • Displaying the request variables as a table similar to the cache panel rather than a JSON strong.
  • Supporting a refresh button within the history panel. This allows the user to load up the debug toolbar info for requests that have been performed since the original request
  • Increased RESULTS_CACHE_SIZE to 25 (from 10). The reason for this is because the original request's data was being dropped before I opened the toolbar during my testing. With ajax-y applications, I feel like this new setting better reflects the number of ajax requests between full requests.

Copy link
Member

@matthiask matthiask left a comment

Choose a reason for hiding this comment

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

This looks very good already, thank you!

docs/panels.rst Outdated
History
~~~~~~~

Path: ``debug_toolbar.panels.history.VersionsPanel``
Copy link
Member

Choose a reason for hiding this comment

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

Typo? (VersionsPanel)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, copy paste error. Thanks for catching it.

sending to less secure functions.
Not comprehensive

Taken from django.contrib.auth.__init__.credentials
Copy link
Member

Choose a reason for hiding this comment

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

I was hoping there would be a way to use https://docs.djangoproject.com/en/3.1/ref/settings/#default-exception-reporter-filter for this. I didn't even know there was a setting for this, I assumed it was internal-only. I'd have referenced the setting earlier if I was aware that it existed at all.

(Also, maybe Django should use the SafeExceptionReporterFilter instead of reimplementing parts of it in django.contrib.auth.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, good call. I'll rework this to make use of that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm thinking I need to remove this. The django implementation only uses it for debug cases which defeats the purpose of this.

@matthiask matthiask merged commit ab08d14 into django-commons:master Aug 22, 2020
@tim-schilling
Copy link
Member Author

@matthiask I'm curious about your v3.0 release plan. Would this be included or would it be smarter to do a 3.0 and then a 3.1.0a immediately after? I can make a post on forum.djangoproject.com asking for alpha testers.

@matthiask
Copy link
Member

@tim-schilling I think it might be worth it to do another alpha. Generally I don't worry too much about stability since mostly people aren't supposed to use django-debug-toolbar in production :-)

It's enough work to create a release already, and I'd rather speed the release process up a bit.

@tim-schilling
Copy link
Member Author

Sounds good. Let me know where I can help out.

@matthiask
Copy link
Member

@tim-schilling I have just pushed 3.0a3 now, if you want to add a post in the forum that would be awesome. Thanks!

@tim-schilling
Copy link
Member Author

@matthiask Thanks for the update! I'll do that this morning.

@matthiask
Copy link
Member

matthiask commented Sep 8, 2020

@tim-schilling It may be worth it uploading another release; I fixed a crash which happened when using request as a file-like object (see 3.0a3...master). What do you think? Maybe it's an edge case....

@tim-schilling
Copy link
Member Author

No, I doubt it's an edge-case if it was found that quickly. I agree it's worth to bump the version since it was caught and fixed.

@matthiask
Copy link
Member

Alright, https://pypi.org/project/django-debug-toolbar/3.0a4/ is available on PyPI.

@tim-schilling
Copy link
Member Author

Thanks, here's the forum post https://forum.djangoproject.com/t/django-debug-toolbar-alpha-release-v3/4250

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better support for DRF browsable API out of the box
2 participants