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

Admins delete under multiple rationale, use template-generated delete reasons #300

Merged
merged 6 commits into from Jan 15, 2016

Conversation

MusikAnimal
Copy link
Collaborator

This is incomplete. Still needs testing, and there's at least one bug I haven't been able to fix

  • Allow admins to delete under multiple rationale
  • Allow admins to use the subgroups to enter in values such as the URL for G12 deletions
  • Use the corresponding template to generate a deletion summary
  • Move custom rationale to top with it's own section, and supply the rationale generated by the CSD template already on the page, if it exists

@MusikAnimal
Copy link
Collaborator Author

@atlight When there's already a CSD tag on the page, the module has "custom rationale" selected with the template-generated rationale in the subgroup input field. Problem is I can't get that subgroup to hide when I click on another radio button. If I go back and forth twice it will work as it should.

Basically I'm having trouble understanding what hides the subgroups. I put a breakpoint in openSubgroupHandler within twinklespeedyGenerateCsdList, and it never gets hit, no matter what I click on.

userMultipleRadioClick: 4, // check boxes, subgroups, need to add a "Submit" button
userSingleSubmit: 5, // radio buttons, subgroups, submit when "Submit" button is clicked
userSingleRadioClick: 6, // radio buttons, subgroups, submit when a radio button is clicked
sysopMultipleSubmit: 2, // check boxes, subgroups, "Submit" button already present
Copy link
Collaborator

Choose a reason for hiding this comment

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

The mode constants were very carefully laid out in an odd-even pattern to satisfy the code at line 247. Please test the code again with the "When to go ahead and tag/delete the page" Twinkle preference set to "As soon as I click an option" (i.e. speedySelectionStyle = radioClick) and fix the code as required.

I also wonder whether there need to be separate single/multiple sysop modes. The reason why tagging has separate single and multiple modes is that some CSD templates (e.g. db-hoax and the A7 subcategories) are not properly supported by db-multiple, so we have to prevent a different list to the user. (Arguably they could be combined into one, but that's a separate issue for another day.) For actual page deletion, is there any reason why we couldn't just get rid of the radio-button list and always show checkboxes?

@MusikAnimal
Copy link
Collaborator Author

Two outstanding bugs, as far as I can tell:

  1. As described above, when there's already a CSD tag, I can't get the custom rationale subgroup (input field) to hide when another radio button is selected.
  2. When in sysopMultipleRadioClick mode with speedySelectionStyle as radio, the module deletes as soon as a rationale checkbox is selected, rather than allowing you to select multiple ones then a hit submit under "When finished choosing criteria, click"

Any insight on these two issues? Thank you very much for your help and time in reviewing this!

@MusikAnimal
Copy link
Collaborator Author

@atlight just making sure you saw my note on the two outstanding bugs :)

@atlight
Copy link
Collaborator

atlight commented Nov 18, 2015

I'll have a look...

@atlight
Copy link
Collaborator

atlight commented Nov 18, 2015

For number 1, try adding event({ target: subnode }); after line 388 in morebits.js. See if that fixes it.

@MusikAnimal
Copy link
Collaborator Author

Deleting last two comments... fixed bug number 1! Now for number 2, which is the bigger issue

@MusikAnimal
Copy link
Collaborator Author

@atlight Alright! Going to move forward with testing in production, with the help our friend Callanecc :)

Let me know if you see any issues code-wise. Thanks!

…te-generated deletion summaries

twinkle: changing of defaults - delete talk page, no confirmation of deletion summary for any criterion, open talk pages in new tab rather than new window, which is blocked by most browser by default
@MusikAnimal
Copy link
Collaborator Author

@atlight Made some changes to the default Twinkle preferences. First, don't prompt for deletion summary under any criterion. This is not needed so much anymore with the advent of being able to pick multiple ratione and supply params, etc. Delete talk page by default, which we almost always want to do (except for user talk pages, which are excluded).

Finally, open user talk pages in a new tab and not a window. This is unrelated to the speedy update, but I figure this has been long overdue. Most browsers will block popups, so you have to explicitly allow Twinkle to do it. Most people prefer tabs anyway, I am willing to bet. Any input on this change is appreciated, otherwise I'm ready to merge in the speedy update. I've got a number of admins waiting for it! :)

label: 'Open user talk page on submit',
value: 'openusertalk',
name: 'openusertalk',
tooltip: 'Use this is override your preferences for the selected rationale',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Word "is" doesn't make sense here.

