Merge pull request #193 from uchicago-sg/add-edit-button

Add support for the edit button

caravel/controllers/custom_fields.py

@@ -1,6 +1,8 @@
 from google.appengine.api import users
 
 import re
+import threading
+
 import wtforms
 from wtforms import widgets, ValidationError
 
@@ -19,6 +21,26 @@ def __call__(self, form, field):
         if not field.data or not field.data.valid:
             raise ValidationError("Invalid credential.")
 
+class MatchesPrincipal(threading.local):
+    """Ensures that only the existing Principal is used."""
+
+    def __init__(self):
+        self.principal = None
+
+    def __call__(self, form, field):
+        """Ensure that only the given user is used for authentication."""
+
+        if not self.principal:
+            raise Exception("No principal specified.")
+
+        if not field.data or not field.data.can_act_as(self.principal):
+            if not field.data:
+                raise ValidationError("Please enter an email.")
+            elif field.data.email == self.principal.email:
+                raise ValidationError("Please sign in with your CNetID.")
+            else:
+                raise ValidationError("Please use the same account for edits.")
+
 class PrincipalField(wtforms.StringField):
     """A PrincipalField is one that represents an email address."""
 
@@ -81,7 +103,7 @@ def __call__(self, **kwargs):
                 )
 
             return Markup(
-                "<div><a href='{}' class='btn btn-success'>Sign in with "
+                "<div><a href='{}' class='signin btn btn-success'>Sign in with "
                 "CNetID</a>{}</div>"
             ).format(users.create_login_url(request.url), alternative)
 

caravel/controllers/forms.py

@@ -4,7 +4,7 @@
                             DecimalField, FieldList, FormField, TextAreaField)
 from wtforms.widgets import CheckboxInput, ListWidget
 from wtforms.validators import DataRequired, Email
-from caravel.controllers.custom_fields import (PrincipalField, PhotoField,                                                     OnlyValid)
+from caravel.controllers.custom_fields import (PrincipalField, PhotoField,                                                     OnlyValid, MatchesPrincipal)
 from caravel import model
 
 class CheckboxesField(SelectMultipleField):
@@ -33,11 +33,11 @@ class ListingForm(flask_wtf.Form):
 
 
 class NewListingForm(ListingForm):
-    principal = PrincipalField("Seller",
-        validators=[DataRequired(), OnlyValid()])
+    principal = PrincipalField("Seller", validators=[DataRequired()])
     submit = SubmitField("Create")
 
 class EditListingForm(ListingForm):
+    principal = PrincipalField("Seller", validators=[MatchesPrincipal()])
     submit = SubmitField("Update")
 
 class InquiryForm(flask_wtf.Form):

caravel/controllers/listings.py

@@ -9,6 +9,7 @@
 
 from caravel import app, model, utils
 from caravel.controllers import forms
+from caravel.controllers import custom_fields
 
 from google.appengine.api import users
 
@@ -149,7 +150,10 @@ def show_listing(listing):
         inquiry = model.UnapprovedInquiry(listing=listing.key)
         form.populate_obj(inquiry)
         inquiry.put()
-        flash("Your inquiry has been sent.")
+        if isinstance(inquiry, model.UnapprovedInquiry):
+            flash("Your inquiry has been recorded and is awaiting moderation.")
+        else:
+            flash("Your inquiry has been sent.")
         return redirect(url_for("show_listing", listing=listing))
 
     return render_template("listing_show.html", listing=listing, form=form)
@@ -163,12 +167,26 @@ def edit_listing(listing):
     if is_from_tor():
         abort(403)
 
-    form = forms.EditListingForm(obj=listing)
+    # FIXME: Clean up this logic. We intentionally don't want to expose the
+    # creator of a listing.
+    _principal, listing.principal = listing.principal, None 
+    try:
+        form = forms.EditListingForm(obj=listing)
+    finally:
+        listing.principal = _principal
+
+    # Ensure that we use the same principal for updates.
+    form.principal.validators[-1].principal = listing.principal
 
     if form.validate_on_submit():
+        listing = model.UnapprovedListing(id=listing.key.id(), version=11)
         form.populate_obj(listing)
         listing.put()
