In T194585#4204694, @Anomie wrote:

That goes beyond exposing User::isBlockedFrom(), and is not supported by the 'UserIsBlockedFrom' hook.

Returning a bool seemed kinda silly when User::isBlockedFrom() determines this from a single block and most (if not all?) clients will need to tell the user the info from the block. I think if the method returns true it should just return the result of User::isBlocked(). But maybe this is a good idea for a param. :)

In T194585#4204705, @dbarratt wrote:

Returning a bool seemed kinda silly when User::isBlockedFrom() determines this from a single block and most (if not all?) clients will need to tell the user the info from the block. I think if the method returns true it should just return the result of User::isBlocked().

Err, User::isBlocked() also returns a boolean. And, as I already pointed out to you, the 'UserIsBlockedFrom' hook does not return a block object at all.

Reworking any of that is probably more in scope of T190350: Epic: ⚡️ Partial blocks.

In T194585#4204710, @Anomie wrote:

Err, User::isBlocked() also returns a boolean. And, as I already pointed out to you, the 'UserIsBlockedFrom' hook does not return a block object at all.

My apologies, I meant User::getBlock()

I think Brad's first suggestion makes the most sense ("Add to ApiQueryInfo an inprop to indicate whether the current user is blocked from the page."). This would be similar to the existing readable prop. Although it would be nice to get the full block info back, I don't think this is strictly necessary. We could always output a generic message like "You are currently blocked from editing this page. Learn more." and have "Learn more" link to the user's block log or something like that. This would also let us work around T194530.

In T194585#4208646, @kaldari wrote:

I think Brad's first suggestion makes the most sense ("Add to ApiQueryInfo an inprop to indicate whether the current user is blocked from the page.").

How would this work if the page doesn't exist yet? This would happen if I was blocked from a specific namespace. I mean the client could always go and get the block and figure out if the user is blocked, but I'd rather avoid having the same logic on the client and the server if that can be avoided.

In T194585#4209964, @dbarratt wrote:

How would this work if the page doesn't exist yet?

It would work well.

In T194585#4209986, @Anomie wrote:

It would work well.

Oh. I didn't know you can query for nonexistant pages.
https://www.mediawiki.org/wiki/API:Query#Missing_and_invalid_titles
Well I learn something new everyday!

Change 461829 had a related patch set uploaded (by Dbarratt; owner: Dbarratt):
[mediawiki/core@master] Add a prop to ApiQueryInfo to indicate if user is blocked from current page.

https://gerrit.wikimedia.org/r/461829

It seems this already works:

https://www.mediawiki.org/w/api.php?action=query&titles=Main%20Page|API:Query&prop=info&intestactions=edit

In T194585#4606933, @Umherirrender wrote:

It seems this already works:

https://www.mediawiki.org/w/api.php?action=query&titles=Main%20Page|API:Query&prop=info&intestactions=edit

Well fancy that! I'll ensure this works properly with the partial block changes, if it does (I don't see why it wouldn't looking at the code) then we can close this task! :)

The partial block changes have to add some checks to Title::userCan to support namespace or page depending user blocks in all places of mediawiki (like edit tabs in the navigation etc.)

Just for hint: The checks are more expensive than the IsProbablyEditable flag used by VisualEditor in the web ui (but that also includes create checks)

@Umherirrender I get a true value even if the user is blocked (on master), so this doesn't resolve the use case.

should User::isBlockedFrom be executed from Title::userCan? If not, then another property should be created, or I suppose this property should take both into consideration?

In T194585#4607666, @dbarratt wrote:

@Umherirrender I get a true value even if the user is blocked (on master), so this doesn't resolve the use case.

should User::isBlockedFrom be executed from Title::userCan? If not, then another property should be created, or I suppose this property should take both into consideration?

For me this reports an empty "actions" key in the json response as expected.
I am testing this as loggedin blocked user on current master.

User::isBlockedFrom is executed from Title::userCan in Title::checkUserBlock which is part for every check in Title::getUserPermissionsErrorsInternal

In T194585#4608015, @Umherirrender wrote:

For me this reports an empty "actions" key in the json response as expected.
I am testing this as loggedin blocked user on current master.

User::isBlockedFrom is executed from Title::userCan in Title::checkUserBlock which is part for every check in Title::getUserPermissionsErrorsInternal

I was using formatversion=2 and getting true with a blocked and unblocked user, but I'll try again.

I double checked this, and it does work correctly.

Reference URL (for myself):
http://localhost:8888/api.php?action=query&titles=Gotham&prop=info&intestactions=edit&formatversion=2

Change 461829 abandoned by Dbarratt:
Add a prop to ApiQueryInfo to indicate if user is blocked from current page.

