Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ARV] Confirm reporting blocked users #817

Closed
wants to merge 3 commits into from
Closed

[ARV] Confirm reporting blocked users #817

wants to merge 3 commits into from

Conversation

DannyS712
Copy link
Contributor

Before proceeding to open the form, check if the relevant user is blocked or range blocked. If blocked, request confirmation.
The former "Twinkle.arv.callback" is now at "Twinkle.arv.callback.showform" - other than the portletLink, I didn't see any callers
Closes #816

Before proceeding to open the form, check if the relevant user is blocked or range blocked. If blocked, request confirmation.
The former "Twinkle.arv.callback" is now at "Twinkle.arv.callback.showform" - other than the portletLink, I didn't see any callers
Closes #816
@DannyS712
Copy link
Contributor Author

Testing in developer tools:

  • Delete the existing portlet
  • Paste in the new code
  • Call Twinkle.load()
    Tested with user blocked, IP blocked, range blocked, and no blocks, works

params.bkusers = username;
}

new mw.Api().get(params).then(function(data) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably this would be nicer after #765 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure - this is based on code in checking for blocks elsewhere in the repo (don't remember where exactly) but morebits api may not be needed

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

twinkleblock. No, it's not needed, but I believe it's the kind of thing that #765 would enable Morebits.wiki.api to handle (note: I haven't really reviewed #765). Not necessarily suggesting you change it, just noting its existence, especially given that it's an odd-one-out.

Copy link
Collaborator

@Amorymeltzer Amorymeltzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested yet but what are the merits of doing something like I (eventually came around to having) did in #757, specifically around #757 (comment)? We're more likely to not want to report someone with ARV if they are currently blocked, so I suppose the popup is more like to make sense here, but worth considering.

format: 'json',
action: 'query',
list: 'blocks',
bkprop: 'range'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably only needs to be added if isIP, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from what I recall, this is ignored if its not an IP

@Amorymeltzer
Copy link
Collaborator

Amorymeltzer commented Feb 10, 2020

Thoughts on the above, @DannyS712?

Also, twinklearv already has some mw.Apis, I think it'd be good to consolidate them.

Beyond that, do we really want to check for every option before loading? Notifying before reporting to AIV and UAA is good, they'll just get autoremoved otherwise. I guess AN3 as well, but do we definitely want to do this before SPI reporting? Especially for the sock option. I suppose it doesn't hurt much, but seems a waste, especially if it's going to be delaying opening the window.

Just thinking out loud: what about checking after the user has submitted the form? We could cancel if AIV/UAA, then check/notify for AN3/SPI. Does that make sense or am I mixed up?

@siddharthvp
Copy link
Member

I too don't think it's good practise to put an API call that delays the opening of the form. One expects the something to open immediately when the they click the menu button. Using the red text option (as in #757) would be better IMO.

Also, is there a reason why mw.Api is used instead of morebits.wiki.api? The latter now supports JSON, so you can simply replace
new mw.Api().get(params).then(successFunc, failureFunc) with
new Morebits.wiki.api('Checking if already blocked', params, successFunc, null, failureFunc).post()

Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Nov 2, 2020
Reworked version of wikimedia-gadgets#817, simply inserting some red text if the user being reported is (already) blocked.  Closes wikimedia-gadgets#816

Co-authored-by: Amory Meltzer <Amorymeltzer@gmail.com>
@Amorymeltzer
Copy link
Collaborator

Done in #1184

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn if reporting a blocked user to AIV/UAA
3 participants