Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 47 additions & 18 deletions .github/scripts/submission.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ def __init__(self, kind):
self.title = None
self.summary = None
self.nothing_new = None
self.already_shipped = None


def read_json(path, default):
Expand All @@ -235,6 +236,11 @@ def _sets(block):
return {name: {normalize(e) for e in (block.get(name) or []) if isinstance(e, str)} for name in SPAM_LISTS}


def listing(block):
"""Entries grouped by list, one `list: entry` per line, in the order the lists are known."""
return "\n".join("%s: %s" % (name, entry) for name in SPAM_LISTS for entry in block.get(name, []))


def build_spam(issue, values, root):
result = Result("spam")
if not is_checked(values.get("privacy")):
Expand Down Expand Up @@ -281,10 +287,17 @@ def build_spam(issue, values, root):

feed = read_json(os.path.join(root, "Feed", "spam.json"), {})
listed, retracted = _sets(feed.get("lists")), _sets(feed.get("retracted"))
fresh, known, withdrawn = {}, {}, {}
# What releases shipped as built-in defaults. The feed never hands one of those over - a streamer who
# deleted one would get it back - so a default is no use to the promotion step, however good it is.
shipped_block = feed.get("shipped") if isinstance(feed.get("shipped"), dict) else {}
knows_shipped = isinstance(shipped_block.get("lists"), dict)
shipped = _sets(shipped_block.get("lists"))
fresh, known, defaults, withdrawn = {}, {}, {}, {}
for name in SPAM_LISTS:
for entry in entries.get(name, []):
if entry in listed[name] and entry not in retracted[name]:
if entry in shipped[name]:
defaults.setdefault(name, []).append(entry)
elif entry in listed[name] and entry not in retracted[name]:
known.setdefault(name, []).append(entry)
else:
fresh.setdefault(name, []).append(entry)
Expand All @@ -293,6 +306,7 @@ def build_spam(issue, values, root):

if not fresh:
result.nothing_new = known
result.already_shipped = defaults
return result

number = issue["number"]
Expand Down Expand Up @@ -320,22 +334,29 @@ def build_spam(issue, values, root):
result.title = "Spam wording from #%d (%d %s)" % (number, total, "entry" if total == 1 else "entries")

lines = ["**Spam wording** shared in #%d by %s." % (number, issue["user"]["login"]), ""]
lines += ["| List | New | Already in the feed |", "|---|---|---|"]
lines += ["| List | New | Already in the feed | A shipped default |", "|---|---|---|---|"]
for name in SPAM_LISTS:
if name in fresh or name in known:
lines.append("| `%s` | %d | %d |" % (name, len(fresh.get(name, [])), len(known.get(name, []))))
lines += ["", "New:", fence("\n".join("%s: %s" % (n, e) for n in SPAM_LISTS for e in fresh.get(n, [])))]
if name in fresh or name in known or name in defaults:
lines.append("| `%s` | %d | %d | %d |" % (name, len(fresh.get(name, [])), len(known.get(name, [])),
len(defaults.get(name, []))))
lines += ["", "New:", fence(listing(fresh))]
if known:
lines += ["Already in the feed, so left out of the file:",
fence("\n".join("%s: %s" % (n, e) for n in SPAM_LISTS for e in known.get(n, [])))]
lines += ["Already in the feed, so left out of the file:", fence(listing(known))]
if defaults:
lines += ["Built-in defaults of a TwitchSentry release, so left out of the file - the feed never hands one over:",
fence(listing(defaults))]
if withdrawn:
lines += ["**Retracted from the feed before** - kept, for you to judge:",
fence("\n".join("%s: %s" % (n, e) for n in SPAM_LISTS for e in withdrawn.get(n, [])))]
lines += ["**Retracted from the feed before** - kept, for you to judge:", fence(listing(withdrawn))]
if context:
lines += ["Where it showed up:", fence(context)]
lines += ["Checked here: the list names and the rules every install applies to an entry. "
"**Not checked:** defaults a release already shipped, and the spam corpus - "
"`tools/check-feed.ps1` covers both when this is promoted into the feed."]
if knows_shipped:
lines += ["Checked here: the list names, the rules every install applies to an entry, and the defaults "
"releases shipped. **Not checked:** the spam corpus - `tools/check-feed.ps1` runs it when this "
"is promoted into the feed."]
else:
lines += ["Checked here: the list names and the rules every install applies to an entry. "
"**Not checked:** defaults a release already shipped (`Feed/spam.json` has no `shipped` block yet), "
"and the spam corpus - `tools/check-feed.ps1` covers both when this is promoted into the feed."]
result.summary = "\n".join(lines)
return result

Expand Down Expand Up @@ -565,6 +586,17 @@ def problems_comment(result):
+ ["", "Edit the ticket to put it right, and it is checked again."])


def nothing_new_comment(result):
lines = ["Thank you - there is nothing here the spam feed could add.", ""]
if result.nothing_new:
lines += ["Already in the spam feed:", fence(listing(result.nothing_new))]
if result.already_shipped:
lines += ["Built-in defaults of a TwitchSentry release. The feed never hands one of those over, "
"so a streamer who deleted one keeps it deleted:", fence(listing(result.already_shipped))]
lines.append("This ticket can be closed.")
return "\n".join(lines)


def publish(result, issue, repo, base, root):
number = issue["number"]
branch = "submission/%d" % number
Expand Down Expand Up @@ -622,11 +654,8 @@ def main():
comment(repo, number, problems_comment(result))
return 0
if result.nothing_new is not None:
print("Ticket #%d holds nothing the feed lacks." % number)
comment(repo, number, "\n".join([
"Thank you - every entry here is already in the spam feed, so there is nothing new to add:", "",
fence("\n".join("%s: %s" % (n, e) for n in SPAM_LISTS for e in result.nothing_new.get(n, []))),
"This ticket can be closed."]))
print("Ticket #%d holds nothing the feed could add." % number)
comment(repo, number, nothing_new_comment(result))
return 0

