If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in 1.2

Status

Webtools
Elmo
P3
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Milos, Assigned: gandalf)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
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.
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.
(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.
Summary: [dashboard][webby] Add a view for pending opt-ins and RSS for that page → [dashboard][webby] Add a syndication feed for pending opt-ins

Updated

7 years ago
Component: Infrastructure → Elmo
Product: Mozilla Localizations → Webtools
QA Contact: infrastructure → elmo
Summary: [dashboard][webby] Add a syndication feed for pending opt-ins → [webby] Add a syndication feed for pending opt-ins
Version: unspecified → 1.0

Updated

7 years ago
Priority: -- → P3
Whiteboard: [good first bug]
(Assignee)

Updated

7 years ago
Assignee: nobody → gandalf
(Assignee)

Comment 3

7 years ago
Created attachment 530283 [details] [diff] [review]
patch, v1

patch adds two feeds - one for all new weblocales, one for just the pending ones
Attachment #530283 - Flags: review?(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.
Attachment #530283 - Flags: review?(stas) → review-
(Assignee)

Comment 5

7 years ago
Created attachment 530286 [details] [diff] [review]
patch, v2

thanks stas!

patch, v2: follow the feedback
Attachment #530283 - Attachment is obsolete: true
Attachment #530286 - Flags: review?(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?
Attachment #530286 - Flags: review?(stas) → review+
(Assignee)

Comment 7

7 years ago
remove Feed inheritance per comment 6
pushed https://github.com/mozilla/elmo/commit/741a365548413da1e979c477122a4126c6db6dfc
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Comment 8

7 years ago
aaand backported to master: https://github.com/mozilla/elmo/commit/638ca2b87887c34d26b02152b79c4ebd87d6a2cf
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.*.
Attachment #530310 - Flags: review?(gandalf)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
(Assignee)

Comment 11

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

yeah ://
Attachment #530310 - Flags: review?(gandalf) → review+
(Assignee)

Comment 12

7 years ago
fixed https://github.com/mozilla/elmo/commit/9123ff11e32c3766db724ea6273d39c5c24348bd
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Target Milestone: --- → 1.2
You need to log in before you can comment on or make changes to this bug.