Code

Opened 6 years ago

Closed 4 years ago

#6991 closed (fixed)

Omit redundant "if" judgements

Reported by: Liang Feng <hutuworm@…> Owned by: alexkoshelev
Component: contrib.admin Version: newforms-admin
Severity: Keywords: nfa-someday yandex-sprint ep2008
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by ramiro)

django/contrib/admin/views/decorators.py:
if request.user.is_authenticated() and request.user.is_staff:

### COMMENT: request.user.is_staff is True implied that request.user.is_authenticated() is True, so request.user.is_authenticated() could be omitted.

docs/authentication.txt:
if not (request.user.is_authenticated() and request.user.has_perm('polls.can_vote')):

### COMMENT: request.user.has_perm('polls.can_vote') is True implied that request.user.is_authenticated() is True, so request.user.is_authenticated() could be omitted.

p.s. may save some precious cpu cycles on Google App Engine. :P

Attachments (2)

6991.diff (2.0 KB) - added by alexkoshelev 6 years ago.
Patch for this ticket and #6990
6991.2.diff (3.3 KB) - added by Ivan Sagalaev <Maniac@…> 6 years ago.
New patch with couple more places

Download all attachments as: .zip

Change History (10)

comment:1 Changed 6 years ago by ramiro

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by Karen Tracey <kmtracey@…>

  • Keywords nfa-someday added; Omit redundant "if" judgements removed
  • Triage Stage changed from Unreviewed to Design decision needed
  • Version changed from SVN to newforms-admin

Like #6990, these checks were not redundant before the fix for #3032. They are still there in newforms-admin so someone needs to decide if it is worth removing them.

comment:3 Changed 6 years ago by alexkoshelev

  • Owner changed from nobody to alexkoshelev

Changed 6 years ago by alexkoshelev

Patch for this ticket and #6990

comment:4 Changed 6 years ago by Honza_Kral

  • Has patch set
  • Keywords ep2008 added
  • Patch needs improvement set
  • Triage Stage changed from Design decision needed to Accepted

After #3032 being fixed, this is really trivial, all tests pass. Just make sure to catch every occurrence of this behavior.

Changed 6 years ago by Ivan Sagalaev <Maniac@…>

New patch with couple more places

comment:5 Changed 6 years ago by Ivan Sagalaev <Maniac@…>

  • Patch needs improvement unset

Catched couple more places. Looks like they're all here.
Honza mentioned create_update.py in generic views but there are only standalone request.user.is_authenticated() checks that are all needed.

comment:6 Changed 6 years ago by Honza_Kral

  • Triage Stage changed from Accepted to Ready for checkin

thanks

comment:7 Changed 6 years ago by Ivan Sagalaev <Maniac@…>

  • Keywords yandex-sprint added

comment:8 Changed 4 years ago by adrian

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

(In [12142]) Fixed #6991 -- Removed some redundant user.is_authenticated() calls in various places. Thanks, alexkoshelev, Liang Feng and Ivan Sagalaev

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


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

 
Note: See TracTickets for help on using tickets.