Opened 7 years ago

Closed 9 months ago

#8690 closed New feature (fixed)

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 7 years ago.
Add request parameter to parse_params
8690.diff (1.5 KB) - added by ramiro 6 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 (13)

Changed 7 years ago by bthomas

Add request parameter to parse_params

comment:1 Changed 7 years ago by bthomas

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

comment:2 Changed 7 years ago by bthomas

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

comment:3 Changed 6 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 6 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 6 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 6 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 4 years ago by lukeplant

  • Severity set to Normal
  • Type set to New feature

comment:7 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:8 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:9 Changed 2 years ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

comment:10 Changed 2 years ago by timo

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

comment:11 Changed 9 months ago by gchp

  • Resolution set to fixed
  • Status changed from new to closed

formtools has been extracted into its own repository (​​​https://github.com/django/django-formtools/). Because of this, the issue tracking for this package has been moved to GitHub issues. I'm going to close this ticket, but I've created a GitHub issue to replace it where the conversation can continue: ​​​https://github.com/django/django-formtools/issues/22. Thanks!

Note: See TracTickets for help on using tickets.
Back to Top