-        flash("Your listing has been created.")
+        if isinstance(listing, model.Listing):
+            flash("Your listing has been updated.")
+        else:
+            flash("Your edit is awaiting moderation. "
+                  "We'll email you when it is approved.")
         return redirect(url_for("show_listing", listing=listing))
 
     return render_template("listing_form.html", form=form)

caravel/model/migration.py

@@ -62,4 +62,4 @@ def _from_pb(klass, *vargs, **kwargs):
             entity.version = 0
         entity.run_migrations()
         return entity
-    
+

caravel/static/default.css

@@ -247,6 +247,10 @@ form .thumbnail {
     border-radius: 5px;
 }
 
+.btn.signin {
+    margin-bottom: 3px;
+}
+
 /* About and show page*/
 .help-page-tall{
     height: 25em;

caravel/templates/listing_form.html

@@ -11,6 +11,10 @@
       <div class="col-xs-12">
         <form class="form" method="post" role="form"
             enctype="multipart/form-data">
+          <p>BSD/Medicine/Medical School affiliates: if you are unable to sign
+             in with your CNetID, please enter your email in the box. Your
+             listing might not be posted immediately. We reserve the right to
+             remove listings at any time.</p>
           {{ form.hidden_tag() }}
           {{ wtf.form_errors(form, hiddens="only") }}
           {{ wtf.form_field(form.title) }}

caravel/tests/helper.py

@@ -47,7 +47,7 @@ def setUp(self):
             body=u"Body of \u2606A",
             posted_at=datetime.datetime.now() - datetime.timedelta(hours=4),
             principal=utils.Principal("seller-a@uchicago.edu", device,
-                                      "GOOGLE_APPS"),
+                                      utils.Principal.GOOGLE_APPS),
             run_trigger=True,
             version=11,
             price=3.10,
@@ -61,7 +61,7 @@ def setUp(self):
             body=u"Body of \u2606B",
             posted_at=datetime.datetime.now() - datetime.timedelta(hours=24),
             principal=utils.Principal("seller-b@uchicago.edu", device,
-                                      "GOOGLE_APPS"),
+                                      utils.Principal.GOOGLE_APPS),
             price=71.10,
             run_trigger=True,
             version=11,

caravel/tests/test_listings.py

@@ -42,10 +42,10 @@ def test_post_inquiry(self):
 
         # Even so, ensure that it seems as though it got through.
         self.assertEqual(self.clean(self.get("/listing_b").data),
-            "New Listing Your inquiry has been sent. Listing \xe2\x98\x86B "
-            "apartments Posted 2d ago by [address hidden] ( sign in to view). "
-            "Price: $71.10 Body of \xe2\x98\x86B Contact Seller From Sign in "
-            "with CNetID or Message")
+            "New Listing Your inquiry has been recorded and is awaiting "
+            "moderation. Listing \xe2\x98\x86B apartments Posted 2d ago by "
+            "[address hidden] ( sign in to view). Price: $71.10 Body of "
+            "\xe2\x98\x86B Contact Seller From Sign in with CNetID or Message")
 
         # Ensure that no emails have been sent.
         self.assertEqual(self.emails, [])
@@ -154,26 +154,6 @@ def test_post_inquiry_with_cnetid(self):
             u"Simply reply to this email if you'd like to get in contact. "
             u"Cheers, The Marketplace Team")
 
-    def test_new_listing_blocked(self):
-        # Try creating a new listing as an unauthenticated user.
-        self.post("/new", data={
-            "csrf_token": self.csrf_token("/new"),
-            "title": u"Title of \u2606D",
-            "body": u"Body of \u2606D",
-            "price": "3.441",
-            "principal": "seller-d@uchicago.edu",
-            "categories": "apartments",
-            "uploaded_photos-2-image":
-                ("caravel/tests/test-pattern.gif", "t.jpg")
-        })
-
-        # Ensure that no listing was created.
-        self.assertEqual(self.clean(self.get("/").data),
-            "New Listing Listing \xe2\x98\x86A $3.10 cars 5h ago Listing "
-            "\xe2\x98\x86B $71.10 apartments 2d ago")
-        self.assertEqual(self.emails, [])
-
-    @unittest.skip("Cannot post unauthenticated listings yet.")
     def test_new_listing(self):
         # Try creating a new listing as an unauthenticated user.
         self.post("/new", data={
@@ -238,6 +218,9 @@ def test_new_listing(self):
             '/_ah/gcs/test.appspot.com/listing-b-small',
         ])
 
