Skip to content

show namespaces alongside url_name in request panel - #1442

Merged
matthiask merged 3 commits into
django-commons:masterfrom
AlexCLeduc:request-panel-namespaces
Jan 23, 2021
Merged

show namespaces alongside url_name in request panel#1442
matthiask merged 3 commits into
django-commons:masterfrom
AlexCLeduc:request-panel-namespaces

Conversation

@AlexCLeduc

Copy link
Copy Markdown
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
Copy Markdown
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

codecov Bot commented Jan 23, 2021

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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