diff --git a/app.py b/app.py index 3f60b55..c553500 100644 --- a/app.py +++ b/app.py @@ -272,6 +272,9 @@ class Item(db.Model): # Where a found item physically is: awaiting drop-off at the security desk, # held there, or released to its owner. Empty for lost reports. custody = db.Column(db.String(20)) + # The finder's answer once the desk logs the item as received: "pending" + # until they confirm it was them, "confirmed", or "disputed". + finder_check = db.Column(db.String(10)) created_at = db.Column(db.DateTime, default=datetime.utcnow, nullable=False) claims = db.relationship("Claim", back_populates="item", cascade="all, delete-orphan") events = db.relationship( @@ -411,6 +414,7 @@ class CustodyEvent(db.Model): VALUABLE_CATEGORIES = {"Electronics", "Wallet & ID", "Keys"} CUSTODY_LABELS = { "awaiting": "Awaiting drop-off at the security desk", + "handed": "Handed in, waiting for the desk to confirm", "held": "Held at the security desk", "office": "Moved to the admin office for safekeeping", "released": "Returned to its owner", @@ -529,6 +533,16 @@ def set_place(item, place="", side=""): item.place_side = found["side"] if found["place"] else None +def add_handover_columns(): + """Add item.finder_check to tables that predate the two-sided handover.""" + inspector = db.inspect(db.engine) + if "item" not in inspector.get_table_names(): + return + if "finder_check" not in {column["name"] for column in inspector.get_columns("item")}: + db.session.execute(db.text("ALTER TABLE item ADD COLUMN finder_check VARCHAR(10)")) + db.session.commit() + + def add_place_columns(): """Add item.place to tables that predate the map, and place old reports from their text.""" inspector = db.inspect(db.engine) @@ -554,6 +568,7 @@ def add_place_columns(): backfill_item_owners() add_desk_columns() add_place_columns() + add_handover_columns() if not app.config["S3_BUCKET"]: Path(app.config["UPLOAD_FOLDER"]).mkdir(parents=True, exist_ok=True) @@ -657,11 +672,17 @@ def inject_current_user(): if user and not user.works_desk: invite = next((i for i in StaffInvite.query.filter_by(email=user.email.lower(), responded_at=None) .order_by(StaffInvite.created_at.desc()).limit(3) if i.is_open), None) + handover_checks = [] + if user: + handover_checks = Item.query.filter_by(owner_id=user.id, finder_check="pending").all() + desk_queue = Item.query.filter_by(status="Found", custody="handed").count() if user and user.works_desk else 0 return { "current_user": user, "incoming_claim_count": incoming, "outgoing_claim_count": outgoing, "staff_invite": invite, + "handover_checks": handover_checks, + "desk_queue": desk_queue, } @@ -2287,35 +2308,132 @@ def desk(): if moved: flash(f"{moved} unclaimed valuable item{'s' if moved != 1 else ''} moved to the admin office.", "success") awaiting = Item.query.filter_by(status="Found", custody="awaiting").order_by(Item.created_at).all() + handed = Item.query.filter_by(status="Found", custody="handed").order_by(Item.created_at).all() + disputed = Item.query.filter_by(finder_check="disputed").order_by(Item.created_at.desc()).all() held = (Item.query.filter(Item.status == "Found", Item.custody.in_(IN_STORAGE)) .order_by(Item.created_at).all()) pending = Claim.query.filter_by(status="Pending").order_by(Claim.created_at).all() approved = Claim.query.filter_by(status="Approved").order_by(Claim.decided_at).all() - return render_template("desk.html", awaiting=awaiting, held=held, pending=pending, approved=approved) + return render_template("desk.html", awaiting=awaiting, handed=handed, disputed=disputed, held=held, + pending=pending, approved=approved) @app.post("/desk/item//received") @staff_required def desk_receive(item_id): item = db.get_or_404(Item, item_id) - if item.status != "Found" or item.custody != "awaiting": - flash("That item is not awaiting drop-off.", "error") + if item.status != "Found" or item.custody not in {"awaiting", "handed"}: + flash("That item is not waiting to be received.", "error") return redirect(url_for("desk")) + staff = get_current_user() + finder = db.session.get(User, item.owner_id) if item.owner_id else None + finder_said_so = item.custody == "handed" item.custody = "held" - log_custody(item, "Received at the security desk", get_current_user()) + if finder_said_so: + item.finder_check = "confirmed" + log_custody(item, "Received at the security desk, confirming the finder's hand-in", staff) + else: + # The item is held at once, so the owner can claim it; the finder is + # asked to confirm it was them, but nothing waits on their answer. + item.finder_check = "pending" if finder and finder.id != staff.id else None + log_custody(item, "Received at the security desk", staff) + db.session.commit() + if finder and finder.id != staff.id: + ask = "" if finder_said_so else ( + "\n\nIf that wasn't you, open ClassFind and tell us: there is a notice on every page " + f"until you answer.\n{url_for('item_detail', item_id=item.id, _external=True)}") + send_email( + finder.email, + f"The desk received {item.title}", + f"Hi {finder.name},\n\nThe security desk logged \"{item.title}\" as received from you. " + f"We'll take it from here. Thank you for returning it.{ask}", + ) + flash(f"{item.title} is now held at the desk.", "success") + return redirect(url_for("desk")) + + +@app.post("/desk/item//not-received") +@staff_required +def desk_not_received(item_id): + """The finder said they handed it in, but the desk does not have it.""" + item = db.get_or_404(Item, item_id) + if item.status != "Found" or item.custody != "handed": + flash("That item is not waiting for the desk to confirm.", "error") + return redirect(url_for("desk")) + item.custody = "awaiting" + log_custody(item, "The desk has not received it; back to awaiting drop-off", get_current_user()) db.session.commit() finder = db.session.get(User, item.owner_id) if item.owner_id else None if finder: send_email( finder.email, - f"Thanks for handing in {item.title}", - f"Hi {finder.name},\n\nThe security desk has received \"{item.title}\". " - "We'll take it from here. Thank you for returning it.", + f"The desk hasn't received {item.title}", + f"Hi {finder.name},\n\nYou marked \"{item.title}\" as handed in, but the security desk " + "doesn't have it. If you still have it, please bring it to the desk in the Main Block.\n" + f"{url_for('item_detail', item_id=item.id, _external=True)}", ) - flash(f"{item.title} is now held at the desk.", "success") + flash(f"{item.title} is back to awaiting drop-off, and the finder has been told.", "success") return redirect(url_for("desk")) +@app.post("/item//handed-in") +@login_required +def mark_handed_in(item_id): + """The finder says they handed the item in; the desk is asked to confirm.""" + item = db.get_or_404(Item, item_id) + user = get_current_user() + if not owns_item(user, item) or item.status != "Found" or item.custody != "awaiting": + flash("Only the finder can mark an item awaiting drop-off as handed in.", "error") + return redirect(url_for("item_detail", item_id=item.id)) + item.custody = "handed" + log_custody(item, "Finder says they handed it in at the security desk", user) + db.session.commit() + for staff in User.query.filter(or_(User.is_staff.is_(True), User.is_admin.is_(True))).all(): + send_email( + staff.email, + f"Confirm you received {item.title}", + f"{user.name} says they handed \"{item.title}\" in at the security desk. " + "Please confirm on the Desk page whether you have it.\n" + f"{url_for('desk', _external=True)}", + ) + flash("Thanks! The desk will confirm once they've checked.", "success") + return redirect(url_for("item_detail", item_id=item.id)) + + +@app.post("/item//handover-check") +@login_required +def answer_handover_check(item_id): + """The finder confirms the desk's record, or says it wasn't them.""" + item = db.get_or_404(Item, item_id) + user = get_current_user() + if item.owner_id != user.id or item.finder_check != "pending": + flash("There is nothing to confirm for that item.", "error") + return redirect(url_for("profile")) + if request.form.get("answer") == "yes": + item.finder_check = "confirmed" + log_custody(item, "Finder confirmed handing it in", user) + flash("Thanks for confirming.", "success") + else: + item.finder_check = "disputed" + log_custody(item, "Finder says they did not hand this in; flagged for the admin", user) + admin = admin_email() + if admin: + send_email( + admin, + f"Check the handover of {item.title}", + f"{user.name} says they did not hand in \"{item.title}\", which the desk logged as " + "received from them. Please check it.\n" + f"{url_for('item_detail', item_id=item.id, _external=True)}", + ) + flash("Thanks for telling us. The admin will check this handover.", "success") + db.session.commit() + # Back to the page the notice was on, but only ever within this site. + target = request.form.get("next", "") + if not target.startswith("/") or target.startswith("//"): + target = url_for("item_detail", item_id=item.id) + return redirect(target) + + @app.post("/claims//reissue") @staff_required def reissue_code(claim_id): diff --git a/static/style.css b/static/style.css index 1e37b38..3147acb 100644 --- a/static/style.css +++ b/static/style.css @@ -286,3 +286,7 @@ body::after{content:"";position:fixed;z-index:-1;width:420px;height:420px;right: .notice.warn{background:#fbf1e3;border-color:#ecd3ac} .flash-stack{position:relative;z-index:1} .invite-question{margin:0 0 18px;font-size:16px;line-height:1.6}.invite-question strong{display:block;font:600 20px "Space Grotesk",sans-serif;margin-bottom:4px} +.custody-handed{background:#fbf1e3;color:#8a5a12} +.notice .secondary-btn.compact{padding:10px 14px} +form.notice{margin-top:4px} +.desk-overview{grid-template-columns:repeat(5,minmax(0,1fr))}@media(max-width:900px){.desk-overview{grid-template-columns:repeat(2,minmax(0,1fr))}} diff --git a/templates/base.html b/templates/base.html index 1ebe5e6..720a298 100644 --- a/templates/base.html +++ b/templates/base.html @@ -32,7 +32,7 @@ Claims{% if incoming_claim_count or outgoing_claim_count %} {{ incoming_claim_count + outgoing_claim_count }}{% endif %} My Reports QR Tags - {% if current_user.works_desk %}Desk{% endif %} + {% if current_user.works_desk %}Desk{% if desk_queue %} {{ desk_queue }}{% endif %}{% endif %} {% if current_user.is_admin %}Admin{% endif %}
{% else %} @@ -55,6 +55,23 @@ See the invitation {% endif %} + {% for check in handover_checks %} +
+