Also, if I were an admin, I would have trouble figuring out how this checkbox interacts with my preferences. This could do with some more explanation in the tooltip.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, this never seems to get checked in the case of multiple criteria

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You could loop through the selected rationale and see if your preferences are all the same for those criteria, but I don't really think this is that advantageous. In the case of multiple criteria we'll just let the admin decide what to do. Maybe it should default to checked, though

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correction, it leaves the checkbox state as-is once you choose to delete under multiple rationale. I feel this is best, as I don't want to make any assumptions or go about the crazy logic I mentioned above.

@atlight
Copy link
Collaborator

atlight commented Nov 25, 2015

I tried deleting a page under G4, typing garbage characters into the subgroup textbox. I was notified that my input was invalid (which is true), but then there was a JavaScript error: TypeError: params.templateParams is null and the CSD dialog sat there in an unusable state.

};

var statusIndicator = new Morebits.status( 'Building deletion summary' );
var api = new Morebits.wiki.api( 'Fetching parsing deletion template', query, function(apiObj) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't grammatical.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Haha, this and the comment above about is are a result of working on this during wee hours of the night

@MusikAnimal
Copy link
Collaborator Author

Fixed the bug you found, and made some copy edits. Not sure about the "open user talk page" when deleting under multiple criteria. I don't want to default to checked as some will find it annoying. If it gets unchecked when they didn't want it to they can view the deleted edits and click on the user talk page link there. I don't think we'll get many complaints.

@atlight
Copy link
Collaborator

atlight commented Nov 25, 2015

I'll just test under "radioClick" and other modes, and then am happy to merge this.

The checkboxes and controls at the top of the list for admins are starting to get a bit out of hand. It would be helpful if, rather than disabling irrelevant controls (i.e. the "tag-related options" when in delete mode, and vice versa), the irrelevant checkboxes were actually hidden altogether. This would save vertical real estate. I won't insist on it for this change though; in fact, it would probably be better to address this in a separate pull request just to keep this PR manageable.

@MusikAnimal
Copy link
Collaborator Author

Yes I was thinking the same. I was even considering making it checkbox-based like the block module. Probably easier said than done.

@MusikAnimal
Copy link
Collaborator Author

^ @atlight one more commit. Figured it's better to at least say a deletion summary couldn't be generated by the template rather than assume the admin didn't enter one. In production, it will most certainly be the latter and not some bug with the templates, but nonetheless good to be informative.

@MusikAnimal
Copy link
Collaborator Author

@atlight fixed the last 2 bugs you mentioned, I believe (some of those subgroups didn't do anything when requesting speedy in multi-mode). How do I pretend to be a sysop?

@atlight
Copy link
Collaborator

atlight commented Dec 9, 2015

How do I pretend to be a sysop?

You are a sysop, so you don't need to pretend; I, on the other hand, am not a sysop, so I run wgUserGroups.push('sysop') in the browser console so Twinkle thinks I am one.

@MusikAnimal
Copy link
Collaborator Author

FYI if you're going through and reviewing stuff, this one still needs a little work. I've just been sidetracked with other matters.

@MusikAnimal
Copy link
Collaborator Author

@atlight Just now getting back to this... I tried deleting as a non-admin but got Deleting page: Failed to delete the page: Permission denied when I submitted. Are you able to reproduce?

Also, just a general question, how do you normally do development/testing of Twinkle (or any MediaWiki JavaScript project)? I noticed you had very little Twinkle related-edits on testwiki. Are you using another wiki or just pasting in the code in the JS console on enwiki?

@MusikAnimal
Copy link
Collaborator Author

Actually maybe I fixed that bug with this. Were there any other outstanding bugs you encountered?

I've been using it regularly on enwiki without issues. In a future update I'd like to clean up the interface some as you suggest. I also had the idea that maybe we could make post-delete templates to put on user talk pages, and automate adding those. I found Template:Nn-warn-deletion and others, which is what admins would add after they've deleted a page (e.g. if user didn't issue a CSD nomination template). I am convinced many admins don't know these even exist. I will have to set it up so that we have "delete" / "add delete template to user talk page" checkboxes, just like we do on the Block module.

@MusikAnimal
Copy link
Collaborator Author

@atlight Going to merge this if there is no opposition. I've been using it for a while without issues, and will be sure to swiftly address any concerns users bring up once it's rolled out. Many thanks

@atlight
Copy link
Collaborator

atlight commented Jan 15, 2016

Feel free to merge it, but make sure you're around to fix any reported bugs :)

@MusikAnimal
Copy link
Collaborator Author

Will do. Going to be cautious and deploy this tomorrow morning (EST) when I'm going to be in front of a computer all day.

Also... still curious how you do Twinkle development. I didn't know if you knew of an easier way? I write code in my text editor, copy/paste into the relevant MediaWiki namespace file on testwiki, reload whatever page, and do testing. This is tedious. Copy/pasting into the JS console would be initially quicker, but with a refresh I'd have to copy/paste again. Chrome has some sort of "work off of filesystem" feature that maybe I should look into.

MusikAnimal added a commit that referenced this pull request Jan 15, 2016
Admins delete under multiple rationale, use template-generated delete reasons
@MusikAnimal MusikAnimal merged commit 554664b into wikimedia-gadgets:master Jan 15, 2016
@MusikAnimal MusikAnimal deleted the speedy-admins branch May 9, 2016 19:17
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Mar 17, 2019
…ick)

