Skip to content

show namespaces alongside url_name in request panel #1442

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

Merged
merged 3 commits into from
Jan 23, 2021

Conversation

AlexCLeduc
Copy link
Contributor

As far as I know, no one really asked for this, but I work on projects that make heavy use of namespaces with repetitive urls (e.g. create, edit, index) and having namespaces available in the request panel helps disambiguate and makes for a quicker grep.

Before this PR:
Screen Shot 2021-01-23 at 11 08 42 AM

After this PR:
Screen Shot 2021-01-23 at 11 09 20 AM

I used : as a separator because that's what you'd pass to e.g. reverse("admin:login"). We can alternatively add a separate 'namespace' title/content section, but this seemed cleaner to me

@AlexCLeduc AlexCLeduc marked this pull request as ready for review January 23, 2021 16:56
@matthiask
Copy link
Member

Thanks, this is great. I think including namespaces directly in the URL name is fine.

However, the new test seems to fail quite often. @AlexCLeduc Do you have an idea why?

@AlexCLeduc AlexCLeduc marked this pull request as draft January 23, 2021 17:16
@codecov
Copy link

codecov bot commented Jan 23, 2021

Codecov Report

Merging #1442 (39bc38b) into master (ea65261) will increase coverage by 0.56%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1442      +/-   ##
==========================================
+ Coverage   87.44%   88.01%   +0.56%     
==========================================
  Files          29       29              
  Lines        1577     1593      +16     
  Branches      220      223       +3     
==========================================
+ Hits         1379     1402      +23     
+ Misses        146      141       -5     
+ Partials       52       50       -2     
Impacted Files Coverage Δ
debug_toolbar/panels/request.py 100.00% <100.00%> (ø)
debug_toolbar/panels/sql/utils.py 92.15% <0.00%> (+0.32%) ⬆️
debug_toolbar/panels/cache.py 84.61% <0.00%> (+1.03%) ⬆️
debug_toolbar/panels/sql/views.py 90.62% <0.00%> (+3.12%) ⬆️
debug_toolbar/panels/profiling.py 91.07% <0.00%> (+4.46%) ⬆️

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 ea65261...39bc38b. Read the comment docs.

@AlexCLeduc
Copy link
Contributor Author

@matthiask Sorry about that, I had trouble running tests locally and figured CI was misconfigured. Turns out the test app never included the admin URLs, looks like it works now.

@AlexCLeduc AlexCLeduc marked this pull request as ready for review January 23, 2021 17:28
@tim-schilling
Copy link
Member

@AlexCLeduc If possible, could you move this test to the RequestPanelTestCase test case in tests/panels/test_request.py? If we don't need it in the integration test case, I'd like to avoid that.

@AlexCLeduc AlexCLeduc force-pushed the request-panel-namespaces branch from eb30b0f to 39bc38b Compare January 23, 2021 17:51
@matthiask matthiask merged commit 800e61e into django-commons:master Jan 23, 2021
@matthiask
Copy link
Member

Thank you! 💪🎉

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.

3 participants