The security desk logged your {{ check.title }} as received from you. Was that you?

+
+ + + + +
+
+ {% endfor %} + {% if desk_queue and request.endpoint != "desk" %} +
+

{{ desk_queue }} item{{ "s" if desk_queue != 1 }} handed in by finders {{ "are" if desk_queue != 1 else "is" }} waiting for the desk to confirm.

+ Open the desk +
+ {% endif %} {% block content %}{% endblock %}
ClassFind · BIT campus lost and foundFind it. Report it. Return it.
diff --git a/templates/desk.html b/templates/desk.html index ac2eea2..b712e1a 100644 --- a/templates/desk.html +++ b/templates/desk.html @@ -11,6 +11,7 @@

Every found item, accounted for.

+
To confirm{{ handed|length }}Finder says it's handed in
Awaiting drop-off{{ awaiting|length }}Reported, not yet handed in
Held at desk{{ held|length }}In storage now
Claims to review{{ pending|length }}Check the proof
@@ -32,6 +33,46 @@

Enter the owner's collection code

+{% if handed %} +
+

HANDED IN, PLEASE CONFIRM

{{ handed|length }} to check

The finder says these are at the desk
+
+ {% for item in handed %} +
+
+ {{ item.category }} +

{{ item.title }}

+

Handed in by {{ item.reporter_name }} · ⌖ {{ item.location }}