This solves a long-standing issue for `radioClick` fans.  Toggling between deletion and tagging modes, or turning OFF "delete/tag with multiple" would increase the number of "Submit" buttons on any subgroup with two or more inputs (currently G6 move and G12).

Per wikimedia-gadgets#300 (comment) this is only affected a handful of users.
siddharthvp pushed a commit to siddharthvp/twinkle that referenced this pull request Mar 29, 2019
…ick)

This solves a long-standing issue for `radioClick` fans.  Toggling between deletion and tagging modes, or turning OFF "delete/tag with multiple" would increase the number of "Submit" buttons on any subgroup with two or more inputs (currently G6 move and G12).

Per wikimedia-gadgets#300 (comment) this is only affected a handful of users.
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request May 28, 2019
Closes wikimedia-gadgets#520 and wikimedia-gadgets#505.  Removes the old "open talk page" sysop option and folds the non-sysop tagging system into a unified `noteToCreator` notification system for both tagfing and deleting.  Uses the existing system for tagging to determine whether or not to notify, rather than the bespoke option for `openUserTalk` from wikimedia-gadgets#300; that is, it will notify only if the corresponding box is checked AND (at least one of) the CSD in question is checked in the user preferences.

The box is checked by default, unless it finds `$('#delete-reason')` on the page, indicating some sort of XfD or CSD or the like; in the latter scenario, if the box is subsequently checked, there is an additional prompt (this might be a good candidate for removal down the line, but I think it's important to include now, especially since it's a new feature).  It uses the existing preference about whether to welcome the user alongside the notification rather than create a separate one just for post-deletion notifications.

New templates all listed at [[Template:Speedy deletion deleted]].
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request May 28, 2019
Closes wikimedia-gadgets#520 and wikimedia-gadgets#505.  Removes the old "open talk page" sysop option and folds the non-sysop tagging system into a unified `noteToCreator` notification system for both tagfing and deleting.  Uses the existing system for tagging to determine whether or not to notify, rather than the bespoke option for `openUserTalk` from wikimedia-gadgets#300; that is, it will notify only if the corresponding box is checked AND (at least one of) the CSD in question is checked in the user preferences.

The box is checked by default, unless it finds `$('#delete-reason')` on the page, indicating some sort of XfD or CSD or the like; in the latter scenario, if the box is subsequently checked, there is an additional prompt (this might be a good candidate for removal down the line, but I think it's important to include now, especially since it's a new feature).  It uses the existing preference about whether to welcome the user alongside the notification rather than create a separate one just for post-deletion notifications.

