Always set new_blogurl if blogurl different from feed.link

Previously, we set blogurl if the prior blogurl was set to empty-string
('') but that change wasn't picked up as part of what needed to be
updated, so instead always use new_blogurl if we need to set the blogurl
as this ensures that we'll realize there was a change and will update
the database accordingly.

At the same time, also send an email in every case that the blogurl is
changed, even when it's being changed from being empty.  This is perhaps
a bit noisy but it's been broken for so long that it's worth it to make
sure people are aware that it's now working and folks don't register new
blogs over and over again anyway.

Lastly, move the block that does the update and sets the new blogurl
value up higher so that if a new blogurl is set then the emails that are
generated from that run will use that new blogurl instead of the old
one (which could be confusing).

2 files changed, 31 insertions, 28 deletions

diff --git a/hamnadmin/hamnadmin/register/management/commands/aggregate_feeds.py b/hamnadmin/hamnadmin/register/management/commands/aggregate_feeds.py
index a4f6a85..703a14f 100644
--- a/hamnadmin/hamnadmin/register/management/commands/aggregate_feeds.py
+++ b/hamnadmin/hamnadmin/register/management/commands/aggregate_feeds.py
@@ -114,6 +114,33 @@ class Command(BaseCommand):
                         feed.lastsuccess = datetime.now()
                         feed.save(update_fields=['lastsuccess'])
 
+                        # If the blog URL changed, update it as requested
+                        # Do this here so that the new blogurl is used for emails below.
+                        if getattr(feed, 'new_blogurl', None):
+                            self.trace("URL changed for %s to %s" % (feed.feedurl, feed.new_blogurl))
+                            send_simple_mail(
+                                settings.EMAIL_SENDER,
+                                settings.NOTIFICATION_RECEIVER,
+                                "A blog url changed on Planet PostgreSQL",
+                                "When checking the blog at {0} by {1}\nthe blog URL was updated to:\n{2}\n(from previous value {3})\n\nTo moderate: https://planet.postgresql.org/register/moderate/\n\n".format(feed.feedurl, feed.user, feed.new_blogurl, feed.blogurl),
+                                sendername="Planet PostgreSQL",
+                                receivername="Planet PostgreSQL Moderators",
+                            )
+                            send_simple_mail(
+                                settings.EMAIL_SENDER,
+                                feed.user.email,
+                                "URL of your blog at Planet PostgreSQL updated",
+                                "The blog aggregator at Planet PostgreSQL has update the URL of your blog\nwith the feed at {0} to:\n{1} (from {2})\nIf this is correct, you don't have to do anything.\nIf not, please contact planet@postgresql.org\n".format(
+                                    feed.feedurl,
+                                    feed.new_blogurl,
+                                    feed.blogurl,
+                                ),
+                                sendername="Planet PostgreSQL",
+                                receivername="{0} {1}".format(feed.user.first_name, feed.user.last_name),
+                            )
+                            feed.blogurl = feed.new_blogurl
+                            feed.save(update_fields=['blogurl'])
+
                         for entry in results:
                             self.trace("Found entry at %s" % entry.link)
                             # Entry is a post, but we need to check if it's already there. Check
@@ -194,31 +221,6 @@ class Command(BaseCommand):
                                 receivername="Planet PostgreSQL Moderators",
                             )
 
-                        # If the blog URL changed, update it as requested
-                        if getattr(feed, 'new_blogurl', None):
-                            self.trace("URL changed for %s to %s" % (feed.feedurl, feed.new_blogurl))
-                            send_simple_mail(
-                                settings.EMAIL_SENDER,
-                                settings.NOTIFICATION_RECEIVER,
-                                "A blog url changed on Planet PostgreSQL",
-                                "When checking the blog at {0} by {1}\nthe blog URL was updated to:\n{2}\n(from previous value {3})\n\nTo moderate: https://planet.postgresql.org/register/moderate/\n\n".format(feed.feedurl, feed.user, feed.new_blogurl, feed.blogurl),
-                                sendername="Planet PostgreSQL",
-                                receivername="Planet PostgreSQL Moderators",
-                            )
-                            send_simple_mail(
-                                settings.EMAIL_SENDER,
-                                feed.user.email,
-                                "URL of your blog at Planet PostgreSQL updated",
-                                "The blog aggregator at Planet PostgreSQL has update the URL of your blog\nwith the feed at {0} to:\n{1} (from {2})\nIf this is correct, you don't have to do anything.\nIf not, please contact planet@postgresql.org\n".format(
-                                    feed.feedurl,
-                                    feed.new_blogurl,
-                                    feed.blogurl,
-                                ),
-                                sendername="Planet PostgreSQL",
-                                receivername="{0} {1}".format(feed.user.first_name, feed.user.last_name),
-                            )
-                            feed.blogurl = feed.new_blogurl
-                            feed.save(update_fields=['blogurl'])
                 if self.debug:
                     # Roll back transaction without error
                     raise BreakoutException()
diff --git a/hamnadmin/hamnadmin/util/aggregate.py b/hamnadmin/hamnadmin/util/aggregate.py
index 226fce5..6615e11 100644
--- a/hamnadmin/hamnadmin/util/aggregate.py
+++ b/hamnadmin/hamnadmin/util/aggregate.py
@@ -56,10 +56,11 @@ class FeedFetcher(object):
 
         self._trace("Fetched %s, status %s" % (self.feed.feedurl, parser.status))
 
+        # Use an exception block in case parser.feed.link is not in the feed for
+        # some reason.  Caller will pick up on new_blogurl being set and will be
+        # sure to send an email about the change and to get it saved to the database.
         try:
-            if self.feed.blogurl == '':
-                self.feed.blogurl = parser.feed.link
-            elif self.feed.blogurl != parser.feed.link:
+            if self.feed.blogurl != parser.feed.link:
                 self.feed.new_blogurl = parser.feed.link
         except Exception as e:
             self._trace("Exception when setting blogurl from parser.feed.link: %s" % e)