+ Reported {{ item.created_at.strftime("%d %b %Y, %I:%M %p") }} +
+
+
+
+
+
+ {% endfor %} +
+
+{% endif %} + +{% if disputed %} +
+

{{ disputed|length }} handover{{ "s" if disputed|length != 1 }} the finder says {{ "were" if disputed|length != 1 else "was" }} not theirs. Check the custody log and ask at the desk.

+
+ {% for item in disputed %} + + {% endfor %} +
+
+{% endif %} + +

CLAIMS TO REVIEW

{{ pending|length }} waiting

Approve only when the proof matches the item
{% if pending %} diff --git a/templates/item.html b/templates/item.html index 0fef290..e1c5fd1 100644 --- a/templates/item.html +++ b/templates/item.html @@ -20,6 +20,15 @@

{{ item.title }}

{% endif %} + {% if can_manage and item.status == "Found" and item.custody == "awaiting" and item.owner_id == current_user.id %} +
+ +

Have you handed this in at the security desk in the Main Block?

+ +
+ {% elif can_manage and item.custody == "handed" %} +

You marked this as handed in. The desk will confirm once they've checked.

+ {% endif %} {% if can_manage %}
Edit report diff --git a/templates/profile.html b/templates/profile.html index 5ed4ba6..3ba3102 100644 --- a/templates/profile.html +++ b/templates/profile.html @@ -20,7 +20,7 @@ {% if items %}
{% for item in items %}
-
{% if item.image_src %}{{ item.title }}{% else %}{{ item.category[:1].upper() }}{% endif %}{{ item.status }}
+
{% if item.image_src %}{{ item.title }}{% else %}{{ item.category[:1].upper() }}{% endif %}{{ item.status }}{% if item.custody in ("awaiting", "handed") %}{{ "Waiting for the desk" if item.custody == "handed" else "Hand it in" }}{% endif %}