New templates all listed at [[Template:Speedy deletion deleted]].
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Feb 11, 2020
Like G7 (disabled in 7191d4c as part of wikimedia-gadgets#300), the rationales here aren't used.  G8 is different from G7, though, in that it uses `rationale` for the tag, but `summary` for the deletion log.  We could instead set `currentParams.summary` to `currentParams.rationale` for G8 (and hide on multiple?), but the reality is that G7 and G8 are two of the CSD areas that a custom summary is least useful for.  Moreover, we only have this for the generic G8 option, and doing so would mean removing the standard G8 text (barring some moderate edits to the template).  T2, on the other hand, is just like G7, in that the summary takes no parameter.

We could, of course, change either template, forcing them to take a `{{{rationale}}}` (and/or `{{{summary}}}`), but not sure that's super helpful here.
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Feb 20, 2020
This is much delayed.

`HideSubgroupWhenMultiple` was added to G5 in Oct 2013 (wikimedia-gadgets@a04f034) and to G4 two years later (Dec 2015) as part of wikimedia-gadgets#300 (wikimedia-gadgets@7191d4c).  At the time, neither was supported by {{db-multiple}}, but both now are: G5 has been supported since Feb 2017 (https://en.wikipedia.org/w/index.php?title=Template%3ADb-multiple&diff=prev&oldid=765075803) with G4 coming later in May (https://en.wikipedia.org/w/index.php?title=Template:Db-multiple&diff=prev&oldid=781332988).

Additionally, I've added support for an `xfd` parameter in the {{db-[notice|deleted]-multiple}} templates and covered that here as well; {{db-csd-[notice|deleted]-custom}} already support G4's parameter (as did Twinkle) and G5 is by definition unsupported as a notice.

Relatedly, we don't particularly need `HideSubgroupWhenSysop`, since both {{db-g4}} and {{db-g5}} make use of their respective parameters via `summary`.
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Feb 23, 2020
This is much delayed.

`HideSubgroupWhenMultiple` was added to G5 in Oct 2013 (wikimedia-gadgets@a04f034) and to G4 two years later (Dec 2015) as part of wikimedia-gadgets#300 (wikimedia-gadgets@7191d4c).  At the time, neither was supported by {{db-multiple}}, but both now are: G5 has been supported since Feb 2017 (https://en.wikipedia.org/w/index.php?title=Template%3ADb-multiple&diff=prev&oldid=765075803) with G4 coming later in May (https://en.wikipedia.org/w/index.php?title=Template:Db-multiple&diff=prev&oldid=781332988).

Additionally, I've added support for an `xfd` parameter in the {{db-[notice|deleted]-multiple}} templates and covered that here as well; {{db-csd-[notice|deleted]-custom}} already support G4's parameter (as did Twinkle) and G5 is by definition unsupported as a notice.

Relatedly, we don't particularly need `HideSubgroupWhenSysop`, since both {{db-g4}} and {{db-g5}} make use of their respective parameters via `summary`.
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Aug 31, 2020
This stems from over four years ago to wikimedia-gadgets#300 (26d974c) when the ability to delete under multiple rationales was to mirror tagging.  That being said, the user-facing description only mentioned tagging after 13ebf0e (part of (but unrelated to) wikimedia-gadgets#563).  Closes wikimedia-gadgets#1113.
wiki-ST47 pushed a commit to wiki-ST47/twinkle that referenced this pull request Sep 2, 2020
Like G7 (disabled in 7191d4c as part of wikimedia-gadgets#300), the rationales here aren't used.  G8 is different from G7, though, in that it uses `rationale` for the tag, but `summary` for the deletion log.  We could instead set `currentParams.summary` to `currentParams.rationale` for G8 (and hide on multiple?), but the reality is that G7 and G8 are two of the CSD areas that a custom summary is least useful for.  Moreover, we only have this for the generic G8 option, and doing so would mean removing the standard G8 text (barring some moderate edits to the template).  T2, on the other hand, is just like G7, in that the summary takes no parameter.

We could, of course, change either template, forcing them to take a `{{{rationale}}}` (and/or `{{{summary}}}`), but not sure that's super helpful here.
wiki-ST47 pushed a commit to wiki-ST47/twinkle that referenced this pull request Sep 2, 2020
This is much delayed.

`HideSubgroupWhenMultiple` was added to G5 in Oct 2013 (wikimedia-gadgets@a04f034) and to G4 two years later (Dec 2015) as part of wikimedia-gadgets#300 (wikimedia-gadgets@7191d4c).  At the time, neither was supported by {{db-multiple}}, but both now are: G5 has been supported since Feb 2017 (https://en.wikipedia.org/w/index.php?title=Template%3ADb-multiple&diff=prev&oldid=765075803) with G4 coming later in May (https://en.wikipedia.org/w/index.php?title=Template:Db-multiple&diff=prev&oldid=781332988).

Additionally, I've added support for an `xfd` parameter in the {{db-[notice|deleted]-multiple}} templates and covered that here as well; {{db-csd-[notice|deleted]-custom}} already support G4's parameter (as did Twinkle) and G5 is by definition unsupported as a notice.

Relatedly, we don't particularly need `HideSubgroupWhenSysop`, since both {{db-g4}} and {{db-g5}} make use of their respective parameters via `summary`.
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.

None yet

3 participants