Reason:
Unnecessary

https://gerrit.wikimedia.org/r/461829

Change 461829 restored by Dbarratt:
Add a prop to ApiQueryInfo to indicate if user is blocked from current page.

https://gerrit.wikimedia.org/r/461829

I'm not sure "you're blocked!" is all that useful of a "why". Perhaps the better solution would be to have intestactions (optionally?) return the message data from Title::getUserPermissionsErrors().

In T194585#4616156, @Anomie wrote:

I'm not sure "you're blocked!" is all that useful of a "why". Perhaps the better solution would be to have intestactions (optionally?) return the message data from Title::getUserPermissionsErrors().

Well the client would have to lookup the block to get the block reason regardless.

In T194585#4616156, @Anomie wrote:

I'm not sure "you're blocked!" is all that useful of a "why". Perhaps the better solution would be to have intestactions (optionally?) return the message data from Title::getUserPermissionsErrors().

Implemented that in https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/463292. Using that with a query like /w/api.php?action=query&prop=info&titles=Test&intestactions=edit&intestactionsdetail=full, a user that is blocked would show as something like

"actions": {
    "edit": [
        {
            "code": "blocked",
            "text": "You have been blocked from editing.",
            "data": {
                "blockinfo": {
                    "blockid": 7,
                    "blockedby": "Anomie",
                    "blockedbyid": 1,
                    "blockreason": "testing",
                    "blockedtimestamp": "2018-09-27T15:10:36Z",
                    "blockexpiry": "2018-09-27T15:11:36Z"
                }
            }
        }
    ],
}

The "blocksitewide" flag from https://gerrit.wikimedia.org/r/c/mediawiki/core/+/442222/39/includes/api/ApiQueryUserInfo.php would also be included in there.

In T194585#4622440, @Anomie wrote:

Implemented that in https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/463292. Using that with a query like /w/api.php?action=query&prop=info&titles=Test&intestactions=edit&intestactionsdetail=full, a user that is blocked would show as something like

I think you may have misunderstood me in T194585#4204705, because what you've done here is exactly what we are looking for. Ideally, the client (in this case, mobile web) should be able to 1) determine if the user is blocked on a per-page basis and 2) get the reason for the block (preferably on a single request). Your patch allows for that. :)

I'm not a huge fan of changing the param type, (from boolean to an object), but other than using another name for "actions" I don't have a solution (and that might make the params weirder anyways). I also wish the block didn't prefix block in it's params, but that's a separate issue.

I'll go ahead an abandon the patch I wrote since I think your solution is better (and what I was looking for in the first place).

In my earlier comments here, I forgot that intestactions existed. I re-found it during code review of your original patch and realized that that would be a better way to handle determining whether a page is editable or not.

In T194585#4627288, @dbarratt wrote:

because what you've done here is exactly what we are looking for.

I'm glad, that's what I was hoping for (:

I'm not a huge fan of changing the param type, (from boolean to an object),

No parameter types are changing in my patch. It does add a parameter to change the output field type, but in a backwards-compatible way since the change is gated behind a parameter.

but other than using another name for "actions" I don't have a solution (and that might make the params weirder anyways).

I agree it would be weirder to have intestactions and something else that does the same thing with a different output format. Unless we were going to deprecate intestactions entirely, but I think it still serves well for its original purpose with the boolean returns.

 I also wish the block didn't prefix block in it's params, but that's a separate issue.

It reuses code that displays permissions errors in other contexts, which in turn reuses the block-formatting code from ApiQueryUserInfo.

Change 461829 abandoned by Dbarratt:
Add a prop to ApiQueryInfo to indicate if user is blocked from current page.

Reason:
Better solution at https://gerrit.wikimedia.org/r/c/mediawiki/core/ /463292

https://gerrit.wikimedia.org/r/461829

Change 463292 had a related patch set uploaded (by Dbarratt; owner: Anomie):
[mediawiki/core@master] API: Allow prop=info intestactions to return reasons

https://gerrit.wikimedia.org/r/463292

@Anomie while I was testing your patch, I found this bug T205895, it's not related to this task explicitly, but partial blocks do not work properly with your patch until we fix it.

In T194585#4630774, @dbarratt wrote:

@Anomie while I was testing your patch, I found this bug T205895, it's not related to this task explicitly, but partial blocks do not work properly with your patch until we fix it.

Correction... since prevents( 'edit' ) always returns true for partial blocks, then T205895 is not an issue for partial blocks.

Change 463292 merged by jenkins-bot:
[mediawiki/core@master] API: Allow prop=info intestactions to return reasons

https://gerrit.wikimedia.org/r/463292