Make WordPress Core

Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#35426 closed defect (bug) (fixed)

Create response status code aliases on WP_Http for convenience

Reported by: joehoyle Owned by: joehoyle
Priority: normal Milestone: 4.5
Component: HTTP API Version:
Severity: normal Keywords: has-unit-tests has-patch 2nd-opinion
Cc: Focuses:

Description

Throughout the REST API for response codes, we want to use aliased constants to be more clear in the code for each response code.

The best place to put these constants I think is WP_Http. This means in the REST API (and any other code) rather than specifying response codes directly (which is terse and opaque) one can use WP_Http::BAD_REQUEST for example.

Attachments (5)

35426.diff (2.8 KB ) - added by joehoyle 11 years ago.
35426.2.diff (2.9 KB ) - added by joehoyle 11 years ago.
35426.3.diff (3.0 KB ) - added by joehoyle 11 years ago.
Added missing status codes and docblock
35426.tests.diff (676 bytes ) - added by johnbillion 11 years ago.
35426.4.diff (568 bytes ) - added by johnbillion 11 years ago.

Download all attachments as: .zip

Change History (16)

@joehoyle
11 years ago

@joehoyle
11 years ago

#2 @joehoyle
11 years ago

  • Owner set to joehoyle
  • Status newassigned

@joehoyle
11 years ago

Added missing status codes and docblock

#3 @joehoyle
11 years ago

  • Resolutionfixed
  • Status assignedclosed

In 36294:

Add response status code aliases on WP_Http for convenience.

These provide a more descriptive way to set response codes elsewhere,
so it's readable and less chance for the wrong response code to be
used such as 401 vs 403.

Props rmccue for the idea.
Fixes #35426.

#4 @johnbillion
11 years ago

  • Keywords needs-patch needs-unit-tests added
  • Resolution fixed
  • Status closedreopened
  • Version trunk

The 1xx statuses are missing.

#5 @johnbillion
11 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

@johnbillion
11 years ago

#6 @johnbillion
11 years ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

I wonder if these constant names need to be prefixed. CONTINUE is a reserved word in PHP so can't be used as a constant name.

Better: WP_Http::STATUS_CONTINUE, WP_Http::STATUS_OK, etc.

#7 @rmccue
11 years ago

@johnbillion That was actually intentional originally (I asked @joehoyle to leave them out), as it's almost impossible for those to get all the way up the stack. Additionally, you should probably never actually send them.

That said, we might want them for coverage. If we have conflicts with keywords, I'd prefer we special-case those specific ones rather than all of them.

Version 0, edited 11 years ago by rmccue (next)

#8 @knutsp
11 years ago

When introduced, they should be used globally, right?

Will it be considered "refactoring just because we can" if these constants was implemented in get_status_header_desc()?

If not, I can create ticket and patch.

#9 @joehoyle
11 years ago

@knutsp give that the descriptions already exist in get_status_header_desc i'm not sure how much value there is to doing that, but I don't feel strongly either way.

@johnbillion I think we'd be better not renaming all constants with STATUS_* just for CONTINUE, I'd rather we give 100 a description that isn't a reserved word.

#10 @johnbillion
10 years ago

  • Resolutionfixed
  • Status reopenedclosed

Fixed in r36749

This ticket was mentioned in Slack in #core by rmccue. View the logs.


10 years ago

Note: See TracTickets for help on using tickets.