#5775 closed (duplicate)
staff_member_required decorator loses query parameters
| Reported by: | jdetaeye | Owned by: | nobody |
|---|---|---|---|
| Component: | contrib.admin | Version: | newforms-admin |
| Severity: | Keywords: | admin staff_member_required sprintdec01 nfa-someday | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
To reproduce the problem:
- Open a new browser window
- Enter a admin url that uses a query parameter.
A good example is the url for displaying a filtered or sorted list of objects:
/admin/yourapp/yourmodel/?yourfield__exact=12
- Since you're not authenticated yet, the login page is shown.
- After a successfull login, you will be redirected to the page:
/admin/yourapp/yourmodel/
The query parameter is lost along the way...
Keeping the query parameters is handy when e.g. the user wants to bookmark such a filtered or sorted list.
The fix is pretty simple: The problem is the use of the request.path field, rather than the request.get_full_path method.
The change affects affects both the normal admin as well as the new-forms admin branch, and the same patch applies.
Attachments (3)
Change History (13)
by , 18 years ago
| Attachment: | staff_member_required.patch added |
|---|
comment:1 by , 18 years ago
| Summary: | staff_member_required decorator looses query parameters → staff_member_required decorator loses query parameters |
|---|
comment:2 by , 18 years ago
| Patch needs improvement: | set |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
| Version: | SVN → newforms-admin |
Looks good, but do you think you could rewrite this patch against newforms-admin? The decorator module has moved inside views, but it looks pretty similar.
by , 18 years ago
| Attachment: | staff_member_required_newadmin.patch added |
|---|
same patch, now for newforms-admin
by , 18 years ago
| Attachment: | test_staffmemberrequired.patch added |
|---|
Additional test for this decorator
comment:4 by , 18 years ago
| Keywords: | sprintdec01 added |
|---|
comment:5 by , 18 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
comment:6 by , 18 years ago
| Keywords: | nfa-someday added |
|---|
Should not block merge since it's a problem with old admin as well.
comment:7 by , 17 years ago
| milestone: | → 1.0 |
|---|
comment:8 by , 17 years ago
Duplicate issue #5801 But that has patch that also looks after the non decorated admin views
comment:9 by , 17 years ago
| Resolution: | → duplicate |
|---|---|
| Status: | new → closed |
Yeah, the patch on #5801 is better; marking this as a dup.
patch (appending me also to the list of authors)