Skip to content

Commit

Permalink
AutoMod: improved handling for media data
Browse files Browse the repository at this point in the history
There were several issues occuring related to rules that require data
from the media embed (which is added on to Link objects asynchronously
by the media scraper). Items are re-checked by AutoMod when the scraper
adds an item, but some rules that required that data were still being
checked and executed anyway before it had actually completed, so were
not behaving correctly.

This change makes the checks to see if the rule needs the data from
the media object more robust, which should ensure that these rules are
never processed until the item comes into the queue again when the
scraper attaches the embed data.
  • Loading branch information
Deimos committed Apr 13, 2015
1 parent d55c468 commit a20d710
Showing 1 changed file with 41 additions and 20 deletions.
61 changes: 41 additions & 20 deletions r2/r2/lib/automoderator.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,23 +100,25 @@ def replace_placeholders(string, data, matches):
"{{kind}}": "comment",
"{{title}}": data["link"].title,
})
media_item = data["link"]
elif isinstance(item, Link):
replacements.update({
"{{kind}}": "submission",
"{{domain}}": item.link_domain(),
"{{title}}": item.title,
"{{url}}": item.url,
})

if item.media_object:
oembed = item.media_object.get("oembed")
if oembed:
replacements.update({
"{{media_author}}": oembed.get("author_name", ""),
"{{media_title}}": oembed.get("title", ""),
"{{media_description}}": oembed.get("description", ""),
"{{media_author_url}}": oembed.get("author_url", ""),
})
media_item = item

if media_item.media_object:
oembed = media_item.media_object.get("oembed")
if oembed:
replacements.update({
"{{media_author}}": oembed.get("author_name", ""),
"{{media_title}}": oembed.get("title", ""),
"{{media_description}}": oembed.get("description", ""),
"{{media_author_url}}": oembed.get("author_url", ""),
})

for placeholder, replacement in replacements.iteritems():
string = string.replace(placeholder, replacement)
Expand Down Expand Up @@ -689,22 +691,23 @@ def get_match_patterns(self, values):

return match_patterns

def item_has_necessary_data(self, item):
"""Check whether the item has all data required by the conditions."""
@property
def needs_media_data(self):
"""Whether the component requires data from the media embed."""
if any(field.startswith("media_") for field in self.match_fields):
if not item.media_object:
return False
return True

if self.reports and not item.reported:
return False
# check if any of the fields that support placeholders have media ones
potential_placeholders = [self.report_reason]
if self.set_flair:
potential_placeholders.extend(self.set_flair.values())
if any(text and "{{media_" in text for text in potential_placeholders):
return True

return True
return False

def check_item(self, item, data):
"""Return whether an item satisfies all of the defined conditions."""
if not self.item_has_necessary_data(item):
return False

if not self.check_nonpattern_conditions(item, data):
return False

Expand Down Expand Up @@ -1177,6 +1180,19 @@ def is_unrepeatable(self):

return False

@property
def needs_media_data(self):
"""Whether the rule requires data from the media embed."""
for attr in ("comment", "modmail", "message"):
text = getattr(self, attr, None)
if text and "{{media_" in text:
return True

if any(comp.needs_media_data for comp in self.targets.values()):
return True

return False

@property
def matches(self):
return self.targets["base"].matches
Expand Down Expand Up @@ -1248,6 +1264,11 @@ def should_check_item(self, item, data):
ban_info.get("unbanner") != ACCOUNT.name):
return False

if self.needs_media_data:
if isinstance(item, Link) and not item.media_object:
return False
elif isinstance(item, Comment) and not data["link"].media_object:
return False

return True

Expand Down

0 comments on commit a20d710

Please sign in to comment.