url, created = publish(result, issue, repo, base, root)
Expand Down
44 changes: 42 additions & 2 deletions .github/scripts/test_submission.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,11 @@ def test_a_fence_outlasts_every_backtick_run_inside_it(self):

class SpamWording(unittest.TestCase):
def setUp(self):
self.root = workspace({"schema": 1, "version": 1,
self.root = workspace({"schema": 1, "version": 2,
"lists": {"spamDomains": ["smmgen"], "strongKeywords": ["botrush"]},
"retracted": {"keywords": ["old offer"]}})
"retracted": {"keywords": ["old offer"]},
"shipped": {"releases": ["v1.0.1", "v2.0.0"],
"lists": {"keywords": ["cheap"], "spamDomains": ["streamboo"]}}})

def build(self, entries, **kw):
return s.build("spam", ticket("spam", spam_pairs(entries, **kw)), self.root)
Expand All @@ -157,6 +159,31 @@ def test_nothing_new_opens_no_pull_request(self):
self.assertIsNone(r.document)
self.assertEqual(r.nothing_new, {"spamDomains": ["smmgen"], "strongKeywords": ["botrush"]})

def test_defaults_a_release_shipped_are_left_out(self):
# An install that started on an older version still carries that version's defaults, so the
# window may offer one; the feed could never hand it over.
r = self.build("keywords: !Cheap\nspamDomains: streamboo\nspamDomains: newsite")
self.assertEqual(r.document["entries"], {"spamDomains": ["newsite"]})
self.assertIn("| `keywords` | 0 | 0 | 1 |", r.summary)
self.assertIn("Built-in defaults of a TwitchSentry release", r.summary)
self.assertIn("spamDomains: streamboo\nkeywords: cheap", r.summary)
self.assertIn("and the defaults releases shipped", r.summary)

def test_a_default_is_not_a_default_in_another_list(self):
r = self.build("strongKeywords: cheap")
self.assertEqual(r.document["entries"], {"strongKeywords": ["cheap"]})

def test_nothing_but_known_entries_and_defaults_opens_no_pull_request(self):
r = self.build("spamDomains: smmgen\nkeywords: cheap")
self.assertIsNone(r.document)
self.assertEqual(r.nothing_new, {"spamDomains": ["smmgen"]})
self.assertEqual(r.already_shipped, {"keywords": ["cheap"]})

def test_a_feed_without_the_shipped_block_is_not_claimed_to_be_checked(self):
r = s.build("spam", ticket("spam", spam_pairs("spamDomains: newsite")), workspace())
self.assertIn("has no `shipped` block yet", r.summary)
self.assertNotIn("and the defaults releases shipped", r.summary)

def test_an_entry_retracted_before_is_kept_and_pointed_out(self):
r = self.build("keywords: old offer")
self.assertEqual(r.document["entries"], {"keywords": ["old offer"]})
Expand Down Expand Up @@ -370,6 +397,19 @@ def test_an_edit_that_changes_nothing_pushes_nothing_and_says_nothing(self):
"gh pr list", "gh pr edit"])
self.assertEqual(rec.calls[-1][3], "7")

def test_a_ticket_the_feed_can_add_nothing_from_only_gets_a_comment_saying_why(self):
root = workspace({"schema": 1, "version": 2, "lists": {"spamDomains": ["smmgen"]}, "retracted": {},
"shipped": {"releases": ["v2.0.0"], "lists": {"keywords": ["cheap"]}}})
issue = ticket("spam", spam_pairs("spamDomains: smmgen\nkeywords: cheap"))
code, rec, _ = self.run_main(issue, [], root)
self.assertEqual(code, 0)
self.assertEqual(rec.commands(), ["gh issue comment"])
with open(rec.calls[0][rec.calls[0].index("--body-file") + 1], encoding="utf-8") as f:
body = f.read()
self.assertIn("Already in the spam feed:\n``` text\nspamDomains: smmgen\n```", body)
self.assertIn("Built-in defaults of a TwitchSentry release", body)
self.assertIn("``` text\nkeywords: cheap\n```", body)

def test_a_ticket_with_mistakes_only_gets_a_comment(self):
issue = ticket("spam", spam_pairs("spamDomains: smm gen"))
_, rec, _ = self.run_main(issue, [])
Expand Down
10 changes: 10 additions & 0 deletions Feed/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ Two files on your PC belong to the feed:
},
"retracted": {
"spamDomains": ["an-entry-that-was-a-mistake"]
},
"shipped": {
"releases": ["v1.0.1", "v2.0.0"],
"lists": {
"keywords": ["a-built-in-default"]
}
}
}
```
Expand All @@ -47,6 +53,10 @@ Two files on your PC belong to the feed:
offline for a month catches up in one go.
- `retracted` takes an entry back from installs that got it from the feed. An entry a streamer
marked with `!` stays, because it is theirs now.
- `shipped` names every entry a release shipped as a built-in default, and the releases counted.
Installs never hand one of these over, even if `lists` carried it by mistake: a streamer who
deleted a default keeps it deleted. The settings window does not offer them for sharing either,
and a share ticket that holds one leaves it out.
- The lists: `spamDomains`, `strongKeywords`, `keywords`, `spacedUrlTlds`, `beatRapport`,
`beatCritique`, `beatSolution`, `beatPitch`, `handoffPhrases`, `serviceOffers`.

Expand Down
Loading