{{ item.category }}

{{ item.title }}

{{ item.description }}

⌖ {{ item.location }}{{ item.created_at.strftime("%d %b %Y") }}
View →Edit →
{% endfor %}
diff --git a/tests/test_app.py b/tests/test_app.py index 448d1dd..87fe9a6 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -424,6 +424,94 @@ def test_found_items_go_through_the_security_desk(self): response = self.post("/desk/handover", data={"code": code}, follow_redirects=True) self.assertIn(b"does not match", response.data) + def test_finder_says_handed_in_and_the_desk_confirms(self): + item_id = self.found_item() + self.register_staff() + self.logout() + self.login("finder@example.com") + page = self.client.get(f"/item/{item_id}").data + self.assertIn(b"handed it in", page) + with mock.patch("app.send_email") as sent: + self.post(f"/item/{item_id}/handed-in", follow_redirects=True) + self.assertIn("desk@example.com", [call.args[0] for call in sent.call_args_list]) + with app.app_context(): + self.assertEqual(db.session.get(Item, item_id).custody, "handed") + self.logout() + self.login("desk@example.com") + home = self.client.get("/").data + self.assertIn(b"waiting for the desk to confirm", home) + self.assertIn(b'Desk 1', home) + self.post(f"/desk/item/{item_id}/received", follow_redirects=True) + with app.app_context(): + item = db.session.get(Item, item_id) + self.assertEqual((item.custody, item.finder_check), ("held", "confirmed")) + self.assertIn("confirming the finder", item.events[-1].action) + self.logout() + self.login("finder@example.com") + # The finder already said so, so they are not asked again. + self.assertNotIn(b"Was that you?", self.client.get("/").data) + + def test_desk_says_not_received_and_the_finder_is_told(self): + item_id = self.found_item() + self.login("finder@example.com") + self.post(f"/item/{item_id}/handed-in") + self.logout() + self.register_staff() + with mock.patch("app.send_email") as sent: + self.post(f"/desk/item/{item_id}/not-received", follow_redirects=True) + self.assertEqual(sent.call_args.args[0], "finder@example.com") + with app.app_context(): + self.assertEqual(db.session.get(Item, item_id).custody, "awaiting") + + def test_desk_receives_first_and_the_finder_confirms_or_disputes(self): + first = self.found_item("Blue bottle") + self.login("finder@example.com") + self.report("Red cap", "Cotton cap", "Clothing", "Gym", "Found") + self.logout() + with app.app_context(): + second = Item.query.filter_by(title="Red cap").one().id + self.register_admin("admin@example.com") + self.post(f"/desk/item/{first}/received") + self.post(f"/desk/item/{second}/received") + with app.app_context(): + # Held at once, so the owner can claim it while the finder is asked. + self.assertEqual(db.session.get(Item, first).custody, "held") + self.assertEqual(db.session.get(Item, first).finder_check, "pending") + self.logout() + self.login("finder@example.com") + home = self.client.get("/").data + self.assertEqual(home.count(b"Was that you?"), 2) + self.post(f"/item/{first}/handover-check", data={"answer": "yes", "next": "/"}) + with mock.patch("app.send_email") as sent: + response = self.post(f"/item/{second}/handover-check", data={"answer": "no", "next": "//evil.example"}) + self.assertEqual(sent.call_args.args[0], "admin@example.com") + self.assertTrue(response.headers["Location"].endswith(f"/item/{second}")) + self.assertNotIn(b"Was that you?", self.client.get("/").data) + with app.app_context(): + self.assertEqual(db.session.get(Item, first).finder_check, "confirmed") + self.assertEqual(db.session.get(Item, second).finder_check, "disputed") + self.logout() + self.login("admin@example.com") + desk = self.client.get("/desk").data + self.assertIn(b"not theirs", desk) + self.assertIn(b"Red cap", desk.split(b"not theirs", 1)[1]) + + def test_only_the_finder_can_mark_an_item_handed_in(self): + item_id = self.found_item() + self.register("Someone", "someone@example.com") + self.post(f"/item/{item_id}/handed-in", follow_redirects=True) + self.post(f"/item/{item_id}/handover-check", data={"answer": "no"}) + with app.app_context(): + item = db.session.get(Item, item_id) + self.assertEqual((item.custody, item.finder_check), ("awaiting", None)) + + def test_the_handover_column_is_added_to_an_older_database(self): + with app.app_context(): + db.session.execute(db.text("ALTER TABLE item DROP COLUMN finder_check")) + db.session.commit() + app_module.add_handover_columns() + self.assertIn("finder_check", {c["name"] for c in db.inspect(db.engine).get_columns("item")}) + def test_handover_waits_for_the_item_to_reach_the_desk(self): item_id = self.found_item() self.register("Owner", "owner@example.com") diff --git a/tools/mutate.py b/tools/mutate.py index 7b8353c..adde20f 100644 --- a/tools/mutate.py +++ b/tools/mutate.py @@ -30,6 +30,8 @@ ["handover", "code", "collect", "desk", "reissue"]), ("app.py", {"escalate_unclaimed_valuables"}, ["escalat", "valuable", "office"]), ("app.py", {"set_place", "search_filter"}, ["place", "search", "picker"]), + ("app.py", {"desk_receive", "desk_not_received", "mark_handed_in", "answer_handover_check"}, + ["handed", "handover", "finder", "not_received", "desk"]), ] COMPARE_SWAPS = {ast.Lt: ast.LtE, ast.LtE: ast.Lt, ast.Gt: ast.GtE, ast.GtE: ast.Gt,