Enhancement for detected path parameter name when using lookup_url_kwarg

[spookylukey]

Follow up from #509 (comment)

[codecov]

Codecov Report

Merging #524 (347059a) into master (cd97613) will increase coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #524      +/-   ##
==========================================
+ Coverage   98.62%   98.74%   +0.11%     
==========================================
  Files          58       58              
  Lines        5951     6352     +401     
==========================================
+ Hits         5869     6272     +403     
+ Misses         82       80       -2     

Impacted Files	Coverage Δ
drf_spectacular/openapi.py	97.30% <100.00%> (+0.35%)	⬆️
tests/test_regressions.py	100.00% <100.00%> (ø)
tests/contrib/test_drf_jwt.py	100.00% <0.00%> (ø)
drf_spectacular/authentication.py	100.00% <0.00%> (ø)
drf_spectacular/contrib/rest_framework_jwt.py	100.00% <0.00%> (ø)
drf_spectacular/contrib/django_filters.py	86.79% <0.00%> (+0.25%)	⬆️
...rf_spectacular/contrib/rest_framework_simplejwt.py	92.30% <0.00%> (+0.64%)	⬆️
drf_spectacular/plumbing.py	98.47% <0.00%> (+0.91%)	⬆️

---

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 cd97613...347059a. Read the comment docs.

[tfranzel]

hey @spookylukey, does this change really make a difference for you? i took your test change without the change to _resolve_path_parameters and it already worked like it should. is there something i'm missing? variable is sourced from the (generated) schema path, so they should always match no matter what.

also fyi: i still need to adapt the _pk replacement as it lead to inconistent urls: #516 (comment) . maybe another setting. not 100% sure yet.

[spookylukey]

I'm not bothered about this PR getting merged at all, it was just a reply to your previous comments.

The only question is whether lookup_url_kwarg is expected to change the OpenAPI schema or not, and whether we could use lookup_url_kwarg to provide something better - and something that isn't effectively exposing our database schema. If I'm using foo__bar__id as a Django lookup field, and that string therefore appears in my API, if I change my database schema so that I can use a simpler lookup, it's unfortunate that my API schema has to change.

That said, I'm probably going to address this in my case using some post processing, which will be more reliable for my case.

[tfranzel]

hey @spookylukey, does this change really make a difference for you?

sry, i should have formulated that more clearly. i was trying to say that your change always has exactly the same outcome as compared to the current version. it's basically rearranging the variable, but the code does exactly the same. it took me a second to realize that.

because of that your added test case works with and without your "variable" change obviously.

what i actually wanted to know is whether we missed a case that is not covered here. the lookup_url_kwarg was already used by the urlpatterns resolution. here we merely tried to establish the link to the model. the name was already subsituted way earlier in DRF. to my understanding the lookup_field="foo__bar__id" was/is not leaked once lookup_url_kwarg is explicitly set.

EDIT: had to fix my last paragraph as i used the wrong words.

[spookylukey]

OK, what happened was that I originally did this branch starting somewhere else, perhaps based on top of my old branch, in which the test did fail without my additional changes (I was careful to do it test driven etc.)

However, since some other commits, probably your merge commit or 386346b my changes became redundant, but I didn't notice this when rebasing, and I didn't properly understand your previous comment. Sorry for the noise!

[tfranzel]

no worries! just wanted to make sure we covered all cases and didn't miss anything. glad this is now working for you.