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

speedy: New option for sysops: notify creator upon deletion #563

Merged

Conversation

Amorymeltzer
Copy link
Collaborator

@Amorymeltzer Amorymeltzer commented Mar 24, 2019

Closes #520 and #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 #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
Copy link
Collaborator Author

Amorymeltzer commented Mar 24, 2019

@MusikAnimal There's some more tidying I can do — one or two of the variables are unnecessary, but helpful for clarity when reviewing — but here's the draft code for #520. I think I explain the changes in the commit message well enough to reply to the points there, but to be explicit:

  • It takes the system for notifying users of a tag and uses that for both notifying (tag) or warning (delete). The box must be checked and the corresponding csd must be checked in your preferences.
  • Does away with the openUserTalk option after deleting a page, and likewise the routine you wrote to process the checkbox.
  • Uses warnings of the same naming style as the notifications (e.g. db-reason-warndb-reason-deleted to match db-reason-notice)

I think using the same structure of templates as we do for notifications but keeping them separate should make things simpler and easier for others to use, but that can be changed I suppose. Regardless, whatever we decide, I'll go and make all the templates. A few exist under a different naming scheme (see Category:CSD_warning_templates) but I think it'd be fine to move those to a more unified name.

More to the point, the larger issue is default behavior. As of now, the checkbox defaults to being checked if there isn't $('#delete-reason') on the page, and I sort of half-assed looked over the default items to be checked in your preferences. I think what should be done, since this will be a big change in behavior, is to ask two questions at AN:

  • Should the box be checked or unchecked by default?
  • What should the default set of criteria to notify include?
    We can discuss here and crib off the other defaults, but I think that will need broader input. Can also ask about moving the above templates if need be.

@Amorymeltzer
Copy link
Collaborator Author

Amorymeltzer commented Apr 29, 2019

@MusikAnimal I'll open a discussion on WP:AN in the next day or two, I want to get back into this.

27 hours later: https://en.wikipedia.org/wiki/Wikipedia:Administrators%27_noticeboard#Should_Twinkle_warn_after_CSD_deletion?_And_default_behavior

@Amorymeltzer Amorymeltzer added this to the June update milestone May 5, 2019
@Amorymeltzer Amorymeltzer marked this pull request as ready for review May 8, 2019 17:23
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request May 10, 2019
…iff content hidden

Fixes wikimedia-gadgets#627.  If a user has "Do not show page content below diffs" selected in Special:Preferences, `wgRevisionId` will be set to 0 on diffs, meaning no links would be shown on diffs.  Comparing `wgRevisionId` to `wgDiffNewId` from wikimedia-gadgets#563 appears to be a holdover from some (local) attempts to handle merge conflicts between wikimedia-gadgets#511 and wikimedia-gadgets#407, when I initially had this structured differently, but that should now handled in the init, so this was excessive.
@Amorymeltzer Amorymeltzer force-pushed the csd-deleteandtag branch 3 times, most recently from 44ec0f6 to df19692 Compare May 16, 2019 19:39
@Amorymeltzer
Copy link
Collaborator Author

@MusikAnimal I'm gonna start creating the templates in the next few days, I've decided to go with {{db-g12-deleted}} (instead of {{db-g12-warn}}) to match {{db-g12-notice}}, how does that sound?

@MusikAnimal
Copy link
Collaborator

@MusikAnimal I'm gonna start creating the templates in the next few days, I've decided to go with {{db-g12-deleted}} (instead of {{db-g12-warn}}) to match {{db-g12-notice}}, how does that sound?

That sounds fine to me! If people don't like it they can redirect the templates and we'll change Twinkle at leisure. Thanks for your work on this!

@Amorymeltzer
Copy link
Collaborator Author

Workflow at User:Amorymeltzer/sandbox/tw fwiw

@Amorymeltzer Amorymeltzer force-pushed the csd-deleteandtag branch 2 times, most recently from 1096d6e to f0d2f69 Compare May 27, 2019 02:38
@Amorymeltzer
Copy link
Collaborator Author

@MusikAnimal FYI I pushed these changes live to testwiki in case you want to take a spin. I haven't looked at or touched the code much in weeks, and will hopefully do some more thorough tests in any spare time the next few days, but it seems to work just dandy.

@MusikAnimal
Copy link
Collaborator

@MusikAnimal FYI I pushed these changes live to testwiki in case you want to take a spin. I haven't looked at or touched the code much in weeks, and will hopefully do some more thorough tests in any spare time the next few days, but it seems to work just dandy.

Looking good!

A few possible issues:

  • When I chose "Tag page only" with "Notify page creator" checked, it did not notify the user. I used G11, and I have my Twinkle preferences set to notify for this criteria.
  • With this notification it did not create a new heading for May 2019.
  • Minor nitpick... wouldn't the word "notify" be more fitting in this context than "warn"? E.g. "Notify page creator of page deletion" instead of "Warn page creator of page deletion". The templates (at least the ones I checked) do actually warn the user about recreating the page, but the main idea here I think is to simply let the user know you deleted the page and why.

@Amorymeltzer
Copy link
Collaborator Author

Amorymeltzer commented May 28, 2019

When I chose "Tag page only" with "Notify page creator" checked, it did not notify the user.

