Add Http Response Codes to doc comments #5867
Conversation
| @@ -258,7 +258,7 @@ public virtual ObjectResult StatusCode(int statusCode, object value) | |||
| } | |||
|
|
|||
| /// <summary> | |||
| /// Creates a <see cref="ContentResult"/> object by specifying a <paramref name="content"/> string. | |||
| /// Creates a <see cref="ContentResult"/> object (HTTP 200) by specifying a <paramref name="content"/> string. | |||
There was a problem hiding this comment.
I want to homoginize these with the existing status code messages, but it's kind of hard to fit OK (200) in. Wording suggestions? My best idea was ...specifying a <paramref name="content"/> string with OK (200). but even that seemed kind of stilted.
There was a problem hiding this comment.
Why not write <see cref="StatusCodes.Status200OK"/> instead?
There was a problem hiding this comment.
I like this idea, I'll update the PR.
| @@ -1306,15 +1306,15 @@ public virtual ChallengeResult Challenge(AuthenticationProperties properties) | |||
| => new ChallengeResult(authenticationSchemes, properties); | |||
|
|
|||
| /// <summary> | |||
| /// Creates a <see cref="ForbidResult"/>. | |||
| /// Creates a <see cref="ForbidResult"/> (HTTP 403). | |||
There was a problem hiding this comment.
FYI, though a 403 response is what happens by default when calling this method, there's no guarantee about that, as the status code is actually specific to the handler that handles the ForbidResult.
For instance, the cookies middleware will redirect the user to the "access denied page" (302) instead of returning an empty 403 response.
There was a problem hiding this comment.
By this logic we wouldn't include any status codes in these comments because any of them could be modified by a middleware. I think we're ok with these being overriden in some cases as long as the convey the intended behavior. @Eilon do you agree?
There was a problem hiding this comment.
Maybe say (HTTP 403 by default)? I want to make sure we don't throw the baby out with the bathwater here.
I'd be fine if, for example, in the <remarks> section it mentioned that some authentication schemes, such as cookies, will typically convert the HTTP 403 to an HTTP redirect to show the user a login page.
ryanbrandenburg
force-pushed
the
rybrande/HttpResponseCode
branch
from
9a3fd3a
to
e5a305d
Compare
|
🆙📅 |
|
Out of curiosity, why doesn't |
|
@PinpointTownes yeah I would think so. @ryanbrandenburg can you update that as well? |
|
Did we want to give possible StatusCodes of SignIn and SignOut too or do we think the result of them likely enough to be different as not to mention it? |
| @@ -258,7 +258,7 @@ public virtual ObjectResult StatusCode(int statusCode, object value) | |||
| } | |||
|
|
|||
| /// <summary> | |||
| /// Creates a <see cref="ContentResult"/> object by specifying a <paramref name="content"/> string. | |||
| /// Creates a <see cref="ContentResult"/> object with <see cref="StatusCodes.Status200OK"/> by specifying a <paramref name="content"/> string. | |||
There was a problem hiding this comment.
nit: Could break it down to two lines. Applies to few other places in this file
ryanbrandenburg
force-pushed
the
rybrande/HttpResponseCode
branch
from
57b18dd
to
4605ff2
Compare
Fixes #5855.