Code

Opened 6 years ago

Last modified 9 months ago

#8690 new New feature

FormPreview should pass request to parse_params

Reported by: bthomas Owned by: nobody
Component: contrib.formtools Version: master
Severity: Normal Keywords: formpreview
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

It would be very useful to have request data in parse_params. The example given is to capture something like a user_id, but the main thing I would want to do here is to verify that the current user has permission to access the URL given. Of course, adding a parameter breaks backwards compatibility, so this is a similar request to #7222.

Attachments (2)

params.diff (973 bytes) - added by bthomas 6 years ago.
Add request parameter to parse_params
8690.diff (1.5 KB) - added by ramiro 5 years ago.
Patch that tries to provide backwards compatibility. No tests and no docs yet, they can be added if idea isn't deeemed wrong

Download all attachments as: .zip

Change History (12)

Changed 6 years ago by bthomas

Add request parameter to parse_params

comment:1 Changed 6 years ago by bthomas

  • Has patch set
  • Keywords formpreview added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by bthomas

Forgot to mention, FormWizard's version of parse_params includes the request parameter.

comment:3 Changed 5 years ago by jacob

  • Triage Stage changed from Unreviewed to Design decision needed

This unfortunately introduces a subtle backwards-incompatibility: if parse_params is defined in the old way (parse_params(*args, **kw)), then args[0] will become request with this patch, thereby breaking user code. So we need to make sure this is really a vital change before we make that change.

comment:4 Changed 5 years ago by bthomas

I'm aware of the incompatibility (I mentioned it in the description), and it is unfortunate. We could define a new function like parse_request_params that passes request as the first argument (and calls parse_params without it), but I think that's a bit ugly.

This will only affect users that:

  1. Are overriding parse_params
  2. Are using unnamed args

Personally, I only use named arguments in URLconfs, but I can't speak for the rest of the community.

Changed 5 years ago by ramiro

Patch that tries to provide backwards compatibility. No tests and no docs yet, they can be added if idea isn't deeemed wrong

comment:5 Changed 5 years ago by bthomas

This works for me. Should we also add a DeprecationWarning for users who define parse_params the old way? Something like:

            import warnings
            warnings.warn(
                message = "Defining parse_params without the request parameter is deprecated. Define it as parse_params(self, request, *args, **kwargs) instead.",
                category = DeprecationWarning,
                stacklevel = 2
            )

comment:6 Changed 3 years ago by lukeplant

  • Severity set to Normal
  • Type set to New feature

comment:7 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:8 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:9 Changed 13 months ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

comment:10 Changed 9 months ago by timo

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.