@MusikAnimal Heh, I think you ran into what I did for a while with some xfd stuff: User:MusikAnimal created the page User:MusikPuppet, so it skipped notifying yourself. Trying it on User:MusikPuppet/test should work, as it just did for me (see also below).

With this notification it did not create a new heading for May 2019.

That's because the new templates don't exist on testwiki. If you copy-paste the wikitext it added to the talk page ({{subst:db-csd-deleted-custom|1=User:MusikPuppet/test|2=notwebhost}}) to something on en.wiki, it should be fine. I didn't go through the process of importing them some or all of them, but I suppose I should.

Minor nitpick...

100%, thanks. I've got a few other language tweaks to do, this is one of them. I made a distinction in the code between being "notified" and "warned" so it was clear which was which, and as the seven existing post-deletion templates were all named criterion-warn-deletion, e.g. Template:Nn-warn-deletion. I dropped that language when making the templates for exactly that reason, favoring {{db-spam-deleted}} over {{db-spam-warn}} or {{db-spam-warning}}, but need to go back here and adjust the front-facing language.

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
Copy link
Collaborator Author

@MusikAnimal Tweaked language (including commit message), look good? https://github.com/azatoth/twinkle/compare/f0d2f6986bd972d4fd29ffac93cbfd98039704c0..e82e7346cb7f0ff1b00b2b36e1912cd9d10e6d86

Also, I changed the default list to just match what we do for tagging notifications (https://github.com/azatoth/twinkle/compare/e82e7346cb7f0ff1b00b2b36e1912cd9d10e6d86..13ebf0e1784f235544c044d61b66efe8c051f115); that's adding A2, A5, F1, F2, F10, T3, and P2 from the openUserTalk system. Seemed simpler, and more likely to be expected behavior.

@Amorymeltzer Amorymeltzer changed the title speedy: New option for sysops: notify creator upon deletion. speedy: New option for sysops: notify creator upon deletion May 28, 2019
@MusikAnimal
Copy link
Collaborator

I figured my testing was flawed. Those bugs I found were too obvious for you to have missed :)

👍 and a 🍺 from me. I admit I have not done a thorough code review so I'll leave merging to you or whoever else. I'm sure admins are going to really enjoy this feature!

@Amorymeltzer
Copy link
Collaborator Author

I think what I'll do is push the twinkle.js and twinkleconfig.js changes in the next day or two and leave a note in the Admin Newsletter that the feature is coming soon (I'm shooting for the 12th for next on-wiki update) but they can update preferences now.

@Amorymeltzer Amorymeltzer merged commit 6891161 into wikimedia-gadgets:master May 30, 2019
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Feb 24, 2020
Since wikimedia-gadgets#578, we've been trying to correctly note in the CSD log whether the user notification went successfully.  That was carried over into wikimedia-gadgets#563, but the changes in wikimedia-gadgets#578 meant that if the user attempted to notify the creator (e.g. `params.usertalk`) but then discovered (in `noteToCreator`) that there was a good reason not to, no CSD log entry was made; it accounted for whether you wanted to notify-then-log or just log, but didn't account for notify-but-notify-not-done-then-log, essentially ignoring the checks from wikimedia-gadgets#436.  Specific cases, like R2 (reported at WT:TW http://en.wikipedia.org/wiki/Special:Permalink/942392393#CSD_R2_instances_not_being_logged), were especially prone to being missed as a notification.

This uses those `initialContrib = null` checks to recheck whether a notification should go out, and if not, log if appropriate.
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Feb 24, 2020
Since wikimedia-gadgets#578, we've been trying to correctly note in the CSD log whether the user notification went successfully.  That was carried over into wikimedia-gadgets#563, but the changes in wikimedia-gadgets#578 meant that if the user attempted to notify the creator (e.g. `params.usertalk`) but then discovered (in `noteToCreator`) that there was a good reason not to, no CSD log entry was made; it accounted for whether you wanted to notify-then-log or just log, but didn't account for notify-but-notify-not-done-then-log, essentially ignoring the checks from wikimedia-gadgets#436.  Specific cases, like R2 (reported at WT:TW http://en.wikipedia.org/wiki/Special:Permalink/942392393#CSD_R2_instances_not_being_logged), were especially prone to being missed in the log.

This uses those `initialContrib = null` checks to recheck whether a notification should go out, and if not, log if appropriate.
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
Since wikimedia-gadgets#578, we've been trying to correctly note in the CSD log whether the user notification went successfully.  That was carried over into wikimedia-gadgets#563, but the changes in wikimedia-gadgets#578 meant that if the user attempted to notify the creator (e.g. `params.usertalk`) but then discovered (in `noteToCreator`) that there was a good reason not to, no CSD log entry was made; it accounted for whether you wanted to notify-then-log or just log, but didn't account for notify-but-notify-not-done-then-log, essentially ignoring the checks from wikimedia-gadgets#436.  Specific cases, like R2 (reported at WT:TW http://en.wikipedia.org/wiki/Special:Permalink/942392393#CSD_R2_instances_not_being_logged), were especially prone to being missed in the log.

This uses those `initialContrib = null` checks to recheck whether a notification should go out, and if not, log if appropriate.
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.

Speedy: Allow sysops to issue warning upon csd
2 participants