Apologies, I always forget to hit "Reply All".
-------- Original Message -------- Subject: Re: AbuseFilter error codes and MobileFrontend Date: Sat, 21 Sep 2013 10:11:28 +1000 From: Andrew Garrett agarrett@wikimedia.org To: Juliusz Gonera jgonera@wikimedia.org
Juliusz Gonera wrote:
After getting some initial information from legoktm, my thoughts are:
- Probably no changes to AbuseFilter are necessary and we should
implement everything in MobileFrontend.
- We should display the warning message (edit.warning in API response)
for all codes (edit.code in API response) that start with abusefilter-warning* and allow the user to resubmit.
- We should display the disallow message (edit.warning in API
response) for abusefilter-disallow and not allow the user to resubmit.
- We should display edit.warning message if present or a general one
and not allow the user to resubmit for all the other error codes (until now we've got abusefilter-blanking, abusefilter-blank, abusefilter-imza, abusefilter-blocked-display and abusefilter-autobiography, but they don't happen too often).
You should know about this change[1], which corrects the error messages to be more in line with the general case, as well as adding some metadata. It's not been approved yet, so I'm nudging a few reviewers.
You can also determine which mobile edits are hitting filters, by looking for edits in the filter log which have 'user_mobile = 1' set. I'm not sure I quite remember how to search in that way, but I'll look into it.
On 09/23/2013 06:48 PM, Andrew Garrett wrote:
You should know about this change[1], which corrects the error messages to be more in line with the general case, as well as adding some metadata. It's not been approved yet, so I'm nudging a few reviewers.
You can also determine which mobile edits are hitting filters, by looking for edits in the filter log which have 'user_mobile = 1' set. I'm not sure I quite remember how to search in that way, but I'll look into it.
I'm not sure if I understand what are the implications of this change for the mobile editor. Does it change the "code" key into "error-msg" key? If yes, should I rely on "error-msg" rather than "code" after this patch is merged to determine what is a warning and what is disallowed?
Or maybe I should just treat "code":"abusefilter-aborted" as if it was "code":"abusefilter-disallowed"?
Juliusz Gonera wrote:
On 09/23/2013 06:48 PM, Andrew Garrett wrote:
You should know about this change[1], which corrects the error messages to be more in line with the general case, as well as adding some metadata. It's not been approved yet, so I'm nudging a few reviewers.
You can also determine which mobile edits are hitting filters, by looking for edits in the filter log which have 'user_mobile = 1' set. I'm not sure I quite remember how to search in that way, but I'll look into it.
I'm not sure if I understand what are the implications of this change for the mobile editor. Does it change the "code" key into "error-msg" key? If yes, should I rely on "error-msg" rather than "code" after this patch is merged to determine what is a warning and what is disallowed?
You will now start to receive a consistent error message of 'abusefilter-aborted' included in the error.code property of the output instead of the hodgepodge of other AbuseFilter errors, which were returned in a nonstandard format. You can still find the detailed error message in other properties of the error object.
— Andrew Garrett
On 09/25/2013 02:38 AM, Andrew Garrett wrote:
On 09/23/2013 06:48 PM, Andrew Garrett wrote:
You should know about this change[1], which corrects the error messages to be more in line with the general case, as well as adding some metadata. It's not been approved yet, so I'm nudging a few reviewers.
You can also determine which mobile edits are hitting filters, by looking for edits in the filter log which have 'user_mobile = 1' set. I'm not sure I quite remember how to search in that way, but I'll look into it.
I'm not sure if I understand what are the implications of this change for the mobile editor. Does it change the "code" key into "error-msg" key? If yes, should I rely on "error-msg" rather than "code" after this patch is merged to determine what is a warning and what is disallowed?
You will now start to receive a consistent error message of 'abusefilter-aborted' included in the error.code property of the output instead of the hodgepodge of other AbuseFilter errors, which were returned in a nonstandard format. You can still find the detailed error message in other properties of the error object.
I've just checked on my local instance and it seems that this patch does not change much for the MobileFrontend. As I stated in the first message, we need to distinguish between warnings and disallowed edits because they require different UI (either allow resubmitting or not). It seems that without this patch the only way to do this is looking at the `code` property and with this patch I should look at the `error-msg` property instead (and keep my fingers crossed that some admin didn't come up with a different word for "warning").
I understand that thanks to this patch we can be 100% sure that the error comes from AbuseFilter even if the `error-msg` has no "abusefilter-" prefix. However, we should also include some information which would allow us to figure out which action from "Actions taken when matched" box in filter config a given API response is referring to. That would be much more reliable than looking for "abusefilter-warning-" prefix which can be omitted by admins setting up the warning.
On 09/25/2013 12:17 PM, Juliusz Gonera wrote:
I've just checked on my local instance and it seems that this patch does not change much for the MobileFrontend. As I stated in the first message, we need to distinguish between warnings and disallowed edits because they require different UI (either allow resubmitting or not). It seems that without this patch the only way to do this is looking at the `code` property and with this patch I should look at the `error-msg` property instead (and keep my fingers crossed that some admin didn't come up with a different word for "warning").
I understand that thanks to this patch we can be 100% sure that the error comes from AbuseFilter even if the `error-msg` has no "abusefilter-" prefix. However, we should also include some information which would allow us to figure out which action from "Actions taken when matched" box in filter config a given API response is referring to. That would be much more reliable than looking for "abusefilter-warning-" prefix which can be omitted by admins setting up the warning.
I think the best way to proceed right now is to use the `code` key to determine what to do and we should figure out how to make it better in future. Is there any way I can get notified when your patch gets merged? Not sure if there is such an option somewhere in Gerrit's confusing UI...
On 09/26/2013 02:46 PM, Juliusz Gonera wrote:
I think the best way to proceed right now is to use the `code` key to determine what to do and we should figure out how to make it better in future. Is there any way I can get notified when your patch gets merged? Not sure if there is such an option somewhere in Gerrit's confusing UI...
If you add yourself as a reviewer, you will be (even if you don't actually review).
Matt Flaschen
On 2013-09-27 2:34 AM, Matthew Flaschen wrote:
On 09/26/2013 02:46 PM, Juliusz Gonera wrote:
I think the best way to proceed right now is to use the `code` key to determine what to do and we should figure out how to make it better in future. Is there any way I can get notified when your patch gets merged? Not sure if there is such an option somewhere in Gerrit's confusing UI...
If you add yourself as a reviewer, you will be (even if you don't actually review).
Matt Flaschen
I think the star also works that way.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
Juliusz Gonera wrote:
I understand that thanks to this patch we can be 100% sure that the error comes from AbuseFilter even if the `error-msg` has no "abusefilter-" prefix. However, we should also include some information which would allow us to figure out which action from "Actions taken when matched" box in filter config a given API response is referring to. That would be much more reliable than looking for "abusefilter-warning-" prefix which can be omitted by admins setting up the warning.
I'll take a look at adding this.
— Andrew Garrett
wikitech-l@lists.wikimedia.org