+    def test_edit_listing_with_cnetid(self):
+        pass
+
     def test_new_listing_with_cnetid(self):
         # Try creating a new listing as an authenticated user.
         with self.google_apps_user("visitor@uchicago.edu"):
@@ -315,6 +298,46 @@ def test_new_listing_with_cnetid(self):
             u"The Marketplace Team"
         )
 
+    def test_edit_listing(self):
+        # Try editing someone else's listing.
+        result = self.post("/listing_a/edit", data=dict(
+            csrf_token=self.csrf_token("/listing_a/edit"),
+            title=u"Title\u2606A",
+            body=u"Body\u2606A",
+            price="2.34",
+            principal="seller-a@uchicago.edu",
+            categories="cars",
+        )).data
+
+        self.assertEqual(self.clean(result),
+            "New Listing BSD/Medicine/Medical School affiliates: if you are "
+            "unable to sign in with your CNetID, please enter your email in "
+            "the box. Your listing might not be posted immediately. We reserve "
+            "the right to remove listings at any time. Title Seller Sign in "
+            "with CNetID or Please sign in with your CNetID. Categories "
+            "Apartments Subleases Appliances Bikes Books Cars Electronics "
+            "Employment Furniture Miscellaneous Services Wanted Price Body "
+            "Body\xe2\x98\x86A Image Image Image Image Image Cancel"
+        )
+
+        # Try editing our own listing.
+        with self.google_apps_user("seller-a@uchicago.edu"):
+            self.post("/listing_a/edit", data=dict(
+                csrf_token=self.csrf_token("/listing_a/edit"),
+                title=u"Title\u2606A",
+                body=u"Body\u2606A",
+                price="2.34",
+                principal="",
+                categories="cars",
+            ))
+
+            self.assertEqual(self.clean(self.get("/listing_a").data),
+                "New Listing Logged in as seller-a@uchicago.edu My Listings "
+                "Logout Your listing has been updated. Title\xe2\x98\x86A cars "
+                "Posted now by seller-a@uchicago.edu . Price: $2.34 "
+                "Body\xe2\x98\x86A Manage Listing Edit"
+            )
+
     @unittest.skip("Claim listing features disabled")
     def test_claim_listing(self):
         # View a listing and click "Claim"
@@ -367,4 +390,4 @@ def test_claim_listing(self):
             "text": ("Posted by seller-a@uchicago.edu "
                      "(<http://localhost/listing_a?key=a_key|approve>)"),
             "username": u"Listing \u2606A"
-        }])
+        }])

caravel/utils/principals.py

@@ -57,4 +57,31 @@ def __repr__(self):
         """
 
         return "Principal(email={!r}, device={!r}, auth_method={!r})".format(
-            self.email, self.device, self.auth_method)
+            self.email, self.device, self.auth_method)
+
+    def can_act_as(self, other):
+        """
+        Returns True iff +self+ is allowed to act as +other+.
+
+        >>> d = Device("nonce", "user agent", "ip address")
+        >>> p1 = Principal("user1@domain", d, Principal.GOOGLE_APPS)
+        >>> p2 = Principal("user1@domain", d, Principal.EMAIL)
+        >>> p3 = Principal("user2@domain", d, Principal.LEGACY)
+        >>> p1.can_act_as(p2)
+        True
+        >>> p2.can_act_as(p1)
+        False
+        >>> p1.can_act_as(p3)
+        False
+        """
+
+        # Make sure it's mostly the same person.
+        if not isinstance(other, Principal) or self.email != other.email:
+            return False
+
+        # Make sure if they're using Apps, it's only a single email address.
+        if (other.auth_method == self.GOOGLE_APPS and
+            self.auth_method != self.GOOGLE_APPS):
+            return False
+
+        return True