640410 - [webby] Add a syndication feed for pending opt-ins

Status: RESOLVED FIXED

Description

[Milos Dinic [:Milos]]

Although it's all said in Summary field, I'll say it once more:

We should have a view for all pending opt-ins, and a RSS so we can act fast.

Comment 1

[Staś Małolepszy :stas]

I think this should be two bugs.  

1. a view for pending opt-ins -- I'm not sure what a pending opt-in is as opposed to a 'satisfied' opt-in.  Maybe it's (not in_verbatim or not in_vcs)?

2. an RSS feed.  That's actually simpler, because you just need to show newly created Weblocale objects.

Comment 2

[Staś Małolepszy :stas]

(In reply to comment #1)
> I think this should be two bugs.  
> 
> 1. a view for pending opt-ins -- I'm not sure what a pending opt-in is as
> opposed to a 'satisfied' opt-in.  Maybe it's (not in_verbatim or not in_vcs)?

Gandalf filed bug 641474 for this.

> 2. an RSS feed.  That's actually simpler, because you just need to show newly
> created Weblocale objects.

Let's focus on the syndication feed part in this bug.

Comment 3

[Zibi Braniecki [:gandalf][:zibi]]

Created attachment 530283 [details] [diff] [review]
patch, v1

patch adds two feeds - one for all new weblocales, one for just the pending ones

Comment 4

[Staś Małolepszy :stas]

Comment on attachment 530283 [details] [diff] [review]
patch, v1

Review of attachment 530283 [details] [diff] [review]:

::: apps/webby/feeds.py
@@ +1,2 @@
+from django.contrib.syndication.views import Feed
+from webby.models import Weblocale

Nit: put this import after a blank line below all from django.*

@@ +22,5 @@
+                                      item.project.name, item.requestee)
+
+    def item_description(self, item):
+        elems = []
+        for k,v in targets.items():

Nit: PEP8 recommends a space between k and v.

@@ +31,5 @@
+        else:
+            return 'Fully deployed'
+
+    def item_link(self, item):
+        return reverse('webby-project', kwargs={'slug':item.project.slug})

Nit: a space after the colon.

@@ +33,5 @@
+
+    def item_link(self, item):
+        return reverse('webby-project', kwargs={'slug':item.project.slug})
+
+class PendingOptinsFeed(Feed):

It looks like PendingOptinsFeed is a good candidate to be a subclass of AllOptinsFeed.  I'll r- the patch for this reason.

Comment 5

[Zibi Braniecki [:gandalf][:zibi]]

Created attachment 530286 [details] [diff] [review]
patch, v2

thanks stas!

patch, v2: follow the feedback

Comment 6

[Staś Małolepszy :stas]

Comment on attachment 530286 [details] [diff] [review]
patch, v2

Review of attachment 530286 [details] [diff] [review]:

r=me with a question below.

::: apps/webby/feeds.py
@@ +36,5 @@
+    def item_link(self, item):
+        return reverse('webby-project', kwargs={'slug': item.project.slug})
+
+
+class PendingOptinsFeed(AllOptinsFeed, Feed):

Python's MRO can be confusing, do you think it would be better to inherit from AllOptinsFeed only here?

Comment 7

[Zibi Braniecki [:gandalf][:zibi]]

remove Feed inheritance per comment 6
pushed https://github.com/mozilla/elmo/commit/741a365548413da1e979c477122a4126c6db6dfc

Comment 8

[Zibi Braniecki [:gandalf][:zibi]]

aaand backported to master: https://github.com/mozilla/elmo/commit/638ca2b87887c34d26b02152b79c4ebd87d6a2cf

Comment 9

[Staś Małolepszy :stas]

Created attachment 530310 [details] [diff] [review]
Bustage fix on master

We need a bustage patch on master to make the feeds work on Django 1.1.*.

Comment 10

[Staś Małolepszy :stas]

On Django 1.1.x, items' titles and descriptions are rendered with html templates, which I didn't add.  As a result, on 1.1.x the feed is a simple list of repr(object), which is, I reckon, good enough for the backporting.

Comment 11

[Zibi Braniecki [:gandalf][:zibi]]

Comment on attachment 530310 [details] [diff] [review]
Bustage fix on master

yeah ://

Comment 12

[Zibi Braniecki [:gandalf][:zibi]]

fixed https://github.com/mozilla/elmo/commit/9123ff11e32c3766db724ea6273d39c5c24348bd