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: A few tweaks to CSD logging #436

Merged
merged 2 commits into from
Sep 20, 2018

Conversation

Amorymeltzer
Copy link
Collaborator

Two different behaviors fixed:

  • A few edge cases exist where twinkle won't notify the original creator, but it would still erroneously say you had notified the user in your log.
  • Avoid using the title of a page nominated for G10. The name of G10 noms was removed from the notification edit summary in e296afc roughly six years ago but it was still present in the edit summary for the log. That's been fixed. This also removes the title from displaying in the log itself, hiding it behind a piped wikilink.

A few edge cases exist where twinkle won't notify the original creator,
but if you have the log turned on it would still say you notified the user.
The name of the page was removed from the notification edit summary in e296afc roughly six years ago
but it was still present in the edit summary for the log.  Moreover, this also removes the title
from displaying in the log itself, hiding it behind a piped wikilink.
@Amorymeltzer
Copy link
Collaborator Author

While I think hiding the name/link of a G10 nom in the log's edit summary should be uncontroversial, I did go out on a limb in removing the display of the page in the log. Theoretically, those things don't get viewed much unless someone is at RfA or has had a rash of poor taggings, but likewise it doesn't cost much to hide the title. The link should remain for the sake of transparency (and actually being a log!) so I think I struck a good middle ground on the abundance of caution front, but I'd appreciate at least a second on that front. Easy to remove if undesired.

Moreover, one other potential source is the notification logging itself. If the username itself is a problem, that will still get logged. I thought about adding in an additional check to change the log to say "notified the {{user|1=BadName|2=page creator}} if:

  • G10 nom
  • page nominated is User:BadName
  • page creator is User:BadName

But that seemed to be too much, and would catch too many false positives while missing a lot of cases. Not sure there's a good way around it, but wanted to float it out there regardless.

@Amorymeltzer Amorymeltzer changed the title speedy: A few tweaks to notification logging speedy: A few tweaks to CSD logging Sep 3, 2018
@MusikAnimal
Copy link
Collaborator

Looks great! I did some tests on testwiki and it checks out. Per usual I can't find anything wrong with your code. Merging!

@MusikAnimal MusikAnimal merged commit 8350509 into wikimedia-gadgets:master Sep 20, 2018
@@ -1566,7 +1569,12 @@ Twinkle.speedy.callbacks = {
appendText += "\n\n=== " + date.getUTCMonthName() + " " + date.getUTCFullYear() + " ===";
}

appendText += "\n# [[:" + Morebits.pageNameNorm + "]]: ";
appendText += "\n# [[:" + Morebits.pageNameNorm;
if (params.normalizeds.indexOf("g10") === -1) { // no article name in log for G10 taggings
Copy link
Member

Choose a reason for hiding this comment

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

@Amorymeltzer @MusikAnimal TypeError: Cannot read property 'indexOf' of undefined is raised on this line when trying to CSD a file under F11 rationale (and probably with other rationales also). First reported at TW talk page

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could've sworn I checked all the di options, but yeah, looks like I should've tested params.normalizeds first/as well. #460 should take care of it? The one below this throws as well.

Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Nov 12, 2018
Amorymeltzer added a commit that referenced this pull request Dec 10, 2018
speedy: bug fix for logging di nominations (#436)
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.
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.

None yet

3 participants