django
django copied to clipboard
django
Fixed #30386 -- Fixed primary key quoting in admin related object links.
This is the branch I created to fix the issue of admin widgets not quoting primary keys. I fixed it from the server side and client side(creating a function in the Javascript file and implementing a quote function to quote the keys) and i added test cases to check that the function i used to quote the primary keys for ticket-30386 works. I also used Javascript test and selenium to automate testing
@carltongibson Please can you check the implementation in the new commit I made? Java script encodeURIComponent() and encodeURI() didn't fix it. So I had to write a function, using the idea of encodeURIComponent() implementation to quote the primary key. I wrote a test for it. I also tested to update related object links when a value is passed, it quotes it and update the link.
It's getting there. You've got the testing harnessing in the right place, but the escaping isn't right.
For Topping pk="_40": Change Form page URL: http://127.0.0.1:8000/admin/test_one/topping/_5F40/change/ Generated Pop-up URL: http://127.0.0.1:8000/admin/test_one/topping/%5F40/change/?_to_field=key&_popup=1 (without patch has just
_40)So essentially the escaping is not the same. Look at the
quoteimplementation inadmin.utils. Match that and it would work:>>> from django.contrib.admin.utils import unquote >>> unquote("%5F40") # what you're sending now. '%5F40' >>> unquote("_5F40") # what's needed. '_40'.
It's getting there. You've got the testing harnessing in the right place, but the escaping isn't right.
For Topping pk="_40": Change Form page URL: http://127.0.0.1:8000/admin/test_one/topping/_5F40/change/ Generated Pop-up URL: http://127.0.0.1:8000/admin/test_one/topping/%5F40/change/?_to_field=key&_popup=1 (without patch has just
_40)So essentially the escaping is not the same. Look at the
quoteimplementation inadmin.utils. Match that and it would work:>>> from django.contrib.admin.utils import unquote >>> unquote("%5F40") # what you're sending now. '%5F40' >>> unquote("_5F40") # what's needed. '_40'.
@carltongibson do you mean I should use the quote implementation in admin.utils for the implementation of the customEncodeURIComponent? since the fix is now from RelatedObjectLookups.js?
The quoting in the JavaScript needs to match that done by the quote until, and expected by unquote, yes.
Yes it makes sense. My question is, does it mean i'm not suppose to click here
self.selenium.find_element(By.ID, "id_house").click()
Since i already saved it here
self.selenium.find_element(By.NAME, "_save").click()
@Oluwayhemisi You need to step through what's happening in the browser. Once you've created the Room you need to be on the change form page in order to check that the "Edit" pencil-icon popup has the correctly escaped URL.
(So you either need to use the save-and-keep-editing button, or return to the change form from the change list… — or you could create the Room in code and then just load the change form once... — either way, what you're testing is that the PK value is correctly quoted and unquoted in the UI.)
Yes it makes sense. My question is, does it mean i'm not suppose to click here self.selenium.find_element(By.ID, "id_house").click() Since i already saved it here self.selenium.find_element(By.NAME, "_save").click()
@Carlton, I have been able to fix it. But i ran into another error When i create the Room it takes me to the change form page and I check that the "Edit" pencil-icon popup has the correct escaped URL.
But the issue is that I'm getting Timeout exception due to this line below
` self.wait_for_value("#id_name", house_name)
`
If i remove the line of code above, on the browser, it takes me to the change form and when i try to check that the "Edit" pencil-icon popup has the correct escaped URL, the browser stops, though the URL shows it has been escaped but it doesn't display anything on the page
Yes it makes sense. My question is, does it mean i'm not suppose to click here self.selenium.find_element(By.ID, "id_house").click() Since i already saved it here self.selenium.find_element(By.NAME, "_save").click()
@carlton, I have been able to fix it. But i ran into another error When i create the Room it takes me to the change form page and I check that the "Edit" pencil-icon popup has the correct escaped URL.
But the issue is that I'm getting Timeout exception due to this line below
` self.wait_for_value("#id_name", house_name)
`
If i remove the line of code above, on the browser, it takes me to the change form and when i try to check that the "Edit" pencil-icon popup has the correct escaped URL, the browser stops, though the URL shows it has been escaped but it doesn't display anything on the page
@carltongibson as regards to the issue of timeout exception, what i did is that i added time.sleep(5) to wait for the element to have the expected value. So it takes me to the page where the "Edit" pencil-icon popup would direct me when I click on it. please what's do you think about it? am I on the right path?
Hey @Oluwayhemisi 👋
Carlton has moved on from his fellow role.
If you're happy the review comments are addressed I suggest unticking the 'needs improvement' box on trac and I'm sure someone else will be along in due course to help review.
https://www.djangoproject.com/weblog/2023/mar/31/welcome-our-new-fellow-natalia-bidart/
Hey @Oluwayhemisi wave
Carlton has moved on from his fellow role.
If you're happy the review comments are addressed I suggest unticking the 'needs improvement' box on trac and I'm sure someone else will be along in due course to help review.
https://www.djangoproject.com/weblog/2023/mar/31/welcome-our-new-fellow-natalia-bidart/
Alright, thank you.
Hello @Oluwayhemisi! I will be trying to help you move this forward. With that in mind, I may push a few commits to this PR so we can continue iterating over it. Let me if you have any concerns!
@Oluwayhemisi I have made a few extra pushes to help resolving most comments in this PR. But there is still the issue reported by Mariusz, in that when a Topping is modified from the Pizzas change form (using the pencil action), when returning to the Pizza form the topping has not changed.
Do you think you could try to diagnose and fix that? Thank you!
@nessita Thank you for the commits, I will go through it and give you a feedback. As regards to the issue reported by Mariusz, I will look into it and try and fix it.
