From 6820149d3bf37d27ac6887e476e6d75e15121451 Mon Sep 17 00:00:00 2001 From: Aditya Patil Date: Mon, 26 Oct 2026 13:41:53 +0530 Subject: [PATCH 01/10] WIP: doctype setup --- packages/types/RavenMessaging/RavenMessage.ts | 2 +- raven/api/native.py | 3 + raven/api/raven_message.py | 119 +++++++- raven/api/threads.py | 6 +- .../doctype/raven_message/raven_message.json | 4 +- .../doctype/raven_message/raven_message.py | 14 +- raven/tests/test_location_messages.py | 267 ++++++++++++++++++ raven/tests/test_voice_notes.py | 12 +- 8 files changed, 398 insertions(+), 29 deletions(-) create mode 100644 raven/tests/test_location_messages.py diff --git a/packages/types/RavenMessaging/RavenMessage.ts b/packages/types/RavenMessaging/RavenMessage.ts index 899754695..0b3ff1061 100644 --- a/packages/types/RavenMessaging/RavenMessage.ts +++ b/packages/types/RavenMessaging/RavenMessage.ts @@ -30,7 +30,7 @@ export interface RavenMessage{ /** Is Thread : Check - This message starts a thread */ is_thread?: 0 | 1 /** Message Type : Select */ - message_type?: "Text" | "Image" | "File" | "Poll" | "System" | "Voice" + message_type?: "Text" | "Image" | "File" | "Poll" | "System" | "Voice" | "Location" /** Content : Long Text */ content?: string /** File : Attach */ diff --git a/raven/api/native.py b/raven/api/native.py index 09c66cd92..4b167165b 100644 --- a/raven/api/native.py +++ b/raven/api/native.py @@ -8,6 +8,8 @@ # answered by the same rule. The origin allowlist is the real boundary, and those origins only # exist on the user's own device. APP_HEADER = "X-Raven-App" +# Features the app checks on every launch. A list so later features add entries. +RAVEN_FEATURES = ["location_messages"] @frappe.whitelist() @@ -19,6 +21,7 @@ def boot(): data = frappe.sessions.get() if frappe.cache.hget("bootinfo", frappe.session.user) else app_boot() data["push_relay_server_url"] = frappe.conf.get("push_relay_server_url") data["server_script_enabled"] = frappe.conf.get("server_script_enabled", True) + data["raven_features"] = RAVEN_FEATURES return data diff --git a/raven/api/raven_message.py b/raven/api/raven_message.py index ec118b310..ae2f43b36 100644 --- a/raven/api/raven_message.py +++ b/raven/api/raven_message.py @@ -1,4 +1,5 @@ import math +import urllib.parse import frappe from frappe import _ @@ -60,6 +61,10 @@ def send_message( _MAX_AUDIO_SECONDS = 660 _MAX_WAVEFORM_BARS = 64 +_MAX_LOCATION_NAME = 120 +_MAX_LOCATION_ADDRESS = 300 +_MAX_LOCATION_ACCURACY = 100_000 + # Inline-image display caps (mirror the web client's reserved box). The stored # aspect ratio is what prevents reflow; the absolute thumbnail size only needs # to be reasonable. The client mirrors these values for its optimistic @@ -108,23 +113,88 @@ def _sane_waveform(value) -> list[int] | None: return bars -def attach_voice_payloads(rows: list[dict], id_field: str = "name") -> list[dict]: +def _sane_number(value) -> float | None: + """A finite number from an untrusted client, or None.""" + if isinstance(value, bool): + return None + try: + number = float(value) + except (TypeError, ValueError): + return None + return number if math.isfinite(number) else None + + +def _sane_coordinate(value, limit: float) -> float | None: + """A coordinate within Β±limit, rounded to ~0.1 m, or None.""" + number = _sane_number(value) + if number is None or not -limit <= number <= limit: + return None + return round(number, 6) + + +def _sane_location_text(value, limit: int) -> str: + """Plain text from an untrusted client: tags stripped, trimmed, capped.""" + return frappe.utils.strip_html_tags(str(value or "")).strip()[:limit] + + +def _sane_location(value) -> dict | None: + """A plausible location payload from an untrusted client, or None.""" + if not isinstance(value, dict): + return None + latitude = _sane_coordinate(value.get("latitude"), 90) + longitude = _sane_coordinate(value.get("longitude"), 180) + if latitude is None or longitude is None: + return None + + location = {"latitude": latitude, "longitude": longitude} + accuracy = _sane_number(value.get("accuracy")) + if accuracy is not None and 0 < accuracy <= _MAX_LOCATION_ACCURACY: + location["accuracy"] = accuracy + name = _sane_location_text(value.get("name"), _MAX_LOCATION_NAME) + if name: + location["name"] = name + address = _sane_location_text(value.get("address"), _MAX_LOCATION_ADDRESS) + if address: + location["address"] = address + return location + + +def _maps_url(location: dict, maps: str | None = None) -> str: + """The maps link for a location, in the sender's app; Google unless Apple is asked for.""" + latitude = location["latitude"] + longitude = location["longitude"] + if maps == "apple": + label = urllib.parse.quote(location.get("name") or "Location", safe="-_.!~*'()") + return f"https://maps.apple.com/?ll={latitude},{longitude}&q={label}" + return f"https://www.google.com/maps/search/?api=1&query={latitude},{longitude}" + + +def _location_text(url: str) -> str: + """A plain link for clients that can't draw the location card.""" + escaped = frappe.utils.escape_html(url) + return f'

{escaped}

' + + +_JSON_PAYLOAD_TYPES = ("Voice", "Location") + + +def attach_json_payloads(rows: list[dict], id_field: str = "name") -> list[dict]: """ - Add `json` to the Voice rows of a list query that doesn't select it. One extra - query, only for voice notes: a text message's `json` is its whole editor document. + Add `json` to the Voice and Location rows of a list query that doesn't select it. + One extra query, only for those types: a text message's `json` is its whole editor document. """ - voice_ids = [row[id_field] for row in rows if row.get("message_type") == "Voice"] - if not voice_ids: + payload_ids = [row[id_field] for row in rows if row.get("message_type") in _JSON_PAYLOAD_TYPES] + if not payload_ids: return rows # The caller has already authorized these rows. Raven Message's list permission is - # owner-only, so a permission-checked query would drop other people's notes. + # owner-only, so a permission-checked query would drop other people's messages. payloads = dict( frappe.get_all( - "Raven Message", filters={"name": ["in", voice_ids]}, fields=["name", "json"], as_list=True + "Raven Message", filters={"name": ["in", payload_ids]}, fields=["name", "json"], as_list=True ) ) for row in rows: - if row.get("message_type") == "Voice": + if row.get("message_type") in _JSON_PAYLOAD_TYPES: row["json"] = payloads.get(row[id_field]) return rows @@ -266,6 +336,7 @@ def send_message_with_attachments( send_silently: bool = False, link_doctype: str | None = None, link_document: str | None = None, + location: dict | str | None = None, ): """ v3 composer send. Creates one message per (already-uploaded) file first β€” @@ -282,6 +353,11 @@ def send_message_with_attachments( client when it recorded the clip; it becomes a `Voice` message with that payload in `json`. Unusable metadata sends the audio as a plain file. + `location` is one point (`{"latitude", "longitude", "accuracy"?, "name"?, + "address"?}`) and must be the whole send: no content, files or linked document. + `maps` (`"apple"` or `"google"`) only picks the link stored in `text` for clients + that can't draw the card; it is not kept in `json`. + `send_silently` suppresses notifications for the whole batch (the flag is set on every message before insert). @@ -296,6 +372,8 @@ def send_message_with_attachments( if isinstance(files, str): files = frappe.parse_json(files) files = files or [] + if isinstance(location, str): + location = frappe.parse_json(location) content = content or "" has_text = bool(frappe.utils.strip_html_tags(content).strip()) @@ -307,7 +385,13 @@ def send_message_with_attachments( has_linked_document = bool(link_doctype and link_document) - if not has_body and not files and not has_linked_document: + if location is not None: + if has_body or files or has_linked_document: + frappe.throw(_("A location is sent on its own")) + sane_location = _sane_location(location) + if not sane_location: + frappe.throw(_("Invalid location")) + elif not has_body and not files and not has_linked_document: frappe.throw(_("Cannot send an empty message")) validate_files_uploaded_by_user([f["file_url"] if isinstance(f, dict) else f for f in files]) @@ -322,6 +406,14 @@ def send_message_with_attachments( # then the text. Each file carries its url + size (size is read straight from # the client's upload response, so no per-file File lookup is needed here). specs = [] + if location is not None: + specs.append( + { + "message_type": "Location", + "json": sane_location, + "text": _location_text(_maps_url(sane_location, location.get("maps"))), + } + ) for f in files: file_url = f["file_url"] if isinstance(f, dict) else f file_size = f.get("file_size") if isinstance(f, dict) else None @@ -495,7 +587,7 @@ def get_pinned_messages(channel_id: str): pinnedMessagesString = frappe.db.get_value("Raven Channel", channel_id, "pinned_messages_string") pinnedMessages = pinnedMessagesString.split("\n") if pinnedMessagesString else [] - return attach_voice_payloads( + return attach_json_payloads( frappe.db.get_all( "Raven Message", filters={"name": ["in", pinnedMessages]}, @@ -606,7 +698,7 @@ def get_saved_messages( else: query = query.orderby(raven_message.creation, order=Order.asc) - return attach_voice_payloads(query.run(as_dict=True)) + return attach_json_payloads(query.run(as_dict=True)) @frappe.whitelist() @@ -1006,8 +1098,9 @@ def _forward_payloads(message_id: str) -> list[dict]: # front. `json` goes with it: it still holds the unquoted body, and the copy # should have one body that carries the quote. if payload.get("is_reply"): - # A voice note has no body to carry the quote; its `json` is the payload. - if payload.get("message_type") != "Voice": + # Voice and Location keep their own body: prepending the quote to a link + # would mangle it, and `json` is the content. + if payload.get("message_type") not in _JSON_PAYLOAD_TYPES: quote = build_reply_blockquote( payload.get("replied_message_details"), linked_message=member.get("linked_message") ) diff --git a/raven/api/threads.py b/raven/api/threads.py index cb944f76f..79bdaec93 100644 --- a/raven/api/threads.py +++ b/raven/api/threads.py @@ -4,7 +4,7 @@ from frappe.query_builder.functions import Coalesce, Count from raven.api.raven_channel import get_peer_user_id_from_dm_users, is_channel_member -from raven.api.raven_message import attach_voice_payloads +from raven.api.raven_message import attach_json_payloads from raven.utils import ( create_members_added_system_message, get_channel_members, @@ -117,7 +117,7 @@ def get_all_threads( query = query.orderby(channel.last_message_timestamp, order=Order.desc) - threads = attach_voice_payloads(query.run(as_dict=True), id_field="thread_message_id") + threads = attach_json_payloads(query.run(as_dict=True), id_field="thread_message_id") # v2 renders participants in the list, so it fetches them inline (the default). v3 sends # fetch_members=False and lazy-loads members per row on view (get_thread_details β†’ @@ -216,7 +216,7 @@ def get_other_threads( query = query.orderby(thread_channel.last_message_timestamp, order=Order.desc) - threads = attach_voice_payloads(query.run(as_dict=True), id_field="thread_message_id") + threads = attach_json_payloads(query.run(as_dict=True), id_field="thread_message_id") # v2 fetches members inline (default); v3 sends fetch_members=False and lazy-loads on # row view (see get_all_threads) to avoid the per-thread fan-out. diff --git a/raven/raven_messaging/doctype/raven_message/raven_message.json b/raven/raven_messaging/doctype/raven_message/raven_message.json index 91641df57..61b9b0de5 100644 --- a/raven/raven_messaging/doctype/raven_message/raven_message.json +++ b/raven/raven_messaging/doctype/raven_message/raven_message.json @@ -71,7 +71,7 @@ "fieldname": "message_type", "fieldtype": "Select", "label": "Message Type", - "options": "Text\nImage\nFile\nPoll\nSystem\nVoice" + "options": "Text\nImage\nFile\nPoll\nSystem\nVoice\nLocation" }, { "fieldname": "message_reactions", @@ -235,7 +235,7 @@ ], "index_web_pages_for_search": 1, "links": [], - "modified": "2026-10-23 12:00:00.000000", + "modified": "2026-10-25 12:00:00.000000", "modified_by": "Administrator", "module": "Raven Messaging", "name": "Raven Message", diff --git a/raven/raven_messaging/doctype/raven_message/raven_message.py b/raven/raven_messaging/doctype/raven_message/raven_message.py index 7b1c821c9..b031c15a8 100644 --- a/raven/raven_messaging/doctype/raven_message/raven_message.py +++ b/raven/raven_messaging/doctype/raven_message/raven_message.py @@ -64,7 +64,7 @@ class RavenMessage(Document): mentions: DF.Table[RavenMention] message_batch_id: DF.Data | None message_reactions: DF.JSON | None - message_type: DF.Literal["Text", "Image", "File", "Poll", "System", "Voice"] + message_type: DF.Literal["Text", "Image", "File", "Poll", "System", "Voice", "Location"] notification: DF.Data | None poll_id: DF.Link | None replied_message_details: DF.JSON | None @@ -272,6 +272,9 @@ def validate(self): _("Direct modification of message_reactions is not allowed. Use the Reactions API.") ) + if not self.is_new() and self.message_type == "Location" and self.has_value_changed("text"): + frappe.throw(_("A location can't be edited")) + def validate_linked_message(self): """ If there is a linked message, the linked message should be in the same channel @@ -329,9 +332,9 @@ def before_insert(self): "owner": details.owner, "creation": datetime.datetime.strftime(details.creation, "%Y-%m-%d %H:%M:%S"), } - # A voice note's preview shows its length. Fetched only then: a text message's - # `json` is its whole editor document. - if details.message_type == "Voice": + # A voice note's preview shows its length; a location's shows its point. + # Fetched only then: a text message's `json` is its whole editor document. + if details.message_type in ("Voice", "Location"): snapshot["json"] = json_dict(frappe.db.get_value("Raven Message", self.linked_message, "json")) self.replied_message_details = frappe.as_json(snapshot) @@ -723,6 +726,9 @@ def get_notification_message_content(self): if isinstance(duration, (int, float)) and duration > 0: return f"🎀 Sent a voice note ({int(duration // 60)}:{int(duration % 60):02d})" return "🎀 Sent a voice note" + elif self.message_type == "Location": + name = json_dict(self.json).get("name") + return f"πŸ“ Shared a location - {name}" if name else "πŸ“ Shared a location" elif self.text: return self.content diff --git a/raven/tests/test_location_messages.py b/raven/tests/test_location_messages.py new file mode 100644 index 000000000..4b12d78dd --- /dev/null +++ b/raven/tests/test_location_messages.py @@ -0,0 +1,267 @@ +import frappe +from frappe.tests import IntegrationTestCase + +from raven.api.raven_message import ( + _forward_payloads, + _maps_url, + _sane_location, + attach_json_payloads, + send_message_with_attachments, +) + +EXTRA_TEST_RECORD_DEPENDENCIES = ["User", "Raven User"] + + +class TestLocationSanitizer(IntegrationTestCase): + def test_sane_location_keeps_and_rounds(self): + self.assertEqual( + _sane_location( + { + "latitude": 12.97560123, + "longitude": "77.6066", + "accuracy": 15.5, + "name": " MG Road ", + "address": "Bengaluru", + } + ), + { + "latitude": 12.975601, + "longitude": 77.6066, + "accuracy": 15.5, + "name": "MG Road", + "address": "Bengaluru", + }, + ) + + def test_sane_location_rejects_bad_coordinates(self): + bad_coordinates = ( + (91, 0), + (0, 181), + (True, 0), + (float("nan"), 0), + ("abc", 0), + (None, 0), + (0, float("inf")), + ) + for lat, lng in bad_coordinates: + self.assertIsNone(_sane_location({"latitude": lat, "longitude": lng}), (lat, lng)) + for junk in (None, "x", [1, 2], 5): + self.assertIsNone(_sane_location(junk), junk) + + def test_sane_location_drops_bad_optionals(self): + loc = _sane_location( + {"latitude": 1, "longitude": 2, "accuracy": 0, "name": "", "address": " "} + ) + self.assertEqual(loc, {"latitude": 1.0, "longitude": 2.0}) + self.assertIsNone( + _sane_location({"latitude": 1, "longitude": 2, "accuracy": 100001}).get("accuracy") + ) + self.assertEqual( + len(_sane_location({"latitude": 1, "longitude": 2, "name": "x" * 500})["name"]), 120 + ) + self.assertEqual( + _sane_location({"latitude": 1, "longitude": 2, "name": "Cafe"})[ + "name" + ], + "aCafe", + ) + + def test_maps_url(self): + loc = {"latitude": 12.97561, "longitude": 77.60659, "name": "MG Road & Co"} + self.assertEqual( + _maps_url(loc, "apple"), + "https://maps.apple.com/?ll=12.97561,77.60659&q=MG%20Road%20%26%20Co", + ) + self.assertEqual( + _maps_url(loc, "google"), + "https://www.google.com/maps/search/?api=1&query=12.97561,77.60659", + ) + self.assertEqual(_maps_url(loc, "junk"), _maps_url(loc, "google")) + self.assertIn("q=Location", _maps_url({"latitude": 1.0, "longitude": 2.0}, "apple")) + + +class TestLocationSend(IntegrationTestCase): + def setUp(self): + for user in ("test@example.com", "test1@example.com"): + frappe.get_doc("User", user).add_roles("Raven User") + + # test creates the channel; test1 (a member) sends the location + frappe.set_user("test@example.com") + workspace = frappe.get_doc( + {"doctype": "Raven Workspace", "workspace_name": "Location Workspace", "type": "Public"} + ).insert() + self.channel = frappe.get_doc( + { + "doctype": "Raven Channel", + "channel_name": "location", + "type": "Public", + "workspace": workspace.name, + } + ).insert() + frappe.get_doc( + {"doctype": "Raven Workspace Member", "user": "test1@example.com", "workspace": workspace.name} + ).insert(ignore_permissions=True) + frappe.get_doc( + { + "doctype": "Raven Channel Member", + "channel_id": self.channel.name, + "user_id": "test1@example.com", + } + ).insert(ignore_permissions=True) + + frappe.set_user("test1@example.com") + frappe.clear_cache() + + def tearDown(self): + frappe.db.rollback() + frappe.set_user("Administrator") + frappe.clear_cache() + + def test_send_creates_location_message(self): + [msg] = send_message_with_attachments( + channel_id=self.channel.name, + client_id="loc-1", + location={ + "latitude": 12.97561, + "longitude": 77.60659, + "accuracy": 15, + "name": "MG Road", + "maps": "google", + }, + ) + doc = frappe.get_doc("Raven Message", msg.name) + self.assertEqual(doc.message_type, "Location") + self.assertEqual( + frappe.parse_json(doc.json), + {"latitude": 12.97561, "longitude": 77.60659, "accuracy": 15.0, "name": "MG Road"}, + ) + self.assertIn( + 'href="https://www.google.com/maps/search/?api=1&query=12.97561,77.60659"', doc.text + ) + self.assertEqual( + doc.content, + "https://www.google.com/maps/search/?api=1&query=12.97561,77.60659", + ) + self.assertEqual( + doc.links.strip(), + "https://www.google.com/maps/search/?api=1&query=12.97561,77.60659", + ) + + def test_link_text_escapes_name(self): + [msg] = send_message_with_attachments( + channel_id=self.channel.name, + location={"latitude": 1, "longitude": 2, "name": '">', "maps": "apple"}, + ) + doc = frappe.get_doc("Raven Message", msg.name) + self.assertEqual(doc.text.count("hi

"}, + {"files": [{"file_url": "/files/a.png", "file_size": 1}]}, + {"link_doctype": "User", "link_document": "Administrator"}, + ) + for extra in extras: + with self.assertRaises(frappe.ValidationError): + send_message_with_attachments( + channel_id=self.channel.name, + location={"latitude": 1, "longitude": 2}, + **extra, + ) + + def test_reply_silent_and_idempotent(self): + parent = send_message_with_attachments( + channel_id=self.channel.name, content="

where?

" + )[0] + kwargs = dict( + channel_id=self.channel.name, + client_id="loc-retry", + is_reply=True, + linked_message=parent.name, + send_silently=True, + location={"latitude": 1, "longitude": 2}, + ) + first = send_message_with_attachments(**kwargs) + again = send_message_with_attachments(**kwargs) + self.assertEqual([m.name for m in first], [m.name for m in again]) + self.assertEqual( + frappe.db.get_value("Raven Message", first[0].name, "linked_message"), parent.name + ) + + def test_notification_text(self): + [a] = send_message_with_attachments( + channel_id=self.channel.name, location={"latitude": 1, "longitude": 2, "name": "MG Road"} + ) + [b] = send_message_with_attachments( + channel_id=self.channel.name, location={"latitude": 1, "longitude": 2} + ) + self.assertEqual( + frappe.get_doc("Raven Message", a.name).get_notification_message_content(), + "πŸ“ Shared a location - MG Road", + ) + self.assertEqual( + frappe.get_doc("Raven Message", b.name).get_notification_message_content(), + "πŸ“ Shared a location", + ) + + def test_json_attached_for_location_rows(self): + [loc] = send_message_with_attachments( + channel_id=self.channel.name, location={"latitude": 1, "longitude": 2} + ) + rows = attach_json_payloads([frappe._dict(name=loc.name, message_type="Location")]) + self.assertEqual(frappe.parse_json(rows[0]["json"])["latitude"], 1.0) + + def test_reply_snapshot_carries_location_json(self): + [loc] = send_message_with_attachments( + channel_id=self.channel.name, + location={"latitude": 1, "longitude": 2, "name": "Cafe"}, + ) + reply = send_message_with_attachments( + channel_id=self.channel.name, + content="

ok

", + is_reply=True, + linked_message=loc.name, + )[0] + snapshot = frappe.parse_json( + frappe.db.get_value("Raven Message", reply.name, "replied_message_details") + ) + self.assertEqual(snapshot["message_type"], "Location") + self.assertEqual(frappe.parse_json(snapshot["json"])["name"], "Cafe") + + def test_forwarded_location_reply_keeps_payload(self): + parent = send_message_with_attachments( + channel_id=self.channel.name, content="

where?

" + )[0] + [loc] = send_message_with_attachments( + channel_id=self.channel.name, + is_reply=True, + linked_message=parent.name, + location={"latitude": 1, "longitude": 2}, + ) + source = frappe.get_doc("Raven Message", loc.name) + [payload] = _forward_payloads(loc.name) + self.assertEqual(payload["message_type"], "Location") + self.assertEqual(payload["text"], source.text) + self.assertEqual(frappe.parse_json(payload["json"])["latitude"], 1.0) + self.assertNotIn("is_reply", payload) + + def test_native_boot_advertises_location(self): + from raven.api.native import boot + + self.assertIn("location_messages", boot()["raven_features"]) + + def test_location_text_not_editable(self): + [msg] = send_message_with_attachments( + channel_id=self.channel.name, location={"latitude": 1, "longitude": 2} + ) + doc = frappe.get_doc("Raven Message", msg.name) + doc.text = "

edited

" + with self.assertRaises(frappe.ValidationError): + doc.save() diff --git a/raven/tests/test_voice_notes.py b/raven/tests/test_voice_notes.py index 2746e96f6..b901b046a 100644 --- a/raven/tests/test_voice_notes.py +++ b/raven/tests/test_voice_notes.py @@ -183,8 +183,8 @@ def test_forwarded_voice_note_keeps_payload(self): self.assertEqual(frappe.parse_json(copy.json), {"duration": 7.25, "waveform": [0, 50, 100]}) self.assertEqual(copy.file, self.audio_url) - def test_attach_voice_payloads_fills_only_voice_rows(self): - from raven.api.raven_message import attach_voice_payloads + def test_attach_json_payloads_fills_only_voice_rows(self): + from raven.api.raven_message import attach_json_payloads voice = self._send(self.audio_url, {"duration": 7.25, "waveform": [1, 2]}) text = send_message_with_attachments(channel_id=self.channel.name, content="

hi

")[0] @@ -193,7 +193,7 @@ def test_attach_voice_payloads_fills_only_voice_rows(self): filters={"name": ["in", [voice.name, text.name]]}, fields=["name", "message_type"], ) - by_name = {row.name: row for row in attach_voice_payloads(rows)} + by_name = {row.name: row for row in attach_json_payloads(rows)} self.assertEqual( frappe.parse_json(by_name[voice.name].json), {"duration": 7.25, "waveform": [1, 2]} ) @@ -209,11 +209,11 @@ def test_saved_messages_carry_the_voice_payload(self): saved = next(row for row in get_saved_messages() if row.name == voice.name) self.assertEqual(frappe.parse_json(saved.json), {"duration": 7.25, "waveform": [1, 2]}) - def test_attach_voice_payloads_reads_other_users_notes(self): - from raven.api.raven_message import attach_voice_payloads + def test_attach_json_payloads_reads_other_users_notes(self): + from raven.api.raven_message import attach_json_payloads voice = self._send(self.audio_url, {"duration": 7.25, "waveform": [1, 2]}) # Another member reads it: message list permissions are owner-only, so the helper must not apply them. frappe.set_user("test@example.com") - rows = attach_voice_payloads([frappe._dict(name=voice.name, message_type="Voice")]) + rows = attach_json_payloads([frappe._dict(name=voice.name, message_type="Voice")]) self.assertEqual(frappe.parse_json(rows[0].json), {"duration": 7.25, "waveform": [1, 2]}) From a2bc9abbb52b4be062d5f325e23dbb4736649717 Mon Sep 17 00:00:00 2001 From: Aditya Patil Date: Mon, 26 Oct 2026 16:05:00 +0530 Subject: [PATCH 02/10] WIP: location payload rules, device position fix and snapshot tile maths --- apps/web/package.json | 6 +- apps/web/src/lib/location.test.ts | 88 ++++++++ apps/web/src/lib/location.ts | 123 +++++++++++ apps/web/src/lib/locationSupport.test.ts | 62 ++++++ apps/web/src/lib/locationSupport.ts | 16 ++ apps/web/src/native/geolocation.test.ts | 182 ++++++++++++++++ apps/web/src/native/geolocation.ts | 104 ++++++++++ packages/types/common/Message.ts | 13 +- raven/api/location.py | 50 +++++ raven/api/raven_message.py | 123 ++++------- .../doctype/raven_message/raven_message.py | 16 +- raven/tests/test_location_messages.py | 196 ++++++++++++------ yarn.lock | 29 +++ 13 files changed, 855 insertions(+), 153 deletions(-) create mode 100644 apps/web/src/lib/location.test.ts create mode 100644 apps/web/src/lib/location.ts create mode 100644 apps/web/src/lib/locationSupport.test.ts create mode 100644 apps/web/src/lib/locationSupport.ts create mode 100644 apps/web/src/native/geolocation.test.ts create mode 100644 apps/web/src/native/geolocation.ts create mode 100644 raven/api/location.py diff --git a/apps/web/package.json b/apps/web/package.json index 9142f9f40..480020cd7 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -20,8 +20,10 @@ "dependencies": { "@capacitor-firebase/messaging": "8.4.0", "@capacitor/app": "8.1.1", + "@capacitor/app-launcher": "8.0.1", "@capacitor/core": "8.5.0", "@capacitor/filesystem": "8.1.3", + "@capacitor/geolocation": "8.2.4", "@capacitor/haptics": "8.0.2", "@capacitor/keyboard": "8.0.5", "@capacitor/preferences": "8.0.1", @@ -65,8 +67,8 @@ "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "cmdk": "^1.1.1", - "date-fns": "^4.4.0", "cronstrue": "^3.27.0", + "date-fns": "^4.4.0", "dayjs": "^1.11.21", "dexie": "^4.4.3", "dexie-react-hooks": "^4.4.0", @@ -78,6 +80,7 @@ "jotai": "^2.20.0", "jotai-family": "^1.0.2", "js-cookie": "^3.0.8", + "leaflet": "^1.9.4", "linkifyjs": "^4.3.3", "lowlight": "^3.3.0", "lucide-react": "^1.17.0", @@ -116,6 +119,7 @@ "@eslint/js": "^9.39.1", "@raven/types": "*", "@types/js-cookie": "^3.0.6", + "@types/leaflet": "^1.9.22", "@types/node": "^25.3.0", "@types/react": "^19.2.15", "@types/react-dom": "^19.2.3", diff --git a/apps/web/src/lib/location.test.ts b/apps/web/src/lib/location.test.ts new file mode 100644 index 000000000..3f4749130 --- /dev/null +++ b/apps/web/src/lib/location.test.ts @@ -0,0 +1,88 @@ +import { afterEach, describe, expect, it, vi } from "vitest" +import { + MAP_ATTRIBUTION, + MAP_TILE_URL, + appleMapsAppUrl, + appleMapsUrl, + formatCoordinates, + geoUri, + googleMapsAppUrl, + googleMapsUrl, + parseLocationJson, + tilesFor, + wazeAppUrl, + type LocationMeta, +} from "./location" + +afterEach(() => vi.unstubAllGlobals()) + +describe("parseLocationJson", () => { + it("reads objects and JSON strings", () => { + expect(parseLocationJson({ latitude: 1, longitude: 2 })).toEqual({ latitude: 1, longitude: 2 }) + expect(parseLocationJson('{"latitude":1,"longitude":2}')).toEqual({ latitude: 1, longitude: 2 }) + }) + it("rejects bad coordinates and junk", () => { + for (const junk of [null, "x", "{", [], { latitude: 91, longitude: 0 }, { latitude: 0, longitude: -181 }, { latitude: Infinity, longitude: 0 }, { latitude: "1", longitude: 2 }, { latitude: NaN, longitude: 0 }]) + expect(parseLocationJson(junk)).toBeNull() + }) + it("drops extra keys", () => { + const json = { latitude: 1, longitude: 2, name: "Cafe", address: "Road", foo: {} } + expect(parseLocationJson(json)).toEqual({ latitude: 1, longitude: 2 }) + }) +}) + +describe("tilesFor", () => { + it("centres the point in the box and covers it", () => { + const tiles = tilesFor(12.9756, 77.6066, 16, 64, 64, 128) + expect(tiles.length).toBeGreaterThan(0) + expect(tiles.length).toBeLessThanOrEqual(4) + expect(Math.min(...tiles.map((t) => t.left))).toBeLessThanOrEqual(0) + expect(Math.max(...tiles.map((t) => t.left + t.size))).toBeGreaterThanOrEqual(64) + expect(tiles[0].url).toMatch(/^https:\/\/tile\.openstreetmap\.org\/16\/\d+\/\d+\.png$/) + }) + it("wraps x at the antimeridian", () => { + const xs = tilesFor(0, 179.9999, 2, 300, 100).map((t) => Number(t.url.split("/")[4])) + expect(xs.every((x) => x >= 0 && x < 4)).toBe(true) + }) + it("does not read the boot", () => { + vi.stubGlobal("window", { + get frappe() { + throw new Error("boot is read") + }, + }) + expect(() => tilesFor(12.9756, 77.6066, 16, 64, 64)).not.toThrow() + }) +}) + +describe("map constants", () => { + it("use OpenStreetMap", () => { + expect(MAP_TILE_URL).toBe("https://tile.openstreetmap.org/{z}/{x}/{y}.png") + expect(MAP_ATTRIBUTION).toBe("Β© OpenStreetMap contributors") + }) +}) + +describe("map URLs", () => { + const meta = { latitude: 12.97561, longitude: 77.60659 } + it("builds web links", () => { + expect(googleMapsUrl(meta)).toBe("https://www.google.com/maps/search/?api=1&query=12.97561,77.60659") + expect(appleMapsUrl(meta)).toBe("https://maps.apple.com/?ll=12.97561,77.60659&q=Location") + }) + it("builds app links", () => { + expect(geoUri(meta)).toBe("geo:12.97561,77.60659?q=12.97561,77.60659(Location)") + expect(googleMapsAppUrl(meta)).toBe("comgooglemaps://?q=12.97561,77.60659¢er=12.97561,77.60659") + expect(wazeAppUrl(meta)).toBe("waze://?ll=12.97561,77.60659&navigate=no") + expect(appleMapsAppUrl(meta)).toBe("maps://?ll=12.97561,77.60659&q=Location") + }) + it("ignores a name carried in the payload", () => { + const named = { ...meta, name: "MG Road" } as LocationMeta + expect(appleMapsUrl(named)).toContain("q=Location") + expect(appleMapsAppUrl(named)).toContain("q=Location") + expect(geoUri(named)).toContain("(Location)") + }) +}) + +describe("formatCoordinates", () => { + it("formats to 5 decimal places", () => { + expect(formatCoordinates({ latitude: 12.975612, longitude: 77.6 })).toBe("12.97561, 77.60000") + }) +}) diff --git a/apps/web/src/lib/location.ts b/apps/web/src/lib/location.ts new file mode 100644 index 000000000..fecfeaae2 --- /dev/null +++ b/apps/web/src/lib/location.ts @@ -0,0 +1,123 @@ +import { isIOS } from "@utils/platform" + +/** A shared location's payload, stored in the message's `json`. */ +export type LocationMeta = { + latitude: number + longitude: number +} + +export const MAP_TILE_URL = "https://tile.openstreetmap.org/{z}/{x}/{y}.png" +export const MAP_ATTRIBUTION = "Β© OpenStreetMap contributors" +export const THUMBNAIL_ZOOM = 16 + +export type MapTile = { url: string; left: number; top: number; size: number } + +const MAX_LATITUDE = 85.05112878 + +const isFiniteNumber = (value: unknown): value is number => + typeof value === "number" && Number.isFinite(value) + +// Mirror of _sane_coordinate in raven/api/location.py β€” keep in sync. +/** A finite latitude/longitude pair inside the WGS84 bounds. */ +const inCoordinateRange = (latitude: number, longitude: number): boolean => + isFiniteNumber(latitude) && + latitude >= -90 && + latitude <= 90 && + isFiniteNumber(longitude) && + longitude >= -180 && + longitude <= 180 + +/** A location's `json` (object, or string from a fetch); null unless it has usable coordinates. */ +export const parseLocationJson = (json: unknown): LocationMeta | null => { + let value = json + if (typeof value === "string") { + try { + value = JSON.parse(value) + } catch { + return null + } + } + if (!value || typeof value !== "object" || Array.isArray(value)) return null + + const { latitude, longitude } = value as Record + if (typeof latitude !== "number" || typeof longitude !== "number") return null + if (!inCoordinateRange(latitude, longitude)) return null + return { latitude, longitude } +} + +const mercatorPixel = (latitude: number, longitude: number, zoom: number, tileSize: number) => { + const worldSize = tileSize * 2 ** zoom + const clampedLatitude = Math.max(-MAX_LATITUDE, Math.min(MAX_LATITUDE, latitude)) + const radians = (clampedLatitude * Math.PI) / 180 + const x = ((longitude + 180) / 360) * worldSize + const y = ((1 - Math.log(Math.tan(radians) + 1 / Math.cos(radians)) / Math.PI) / 2) * worldSize + return { x, y } +} + +const tileUrl = (zoom: number, x: number, y: number): string => + MAP_TILE_URL.replace("{z}", String(zoom)) + .replace("{x}", String(x)) + .replace("{y}", String(y)) + +/** OSM tiles covering a `width Γ— height` box with the point at its centre. */ +export const tilesFor = ( + latitude: number, + longitude: number, + zoom: number, + width: number, + height: number, + tileSize = 256, +): MapTile[] => { + const { x: pixelX, y: pixelY } = mercatorPixel(latitude, longitude, zoom, tileSize) + const count = 2 ** zoom + const firstX = Math.floor((pixelX - width / 2) / tileSize) + const lastX = Math.ceil((pixelX + width / 2) / tileSize) - 1 + const firstY = Math.floor((pixelY - height / 2) / tileSize) + const lastY = Math.ceil((pixelY + height / 2) / tileSize) - 1 + + const tiles: MapTile[] = [] + for (let tileX = firstX; tileX <= lastX; tileX++) { + const wrappedX = ((tileX % count) + count) % count + for (let tileY = firstY; tileY <= lastY; tileY++) { + if (tileY < 0 || tileY >= count) continue + tiles.push({ + url: tileUrl(zoom, wrappedX, tileY), + left: tileX * tileSize - pixelX + width / 2, + top: tileY * tileSize - pixelY + height / 2, + size: tileSize, + }) + } + } + return tiles +} + +const coordinates = (meta: Pick): string => + `${meta.latitude},${meta.longitude}` + +/** `"12.97561, 77.60659"` (5 decimal places). */ +export const formatCoordinates = (meta: Pick): string => + `${meta.latitude.toFixed(5)}, ${meta.longitude.toFixed(5)}` + +// Mirror of maps_url in raven/api/location.py β€” keep in sync. +export const googleMapsUrl = (meta: LocationMeta): string => + `https://www.google.com/maps/search/?api=1&query=${coordinates(meta)}` + +export const appleMapsUrl = (meta: LocationMeta): string => + `https://maps.apple.com/?ll=${coordinates(meta)}&q=Location` + +export const appleMapsAppUrl = (meta: LocationMeta): string => + `maps://?ll=${coordinates(meta)}&q=Location` + +export const googleMapsAppUrl = (meta: LocationMeta): string => + `comgooglemaps://?q=${coordinates(meta)}¢er=${coordinates(meta)}` + +export const wazeAppUrl = (meta: LocationMeta): string => + `waze://?ll=${coordinates(meta)}&navigate=no` + +export const geoUri = (meta: LocationMeta): string => + `geo:${coordinates(meta)}?q=${coordinates(meta)}(Location)` + +export const senderMapsProvider = (): "apple" | "google" => (isIOS ? "apple" : "google") + +export const senderMapsUrl = (meta: LocationMeta): string => + senderMapsProvider() === "apple" ? appleMapsUrl(meta) : googleMapsUrl(meta) diff --git a/apps/web/src/lib/locationSupport.test.ts b/apps/web/src/lib/locationSupport.test.ts new file mode 100644 index 000000000..d15f7e802 --- /dev/null +++ b/apps/web/src/lib/locationSupport.test.ts @@ -0,0 +1,62 @@ +import { afterEach, describe, expect, it, vi } from "vitest" + +// isNative reads the bridge global; node has no window. +afterEach(() => vi.unstubAllGlobals()) + +const nativeWindow = (boot: Record, platform = "ios") => + vi.stubGlobal("window", { + Capacitor: { isNativePlatform: () => true, getPlatform: () => platform }, + frappe: { boot }, + }) + +const freshModule = async () => { + vi.resetModules() + return import("./locationSupport") +} + +describe("siteSupportsLocationMessages", () => { + it("web always supports location messages", async () => { + vi.stubGlobal("window", {}) + const { siteSupportsLocationMessages } = await import("./locationSupport") + expect(siteSupportsLocationMessages()).toBe(true) + }) + + it("native needs the boot flag", async () => { + nativeWindow({}) + const { siteSupportsLocationMessages } = await import("./locationSupport") + expect(siteSupportsLocationMessages()).toBe(false) + + nativeWindow({ raven_features: ["location_messages"] }) + expect(siteSupportsLocationMessages()).toBe(true) + }) + + it("native ignores a non-array flag", async () => { + nativeWindow({ raven_features: "location_messages" }) + const { siteSupportsLocationMessages } = await import("./locationSupport") + expect(siteSupportsLocationMessages()).toBe(false) + }) +}) + +describe("canShareLocation", () => { + it("is true in the native app", async () => { + nativeWindow({}) + const { canShareLocation } = await freshModule() + expect(canShareLocation()).toBe(true) + }) + + it("needs a secure context and the geolocation API on the web", async () => { + vi.stubGlobal("window", { isSecureContext: true }) + vi.stubGlobal("navigator", { geolocation: {} }) + const supported = await freshModule() + expect(supported.canShareLocation()).toBe(true) + + vi.stubGlobal("window", { isSecureContext: false }) + const insecure = await freshModule() + expect(insecure.canShareLocation()).toBe(false) + + vi.stubGlobal("window", { isSecureContext: true }) + vi.stubGlobal("navigator", {}) + const noApi = await freshModule() + expect(noApi.canShareLocation()).toBe(false) + }) +}) diff --git a/apps/web/src/lib/locationSupport.ts b/apps/web/src/lib/locationSupport.ts new file mode 100644 index 000000000..cc1e7403b --- /dev/null +++ b/apps/web/src/lib/locationSupport.ts @@ -0,0 +1,16 @@ +import { isNative } from "../native/platform" + +let canShare: boolean | undefined + +/** The device exposes a location API. A browser stays hidden until asked, so a denial is not cached. */ +export const canShareLocation = (): boolean => { + canShare ??= isNative() || (!!window.isSecureContext && "geolocation" in navigator) + return canShare +} + +/** Web always serves the location client; the installed app needs the site to advertise the feature. */ +export const siteSupportsLocationMessages = (): boolean => { + if (!isNative()) return true + const features = window.frappe?.boot?.raven_features + return Array.isArray(features) && features.includes("location_messages") +} diff --git a/apps/web/src/native/geolocation.test.ts b/apps/web/src/native/geolocation.test.ts new file mode 100644 index 000000000..9ce3df0eb --- /dev/null +++ b/apps/web/src/native/geolocation.test.ts @@ -0,0 +1,182 @@ +import { afterEach, describe, expect, it, vi } from "vitest" + +const HIGH_ACCURACY = { enableHighAccuracy: true, timeout: 15000, maximumAge: 30000 } +const LOW_ACCURACY = { enableHighAccuracy: false, timeout: 10000, maximumAge: 60000 } + +const position = (accuracy: number) => ({ + coords: { latitude: 12.9716, longitude: 77.5946, accuracy }, +}) + +afterEach(() => { + vi.unstubAllGlobals() + vi.useRealTimers() + vi.doUnmock("@capacitor/geolocation") + vi.resetModules() +}) + +const webModule = async (getCurrentPosition: ReturnType) => { + vi.stubGlobal("window", {}) + vi.stubGlobal("navigator", { geolocation: { getCurrentPosition } }) + vi.resetModules() + return import("./geolocation") +} + +type NativeMocks = { + requestPermissions?: ReturnType + getCurrentPosition?: ReturnType +} + +const nativeModule = async (mocks: NativeMocks = {}) => { + const requestPermissions = mocks.requestPermissions ?? vi.fn(async () => ({ location: "granted" })) + const getCurrentPosition = mocks.getCurrentPosition ?? vi.fn(async () => position(5)) + vi.stubGlobal("window", { Capacitor: { isNativePlatform: () => true } }) + vi.doMock("@capacitor/geolocation", () => ({ + Geolocation: { requestPermissions, getCurrentPosition }, + })) + vi.resetModules() + const { getCurrentPosition: currentPosition } = await import("./geolocation") + return { currentPosition, requestPermissions, getCurrentPosition } +} + +describe("web geolocation", () => { + it("returns the fix on success", async () => { + const getCurrentPosition = vi.fn((success) => success(position(12))) + const { getCurrentPosition: currentPosition } = await webModule(getCurrentPosition) + + await expect(currentPosition()).resolves.toEqual({ + fix: { latitude: 12.9716, longitude: 77.5946, accuracy: 12 }, + }) + expect(getCurrentPosition).toHaveBeenCalledTimes(1) + expect(getCurrentPosition).toHaveBeenCalledWith(expect.any(Function), expect.any(Function), HIGH_ACCURACY) + }) + + it("does not retry when permission is denied", async () => { + const getCurrentPosition = vi.fn((_success, error) => error({ code: 1 })) + const { getCurrentPosition: currentPosition } = await webModule(getCurrentPosition) + + await expect(currentPosition()).resolves.toEqual({ error: "denied" }) + expect(getCurrentPosition).toHaveBeenCalledTimes(1) + }) + + it("retries a timeout with low accuracy and returns that fix", async () => { + const getCurrentPosition = vi.fn((success, error, options) => { + if (options.enableHighAccuracy) error({ code: 3 }) + else success(position(250)) + }) + const { getCurrentPosition: currentPosition } = await webModule(getCurrentPosition) + + await expect(currentPosition()).resolves.toEqual({ + fix: { latitude: 12.9716, longitude: 77.5946, accuracy: 250 }, + }) + expect(getCurrentPosition).toHaveBeenCalledTimes(2) + expect(getCurrentPosition.mock.calls[1][2]).toEqual(LOW_ACCURACY) + }) + + it("returns unavailable when both attempts fail", async () => { + const getCurrentPosition = vi.fn((_success, error, _options) => error({ code: 2 })) + const { getCurrentPosition: currentPosition } = await webModule(getCurrentPosition) + + await expect(currentPosition()).resolves.toEqual({ error: "unavailable" }) + expect(getCurrentPosition).toHaveBeenCalledTimes(2) + expect(getCurrentPosition.mock.calls[1][2]).toEqual(LOW_ACCURACY) + }) + + it("returns unavailable without the geolocation API", async () => { + vi.stubGlobal("window", {}) + vi.stubGlobal("navigator", {}) + vi.resetModules() + const { getCurrentPosition } = await import("./geolocation") + + await expect(getCurrentPosition()).resolves.toEqual({ error: "unavailable" }) + }) + + it("times out at 30s and ignores a late callback", async () => { + vi.useFakeTimers() + let succeed: ((fix: ReturnType) => void) | undefined + const getCurrentPosition = vi.fn((success) => { + succeed = success + }) + const { getCurrentPosition: currentPosition } = await webModule(getCurrentPosition) + + const result = currentPosition() + await vi.advanceTimersByTimeAsync(30000) + + await expect(result).resolves.toEqual({ error: "timeout" }) + + succeed?.(position(8)) + await vi.advanceTimersByTimeAsync(0) + + await expect(result).resolves.toEqual({ error: "timeout" }) + }) +}) + +describe("native geolocation", () => { + it("returns denied when the permission prompt is denied", async () => { + const requestPermissions = vi.fn(async () => ({ location: "denied" })) + const { currentPosition, getCurrentPosition } = await nativeModule({ requestPermissions }) + + await expect(currentPosition()).resolves.toEqual({ error: "denied" }) + expect(getCurrentPosition).not.toHaveBeenCalled() + }) + + it("maps an OS-PLUG-GLOC-0003 error to denied", async () => { + const getCurrentPosition = vi.fn(async () => { + throw { code: "OS-PLUG-GLOC-0003" } + }) + const { currentPosition } = await nativeModule({ getCurrentPosition }) + + await expect(currentPosition()).resolves.toEqual({ error: "denied" }) + expect(getCurrentPosition).toHaveBeenCalledTimes(1) + }) + + it("retries an OS-PLUG-GLOC-0010 timeout with low accuracy", async () => { + const getCurrentPosition = vi + .fn() + .mockRejectedValueOnce({ code: "OS-PLUG-GLOC-0010" }) + .mockResolvedValueOnce(position(180)) + const { currentPosition } = await nativeModule({ getCurrentPosition }) + + await expect(currentPosition()).resolves.toEqual({ + fix: { latitude: 12.9716, longitude: 77.5946, accuracy: 180 }, + }) + expect(getCurrentPosition).toHaveBeenCalledTimes(2) + expect(getCurrentPosition.mock.calls[1][0]).toEqual(LOW_ACCURACY) + }) + + it("returns unavailable when both attempts fail", async () => { + const getCurrentPosition = vi.fn().mockRejectedValue({ code: "OS-PLUG-GLOC-0007" }) + const { currentPosition } = await nativeModule({ getCurrentPosition }) + + await expect(currentPosition()).resolves.toEqual({ error: "unavailable" }) + expect(getCurrentPosition).toHaveBeenCalledTimes(2) + }) + + it("returns unavailable for an unknown error", async () => { + const getCurrentPosition = vi.fn().mockRejectedValue(new Error("boom")) + const { currentPosition } = await nativeModule({ getCurrentPosition }) + + await expect(currentPosition()).resolves.toEqual({ error: "unavailable" }) + }) + + it("maps a message-only OS-PLUG-GLOC-0008 error to denied", async () => { + const getCurrentPosition = vi.fn(async () => { + throw { message: "Location permission failed: OS-PLUG-GLOC-0008" } + }) + const { currentPosition } = await nativeModule({ getCurrentPosition }) + + await expect(currentPosition()).resolves.toEqual({ error: "denied" }) + expect(getCurrentPosition).toHaveBeenCalledTimes(1) + }) + + it("times out at 30s while the permission prompt is open", async () => { + vi.useFakeTimers() + const requestPermissions = vi.fn(() => new Promise(() => { })) + const { currentPosition, getCurrentPosition } = await nativeModule({ requestPermissions }) + + const result = currentPosition() + await vi.advanceTimersByTimeAsync(30000) + + await expect(result).resolves.toEqual({ error: "timeout" }) + expect(getCurrentPosition).not.toHaveBeenCalled() + }) +}) diff --git a/apps/web/src/native/geolocation.ts b/apps/web/src/native/geolocation.ts new file mode 100644 index 000000000..af2717727 --- /dev/null +++ b/apps/web/src/native/geolocation.ts @@ -0,0 +1,104 @@ +import { isNative } from "./platform" + +export type PositionFix = { latitude: number; longitude: number; accuracy: number } +export type FixError = "denied" | "unavailable" | "timeout" +export type PositionResult = { fix: PositionFix } | { error: FixError } + +const HIGH_ACCURACY = { enableHighAccuracy: true, timeout: 15000, maximumAge: 30000 } +const LOW_ACCURACY = { enableHighAccuracy: false, timeout: 10000, maximumAge: 60000 } +// The API's own timeout excludes the permission prompt; Firefox's "Not now" never calls back. +const FIX_WATCHDOG_MS = 30000 + +const fixOf = (coords: { latitude: number; longitude: number; accuracy: number }): PositionFix => ({ + latitude: coords.latitude, + longitude: coords.longitude, + accuracy: coords.accuracy, +}) + +const webError = (error: GeolocationPositionError): FixError => { + if (error.code === 1) return "denied" + if (error.code === 3) return "timeout" + return "unavailable" +} + +const webPosition = (options: PositionOptions): Promise => + new Promise((resolve) => { + if (!navigator.geolocation) { + resolve({ error: "unavailable" }) + return + } + navigator.geolocation.getCurrentPosition( + (position) => resolve({ fix: fixOf(position.coords) }), + (error) => resolve({ error: webError(error) }), + options, + ) + }) + +// @capacitor/geolocation's OS-PLUG-GLOC error table. +const GLOC_DENIED = 3 +const GLOC_RESTRICTED = 8 +const GLOC_TIMEOUT = 10 + +const nativeErrorCode = (error: unknown): number | null => { + const record = (error ?? {}) as { code?: unknown; message?: unknown } + const text = + typeof record.code === "string" ? record.code : typeof record.message === "string" ? record.message : String(error) + const match = /OS-PLUG-GLOC-(\d{4})/.exec(text) + return match ? Number(match[1]) : null +} + +const nativeError = (error: unknown): FixError => { + const code = nativeErrorCode(error) + if (code === GLOC_TIMEOUT) return "timeout" + if (code === GLOC_DENIED || code === GLOC_RESTRICTED) return "denied" + return "unavailable" +} + +// Indoor and laptop GPS often times out at high accuracy; a coarse fix is still useful. +const withFallback = async ( + attempt: (options: PositionOptions) => Promise, +): Promise => { + const first = await attempt(HIGH_ACCURACY) + // A denied permission stays denied, so retrying would only re-prompt. + if ("fix" in first || first.error === "denied") return first + return attempt(LOW_ACCURACY) +} + +const webRun = (): Promise => withFallback(webPosition) + +const nativeRun = async (): Promise => { + try { + const { Geolocation } = await import("@capacitor/geolocation") + const permission = await Geolocation.requestPermissions() + if (permission.location === "denied") return { error: "denied" } + const attempt = async (options: PositionOptions): Promise => { + try { + const position = await Geolocation.getCurrentPosition(options) + return { fix: fixOf(position.coords) } + } catch (error) { + return { error: nativeError(error) } + } + } + return withFallback(attempt) + } catch (error) { + return { error: nativeError(error) } + } +} + +const withWatchdog = (run: () => Promise): Promise => + new Promise((resolve) => { + let settled = false + let timer: ReturnType + const settle = (result: PositionResult) => { + if (settled) return + settled = true + clearTimeout(timer) + resolve(result) + } + timer = setTimeout(() => settle({ error: "timeout" }), FIX_WATCHDOG_MS) + run().then(settle, () => settle({ error: "unavailable" })) + }) + +/** Current device position; resolves with a typed error rather than rejecting. */ +export const getCurrentPosition = (): Promise => + withWatchdog(() => (isNative() ? nativeRun() : webRun())) diff --git a/packages/types/common/Message.ts b/packages/types/common/Message.ts index 5841b50ad..e496762a2 100644 --- a/packages/types/common/Message.ts +++ b/packages/types/common/Message.ts @@ -1,4 +1,4 @@ -export type Message = FileMessage | TextMessage | ImageMessage | PollMessage | SystemMessage | VoiceMessage +export type Message = FileMessage | TextMessage | ImageMessage | PollMessage | SystemMessage | VoiceMessage | LocationMessage export interface BaseMessage { name: string, @@ -7,7 +7,7 @@ export interface BaseMessage { channel_id: string, creation: string, modified: string, - message_type: 'Text' | 'File' | 'Image' | 'Poll' | 'System' | 'Voice', + message_type: 'Text' | 'File' | 'Image' | 'Poll' | 'System' | 'Voice' | 'Location', message_reactions?: string | null, is_continuation: 1 | 0 /** Stable React key for the stream row (stream selector decoration). A @@ -34,7 +34,8 @@ export interface BaseMessage { content?: string, isOpenInThread?: boolean, /** Type-specific payload: a Voice message's `{ duration, waveform }` (read with parseVoiceJson), - * or system payloads (converted_to_channel_*, forwarded_thread). String when fetched, object when live. */ + * a Location message's point (read with parseLocationJson), or system payloads + * (converted_to_channel_*, forwarded_thread). String when fetched, object when live. */ json?: string | Record, /** Shared id stamped on messages sent together as one batch (e.g. multiple files). Consecutive messages with the same id render as one block. */ message_batch_id?: string | null, @@ -58,6 +59,12 @@ export interface VoiceMessage extends BaseMessage { file_size?: number, } +/** A shared location: `{ latitude, longitude }` in `json` (see parseLocationJson). */ +export interface LocationMessage extends BaseMessage { + text: string, + message_type: 'Location' +} + export interface ImageMessage extends BaseMessage { text: string, file: string, diff --git a/raven/api/location.py b/raven/api/location.py new file mode 100644 index 000000000..18e9ba965 --- /dev/null +++ b/raven/api/location.py @@ -0,0 +1,50 @@ +import math + +import frappe + + +def _sane_number(value) -> float | None: + """A finite number from an untrusted client, or None.""" + if isinstance(value, bool): + return None + try: + number = float(value) + except (TypeError, ValueError): + return None + return number if math.isfinite(number) else None + + +# Mirror of inCoordinateRange in apps/web/src/lib/location.ts β€” keep in sync. +def _sane_coordinate(value, limit: float) -> float | None: + """A coordinate within Β±limit, rounded to ~0.1 m, or None.""" + number = _sane_number(value) + if number is None or not -limit <= number <= limit: + return None + return round(number, 6) + + +def sane_location(value) -> dict | None: + """A plausible location payload from an untrusted client, or None.""" + if not isinstance(value, dict): + return None + latitude = _sane_coordinate(value.get("latitude"), 90) + longitude = _sane_coordinate(value.get("longitude"), 180) + if latitude is None or longitude is None: + return None + return {"latitude": latitude, "longitude": longitude} + + +# Mirror of appleMapsUrl / googleMapsUrl in apps/web/src/lib/location.ts β€” keep in sync. +def maps_url(location: dict, provider: str | None = None) -> str: + """The maps link for a location, in the sender's app; Google unless Apple is asked for.""" + latitude = location["latitude"] + longitude = location["longitude"] + if provider == "apple": + return f"https://maps.apple.com/?ll={latitude},{longitude}&q=Location" + return f"https://www.google.com/maps/search/?api=1&query={latitude},{longitude}" + + +def location_link_html(url: str) -> str: + """A plain link for clients that can't draw the location card.""" + escaped = frappe.utils.escape_html(url) + return f'

{escaped}

' diff --git a/raven/api/raven_message.py b/raven/api/raven_message.py index ae2f43b36..e162795e5 100644 --- a/raven/api/raven_message.py +++ b/raven/api/raven_message.py @@ -1,5 +1,4 @@ import math -import urllib.parse import frappe from frappe import _ @@ -8,6 +7,7 @@ from frappe.utils import cint from raven.api.chat_stream import message_columns +from raven.api.location import location_link_html, maps_url, sane_location from raven.api.raven_channel import create_direct_message_channel, get_peer_user_id_from_dm_users from raven.api.search import channel_access_condition from raven.utils import desk_has_raven_icon, get_channel_member @@ -61,10 +61,6 @@ def send_message( _MAX_AUDIO_SECONDS = 660 _MAX_WAVEFORM_BARS = 64 -_MAX_LOCATION_NAME = 120 -_MAX_LOCATION_ADDRESS = 300 -_MAX_LOCATION_ACCURACY = 100_000 - # Inline-image display caps (mirror the web client's reserved box). The stored # aspect ratio is what prevents reflow; the absolute thumbnail size only needs # to be reasonable. The client mirrors these values for its optimistic @@ -113,68 +109,6 @@ def _sane_waveform(value) -> list[int] | None: return bars -def _sane_number(value) -> float | None: - """A finite number from an untrusted client, or None.""" - if isinstance(value, bool): - return None - try: - number = float(value) - except (TypeError, ValueError): - return None - return number if math.isfinite(number) else None - - -def _sane_coordinate(value, limit: float) -> float | None: - """A coordinate within Β±limit, rounded to ~0.1 m, or None.""" - number = _sane_number(value) - if number is None or not -limit <= number <= limit: - return None - return round(number, 6) - - -def _sane_location_text(value, limit: int) -> str: - """Plain text from an untrusted client: tags stripped, trimmed, capped.""" - return frappe.utils.strip_html_tags(str(value or "")).strip()[:limit] - - -def _sane_location(value) -> dict | None: - """A plausible location payload from an untrusted client, or None.""" - if not isinstance(value, dict): - return None - latitude = _sane_coordinate(value.get("latitude"), 90) - longitude = _sane_coordinate(value.get("longitude"), 180) - if latitude is None or longitude is None: - return None - - location = {"latitude": latitude, "longitude": longitude} - accuracy = _sane_number(value.get("accuracy")) - if accuracy is not None and 0 < accuracy <= _MAX_LOCATION_ACCURACY: - location["accuracy"] = accuracy - name = _sane_location_text(value.get("name"), _MAX_LOCATION_NAME) - if name: - location["name"] = name - address = _sane_location_text(value.get("address"), _MAX_LOCATION_ADDRESS) - if address: - location["address"] = address - return location - - -def _maps_url(location: dict, maps: str | None = None) -> str: - """The maps link for a location, in the sender's app; Google unless Apple is asked for.""" - latitude = location["latitude"] - longitude = location["longitude"] - if maps == "apple": - label = urllib.parse.quote(location.get("name") or "Location", safe="-_.!~*'()") - return f"https://maps.apple.com/?ll={latitude},{longitude}&q={label}" - return f"https://www.google.com/maps/search/?api=1&query={latitude},{longitude}" - - -def _location_text(url: str) -> str: - """A plain link for clients that can't draw the location card.""" - escaped = frappe.utils.escape_html(url) - return f'

{escaped}

' - - _JSON_PAYLOAD_TYPES = ("Voice", "Location") @@ -199,6 +133,14 @@ def attach_json_payloads(rows: list[dict], id_field: str = "name") -> list[dict] return rows +def _attachment_url(file: dict | str) -> str: + return file["file_url"] if isinstance(file, dict) else file + + +def _attachment_size(file: dict | str) -> int: + return (file.get("file_size") if isinstance(file, dict) else None) or 0 + + def _file_message_type(file_url: str) -> str: return "Image" if _file_extension(file_url) in IMAGE_EXTENSIONS else "File" @@ -353,10 +295,11 @@ def send_message_with_attachments( client when it recorded the clip; it becomes a `Voice` message with that payload in `json`. Unusable metadata sends the audio as a plain file. - `location` is one point (`{"latitude", "longitude", "accuracy"?, "name"?, - "address"?}`) and must be the whole send: no content, files or linked document. - `maps` (`"apple"` or `"google"`) only picks the link stored in `text` for clients - that can't draw the card; it is not kept in `json`. + `location` is one point (`{"latitude", "longitude"}`) and must be the whole send: + no content, no linked document, and at most one file β€” the pre-rendered map + snapshot, which must be an image and lands in `file` / `file_size`. `maps` + (`"apple"` or `"google"`) only picks the link stored in `text` for clients that + can't draw the card; it is not kept in `json`. `send_silently` suppresses notifications for the whole batch (the flag is set on every message before insert). @@ -386,15 +329,21 @@ def send_message_with_attachments( has_linked_document = bool(link_doctype and link_document) if location is not None: - if has_body or files or has_linked_document: + snapshot = files[0] if files else None + if ( + has_body + or len(files) > 1 + or has_linked_document + or (snapshot and _file_message_type(_attachment_url(snapshot)) != "Image") + ): frappe.throw(_("A location is sent on its own")) - sane_location = _sane_location(location) - if not sane_location: + clean_location = sane_location(location) + if not clean_location: frappe.throw(_("Invalid location")) elif not has_body and not files and not has_linked_document: frappe.throw(_("Cannot send an empty message")) - validate_files_uploaded_by_user([f["file_url"] if isinstance(f, dict) else f for f in files]) + validate_files_uploaded_by_user([_attachment_url(f) for f in files]) # Every message from a send carries the client_id as its batch id: it groups # multi-message sends in the UI AND lets the client match the realtime echo of @@ -407,16 +356,20 @@ def send_message_with_attachments( # the client's upload response, so no per-file File lookup is needed here). specs = [] if location is not None: - specs.append( - { - "message_type": "Location", - "json": sane_location, - "text": _location_text(_maps_url(sane_location, location.get("maps"))), - } - ) - for f in files: - file_url = f["file_url"] if isinstance(f, dict) else f - file_size = f.get("file_size") if isinstance(f, dict) else None + location_spec = { + "message_type": "Location", + "json": clean_location, + "text": location_link_html(maps_url(clean_location, location.get("maps"))), + } + if files: + location_spec["file"] = _attachment_url(files[0]) + location_spec["file_size"] = _attachment_size(files[0]) + specs.append(location_spec) + # A location's file is its snapshot, carried on the Location message itself. + attachment_files = files if location is None else [] + for f in attachment_files: + file_url = _attachment_url(f) + file_size = _attachment_size(f) spec = { "message_type": _file_message_type(file_url), "file": file_url, diff --git a/raven/raven_messaging/doctype/raven_message/raven_message.py b/raven/raven_messaging/doctype/raven_message/raven_message.py index b031c15a8..3d3316621 100644 --- a/raven/raven_messaging/doctype/raven_message/raven_message.py +++ b/raven/raven_messaging/doctype/raven_message/raven_message.py @@ -179,13 +179,18 @@ def parse_html_content(self): def queue_link_preview_processing(self): """ - Make sure every link in this message gets a Raven Link Preview doc. + Make sure every link in this message gets a Raven Link Preview doc, + except a location's fallback link. Runs as a background job so a message save never waits on it. Skip when the links did not change. Reaction saves, metadata edits and AI token streaming also pass through on_update. """ if frappe.flags.in_patch or frappe.flags.in_migrate or frappe.flags.in_install: return + # Clients never show a location's fallback link preview: its Google + # page previews the server's location, not the pin. + if self.message_type == "Location": + return if not self.links_table: return old_doc = self.get_doc_before_save() @@ -332,9 +337,9 @@ def before_insert(self): "owner": details.owner, "creation": datetime.datetime.strftime(details.creation, "%Y-%m-%d %H:%M:%S"), } - # A voice note's preview shows its length; a location's shows its point. - # Fetched only then: a text message's `json` is its whole editor document. - if details.message_type in ("Voice", "Location"): + # A voice note's preview shows its length. Fetched only then: a + # text message's `json` is its whole editor document. + if details.message_type == "Voice": snapshot["json"] = json_dict(frappe.db.get_value("Raven Message", self.linked_message, "json")) self.replied_message_details = frappe.as_json(snapshot) @@ -727,8 +732,7 @@ def get_notification_message_content(self): return f"🎀 Sent a voice note ({int(duration // 60)}:{int(duration % 60):02d})" return "🎀 Sent a voice note" elif self.message_type == "Location": - name = json_dict(self.json).get("name") - return f"πŸ“ Shared a location - {name}" if name else "πŸ“ Shared a location" + return "πŸ“ Shared a location" elif self.text: return self.content diff --git a/raven/tests/test_location_messages.py b/raven/tests/test_location_messages.py index 4b12d78dd..43692b9a8 100644 --- a/raven/tests/test_location_messages.py +++ b/raven/tests/test_location_messages.py @@ -1,36 +1,57 @@ +from unittest.mock import patch + import frappe from frappe.tests import IntegrationTestCase +from raven.api.location import maps_url, sane_location from raven.api.raven_message import ( _forward_payloads, - _maps_url, - _sane_location, attach_json_payloads, + get_all_files_shared_in_channel, send_message_with_attachments, ) EXTRA_TEST_RECORD_DEPENDENCIES = ["User", "Raven User"] +# File validates that an uploaded PDF parses, so non-image content must be a real PDF. +MINIMAL_PDF = b"""%PDF-1.4 +1 0 obj +<< /Type /Catalog /Pages 2 0 R >> +endobj +2 0 obj +<< /Type /Pages /Kids [3 0 R] /Count 1 >> +endobj +3 0 obj +<< /Type /Page /Parent 2 0 R /MediaBox [0 0 10 10] >> +endobj +xref +0 4 +0000000000 65535 f +0000000009 00000 n +0000000058 00000 n +0000000115 00000 n +trailer +<< /Size 4 /Root 1 0 R >> +startxref +190 +%%EOF +""" + class TestLocationSanitizer(IntegrationTestCase): - def test_sane_location_keeps_and_rounds(self): + def test_sane_location_keeps_only_coordinates(self): self.assertEqual( - _sane_location( + sane_location( { "latitude": 12.97560123, "longitude": "77.6066", "accuracy": 15.5, "name": " MG Road ", "address": "Bengaluru", + "foo": {"bar": 1}, } ), - { - "latitude": 12.975601, - "longitude": 77.6066, - "accuracy": 15.5, - "name": "MG Road", - "address": "Bengaluru", - }, + {"latitude": 12.975601, "longitude": 77.6066}, ) def test_sane_location_rejects_bad_coordinates(self): @@ -44,40 +65,35 @@ def test_sane_location_rejects_bad_coordinates(self): (0, float("inf")), ) for lat, lng in bad_coordinates: - self.assertIsNone(_sane_location({"latitude": lat, "longitude": lng}), (lat, lng)) + self.assertIsNone(sane_location({"latitude": lat, "longitude": lng}), (lat, lng)) for junk in (None, "x", [1, 2], 5): - self.assertIsNone(_sane_location(junk), junk) + self.assertIsNone(sane_location(junk), junk) - def test_sane_location_drops_bad_optionals(self): - loc = _sane_location( - {"latitude": 1, "longitude": 2, "accuracy": 0, "name": "", "address": " "} + def test_sane_location_drops_extra_keys(self): + loc = sane_location( + { + "latitude": 1, + "longitude": 2, + "accuracy": 0, + "name": "x", + "address": " ", + "foo": {}, + } ) self.assertEqual(loc, {"latitude": 1.0, "longitude": 2.0}) - self.assertIsNone( - _sane_location({"latitude": 1, "longitude": 2, "accuracy": 100001}).get("accuracy") - ) - self.assertEqual( - len(_sane_location({"latitude": 1, "longitude": 2, "name": "x" * 500})["name"]), 120 - ) - self.assertEqual( - _sane_location({"latitude": 1, "longitude": 2, "name": "Cafe"})[ - "name" - ], - "aCafe", - ) def test_maps_url(self): - loc = {"latitude": 12.97561, "longitude": 77.60659, "name": "MG Road & Co"} + loc = {"latitude": 12.97561, "longitude": 77.60659} self.assertEqual( - _maps_url(loc, "apple"), - "https://maps.apple.com/?ll=12.97561,77.60659&q=MG%20Road%20%26%20Co", + maps_url(loc, "apple"), + "https://maps.apple.com/?ll=12.97561,77.60659&q=Location", ) self.assertEqual( - _maps_url(loc, "google"), + maps_url(loc, "google"), "https://www.google.com/maps/search/?api=1&query=12.97561,77.60659", ) - self.assertEqual(_maps_url(loc, "junk"), _maps_url(loc, "google")) - self.assertIn("q=Location", _maps_url({"latitude": 1.0, "longitude": 2.0}, "apple")) + self.assertEqual(maps_url(loc, "junk"), maps_url(loc, "google")) + self.assertNotIn("Cafe", maps_url({**loc, "name": "Cafe"}, "apple")) class TestLocationSend(IntegrationTestCase): @@ -131,13 +147,11 @@ def test_send_creates_location_message(self): ) doc = frappe.get_doc("Raven Message", msg.name) self.assertEqual(doc.message_type, "Location") - self.assertEqual( - frappe.parse_json(doc.json), - {"latitude": 12.97561, "longitude": 77.60659, "accuracy": 15.0, "name": "MG Road"}, - ) + self.assertEqual(frappe.parse_json(doc.json), {"latitude": 12.97561, "longitude": 77.60659}) self.assertIn( 'href="https://www.google.com/maps/search/?api=1&query=12.97561,77.60659"', doc.text ) + self.assertNotIn("MG Road", doc.text) self.assertEqual( doc.content, "https://www.google.com/maps/search/?api=1&query=12.97561,77.60659", @@ -147,7 +161,7 @@ def test_send_creates_location_message(self): "https://www.google.com/maps/search/?api=1&query=12.97561,77.60659", ) - def test_link_text_escapes_name(self): + def test_link_text_ignores_name(self): [msg] = send_message_with_attachments( channel_id=self.channel.name, location={"latitude": 1, "longitude": 2, "name": '">', "maps": "apple"}, @@ -155,6 +169,7 @@ def test_link_text_escapes_name(self): doc = frappe.get_doc("Raven Message", msg.name) self.assertEqual(doc.text.count("hi

"}, - {"files": [{"file_url": "/files/a.png", "file_size": 1}]}, + {"files": [{"file_url": "/files/a.png", "file_size": 1}] * 2}, {"link_doctype": "User", "link_document": "Administrator"}, ) for extra in extras: @@ -177,9 +192,7 @@ def test_location_must_be_alone(self): ) def test_reply_silent_and_idempotent(self): - parent = send_message_with_attachments( - channel_id=self.channel.name, content="

where?

" - )[0] + parent = send_message_with_attachments(channel_id=self.channel.name, content="

where?

")[0] kwargs = dict( channel_id=self.channel.name, client_id="loc-retry", @@ -196,18 +209,11 @@ def test_reply_silent_and_idempotent(self): ) def test_notification_text(self): - [a] = send_message_with_attachments( + [msg] = send_message_with_attachments( channel_id=self.channel.name, location={"latitude": 1, "longitude": 2, "name": "MG Road"} ) - [b] = send_message_with_attachments( - channel_id=self.channel.name, location={"latitude": 1, "longitude": 2} - ) - self.assertEqual( - frappe.get_doc("Raven Message", a.name).get_notification_message_content(), - "πŸ“ Shared a location - MG Road", - ) self.assertEqual( - frappe.get_doc("Raven Message", b.name).get_notification_message_content(), + frappe.get_doc("Raven Message", msg.name).get_notification_message_content(), "πŸ“ Shared a location", ) @@ -218,10 +224,10 @@ def test_json_attached_for_location_rows(self): rows = attach_json_payloads([frappe._dict(name=loc.name, message_type="Location")]) self.assertEqual(frappe.parse_json(rows[0]["json"])["latitude"], 1.0) - def test_reply_snapshot_carries_location_json(self): + def test_reply_snapshot_has_no_location_json(self): [loc] = send_message_with_attachments( channel_id=self.channel.name, - location={"latitude": 1, "longitude": 2, "name": "Cafe"}, + location={"latitude": 1, "longitude": 2}, ) reply = send_message_with_attachments( channel_id=self.channel.name, @@ -233,12 +239,36 @@ def test_reply_snapshot_carries_location_json(self): frappe.db.get_value("Raven Message", reply.name, "replied_message_details") ) self.assertEqual(snapshot["message_type"], "Location") - self.assertEqual(frappe.parse_json(snapshot["json"])["name"], "Cafe") + self.assertNotIn("json", snapshot) + + def _enqueued_link_previews(self, enqueue): + return [ + call + for call in enqueue.call_args_list + if call.args and call.args[0] == "raven.links.process_message_links" + ] + + def test_location_skips_link_preview(self): + with patch( + "raven.raven_messaging.doctype.raven_message.raven_message.frappe.enqueue" + ) as enqueue: + send_message_with_attachments( + channel_id=self.channel.name, location={"latitude": 1, "longitude": 2} + ) + self.assertEqual(self._enqueued_link_previews(enqueue), []) + + def test_text_with_link_queues_link_preview(self): + with patch( + "raven.raven_messaging.doctype.raven_message.raven_message.frappe.enqueue" + ) as enqueue: + send_message_with_attachments( + channel_id=self.channel.name, + content='

page

', + ) + self.assertEqual(len(self._enqueued_link_previews(enqueue)), 1) def test_forwarded_location_reply_keeps_payload(self): - parent = send_message_with_attachments( - channel_id=self.channel.name, content="

where?

" - )[0] + parent = send_message_with_attachments(channel_id=self.channel.name, content="

where?

")[0] [loc] = send_message_with_attachments( channel_id=self.channel.name, is_reply=True, @@ -252,6 +282,56 @@ def test_forwarded_location_reply_keeps_payload(self): self.assertEqual(frappe.parse_json(payload["json"])["latitude"], 1.0) self.assertNotIn("is_reply", payload) + def _upload_image(self, name): + return frappe.get_doc( + { + "doctype": "File", + "file_name": name, + "content": "image-bytes", + "is_private": 1, + "attached_to_doctype": "Raven Message", + } + ).insert(ignore_permissions=True) + + def _upload_file(self, name): + return frappe.get_doc( + { + "doctype": "File", + "file_name": name, + "content": MINIMAL_PDF, + "is_private": 1, + "attached_to_doctype": "Raven Message", + } + ).insert(ignore_permissions=True) + + def test_location_with_snapshot(self): + snapshot = self._upload_image("location-1-2.webp") + [msg] = send_message_with_attachments( + channel_id=self.channel.name, + location={"latitude": 1, "longitude": 2}, + files=[{"file_url": snapshot.file_url, "file_size": 1234}], + ) + doc = frappe.get_doc("Raven Message", msg.name) + self.assertEqual( + (doc.message_type, doc.file, doc.file_size), ("Location", snapshot.file_url, 1234) + ) + files = get_all_files_shared_in_channel(self.channel.name) + self.assertNotIn(msg.name, [f.get("name") or f.get("message_id") for f in files]) + + def test_location_snapshot_rules(self): + img = self._upload_image("a.webp") + pdf = self._upload_file("a.pdf") + for files in ( + [{"file_url": img.file_url, "file_size": 1}] * 2, + [{"file_url": pdf.file_url, "file_size": 1}], + ): + with self.assertRaises(frappe.ValidationError): + send_message_with_attachments( + channel_id=self.channel.name, + location={"latitude": 1, "longitude": 2}, + files=files, + ) + def test_native_boot_advertises_location(self): from raven.api.native import boot diff --git a/yarn.lock b/yarn.lock index a2064fa2f..7b10363cf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -860,6 +860,11 @@ resolved "https://registry.yarnpkg.com/@capacitor/android/-/android-8.5.0.tgz#1fa7c7eb00b6600df1f346b6548869670e6cd0d3" integrity sha512-Rb3prJeQiTp0pQhSSOReJuG9VgibOGMG4ECs+UhMwr6TCCHZ3NCZO811bDA0HxD0xrT/gCrJAsY5yfEATu84JQ== +"@capacitor/app-launcher@8.0.1": + version "8.0.1" + resolved "https://registry.yarnpkg.com/@capacitor/app-launcher/-/app-launcher-8.0.1.tgz#63f5ed3c0c8745c936d5af4698f40119ae5a0618" + integrity sha512-23D8zi74sn7kxvISix8qYwgqdxGJN+4NImcNGvHen98LB1zb4eZfkJvFvp7pwntxGw4OjIE7yuf4wbzZxQHpog== + "@capacitor/app@8.1.1": version "8.1.1" resolved "https://registry.yarnpkg.com/@capacitor/app/-/app-8.1.1.tgz#af00649d0c7799ec83469b1c4ada921f9933a77e" @@ -926,6 +931,13 @@ dependencies: "@capacitor/synapse" "^1.0.4" +"@capacitor/geolocation@8.2.4": + version "8.2.4" + resolved "https://registry.yarnpkg.com/@capacitor/geolocation/-/geolocation-8.2.4.tgz#a2e8cb1175d18504d678114a4eba1cf5c0a35750" + integrity sha512-FDcUiPQc2AXSTAV61QSDPXml11guKBx5HPEkdWvUqw7kMqrc2Ucp0bsWngeLyRs/01a5aWBI0hEuvetuF3PsrQ== + dependencies: + "@capacitor/synapse" "^1.0.4" + "@capacitor/haptics@8.0.2": version "8.0.2" resolved "https://registry.yarnpkg.com/@capacitor/haptics/-/haptics-8.0.2.tgz#821d2687b81ff8855327273314d0d146476f069f" @@ -3531,6 +3543,11 @@ dependencies: "@types/node" "*" +"@types/geojson@*": + version "7946.0.16" + resolved "https://registry.yarnpkg.com/@types/geojson/-/geojson-7946.0.16.tgz#8ebe53d69efada7044454e3305c19017d97ced2a" + integrity sha512-6C8nqWur3j98U6+lXDfTUWIfgvZU+EumvpHKcYjujKH7woYyLj2sUmff0tRhrqM7BohUw7Pz3ZB1jj2gW9Fvmg== + "@types/hast@^3.0.0": version "3.0.4" resolved "https://registry.yarnpkg.com/@types/hast/-/hast-3.0.4.tgz#1d6b39993b82cea6ad783945b0508c25903e15aa" @@ -3548,6 +3565,13 @@ resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.15.tgz#596a1747233694d50f6ad8a7869fcb6f56cf5841" integrity sha512-5+fP8P8MFNC+AyZCDxrB2pkZFPGzqQWUzpSeuuVLvm8VMcorNYavBqoFcxK8bQz4Qsbn4oUEEem4wDLfcysGHA== +"@types/leaflet@^1.9.22": + version "1.9.22" + resolved "https://registry.yarnpkg.com/@types/leaflet/-/leaflet-1.9.22.tgz#d46d501e5bcdf5a99374cf113ab33562cb887783" + integrity sha512-h3lhECYEKDasG7LFHu+GiHqAvsgLuQvlJvVZzJDGONo3sEL+wUOqSFLnwkZlK0qVxnxbuGFW8iBlJNYs5wgndA== + dependencies: + "@types/geojson" "*" + "@types/mdast@^4.0.0": version "4.0.4" resolved "https://registry.yarnpkg.com/@types/mdast/-/mdast-4.0.4.tgz#7ccf72edd2f1aa7dd3437e180c64373585804dd6" @@ -5861,6 +5885,11 @@ kleur@^4.1.5: resolved "https://registry.yarnpkg.com/kleur/-/kleur-4.1.5.tgz#95106101795f7050c6c650f350c683febddb1780" integrity sha512-o+NO+8WrRiQEE4/7nwRJhN1HWpVmJm511pBHUxPLtp0BUISzlBplORYSmTclCnJvQq2tKu/sgl3xVpkc7ZWuQQ== +leaflet@^1.9.4: + version "1.9.4" + resolved "https://registry.yarnpkg.com/leaflet/-/leaflet-1.9.4.tgz#23fae724e282fa25745aff82ca4d394748db7d8d" + integrity sha512-nxS1ynzJOmOlHp+iL3FyWqK89GtNL8U8rvlMOsQdTTssxZwCXh8N2NB3GDQOL+YR3XnWyZAxwQixURb+FA74PA== + leven@^3.1.0: version "3.1.0" resolved "https://registry.yarnpkg.com/leven/-/leven-3.1.0.tgz#77891de834064cccba82ae7842bb6b14a13ed7f2" From fec3dc118cb9d36d115d1f8e6db56aca5cdc76b4 Mon Sep 17 00:00:00 2001 From: Aditya Patil Date: Mon, 26 Oct 2026 16:24:51 +0530 Subject: [PATCH 03/10] WIP: carry location and snapshot through optimistic send, outbox and retry --- apps/web/src/db/db.ts | 3 + .../src/stores/messages/messageSender.test.ts | 172 +++++++++++++++++- apps/web/src/stores/messages/messageSender.ts | 89 ++++++--- 3 files changed, 237 insertions(+), 27 deletions(-) diff --git a/apps/web/src/db/db.ts b/apps/web/src/db/db.ts index 062930ad2..51158d88f 100644 --- a/apps/web/src/db/db.ts +++ b/apps/web/src/db/db.ts @@ -2,6 +2,7 @@ import { RavenUser } from "@raven/types/Raven/RavenUser" import { siteKey } from "@lib/site" import { Dexie, type EntityTable } from "dexie" import type { VoiceNoteMeta } from "@lib/voiceNote" +import type { LocationMeta } from "@lib/location" export type UserData = Pick @@ -45,6 +46,8 @@ export interface OutboxMessage { /** A system document attached to the send (rendered as a card on the message). */ link_doctype?: string link_document?: string + /** A shared location; its snapshot, when present, is `files[0]`. */ + location?: LocationMeta } /** diff --git a/apps/web/src/stores/messages/messageSender.test.ts b/apps/web/src/stores/messages/messageSender.test.ts index f47ae587f..fadcfa24a 100644 --- a/apps/web/src/stores/messages/messageSender.test.ts +++ b/apps/web/src/stores/messages/messageSender.test.ts @@ -1,8 +1,67 @@ -import { describe, expect, it } from "vitest" -import { buildOptimisticMessages } from "./messageSender" +import { beforeEach, describe, expect, it, vi } from "vitest" +import type { OutboxMessage } from "@db" +import { buildOptimisticMessages, enqueueSend, injectOutboxRecord, retryOutboxRecord, retrySend } from "./messageSender" + +const mocks = vi.hoisted(() => ({ + getOptimisticMessages: vi.fn(), + addOptimisticMessages: vi.fn(), + retryOptimisticSend: vi.fn(), + resolveOptimisticSend: vi.fn(), + getOutbox: vi.fn(), + putOutbox: vi.fn(), + setOutboxStatus: vi.fn(), + markRead: vi.fn(), +})) + +vi.mock("@lib/location", async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, senderMapsProvider: () => "google" } +}) + +vi.mock("./store", () => ({ + channelMessagesStore: { + getOptimisticMessages: mocks.getOptimisticMessages, + addOptimisticMessages: mocks.addOptimisticMessages, + retryOptimisticSend: mocks.retryOptimisticSend, + resolveOptimisticSend: mocks.resolveOptimisticSend, + }, +})) + +vi.mock("./outbox", () => ({ + putOutbox: mocks.putOutbox, + removeOutbox: vi.fn(), + setOutboxStatus: mocks.setOutboxStatus, + isSettling: () => false, + getOutbox: mocks.getOutbox, +})) + +vi.mock("@stores/unread/store", () => ({ + channelUnreadStore: { markRead: mocks.markRead }, +})) const CREATION = "2026-10-23 10:00:00.000000" +const SNAPSHOT = { file_url: "/private/files/location-1-2.webp", file_size: 900 } + +const outboxRecord = (overrides: Partial = {}): OutboxMessage => ({ + client_id: "b", + channel_id: "ch", + owner: "me", + content: "", + files: [SNAPSHOT], + creation: CREATION, + status: "sending", + queued_at: 1, + location: { latitude: 1, longitude: 2 }, + ...overrides, +}) + +beforeEach(() => { + vi.clearAllMocks() + mocks.getOptimisticMessages.mockReturnValue([]) + mocks.getOutbox.mockResolvedValue(undefined) +}) + describe("buildOptimisticMessages", () => { it("builds a Voice placeholder for a file carrying voice metadata", () => { const voice = { duration: 7.25, waveform: [0, 50, 100] } @@ -15,4 +74,113 @@ describe("buildOptimisticMessages", () => { expect(message.message_type).toBe("File") expect("json" in message).toBe(false) }) + + it("builds one Location placeholder carrying the reply", () => { + const location = { latitude: 1, longitude: 2 } + const messages = buildOptimisticMessages("ch", "b", "me", "", [], CREATION, { linkedMessage: "m1" }, undefined, location) + expect(messages).toHaveLength(1) + expect(messages[0]).toMatchObject({ name: "b-location", message_type: "Location", json: location, is_reply: 1, linked_message: "m1", _status: "sending" }) + }) + + it("puts the snapshot on the Location placeholder", () => { + const [m] = buildOptimisticMessages("ch", "b", "me", "", [{ file_url: "/private/files/location-1-2.webp", file_size: 900 }], CREATION, undefined, undefined, { latitude: 1, longitude: 2 }) + expect(m).toMatchObject({ message_type: "Location", file: "/private/files/location-1-2.webp", file_size: 900 }) + }) + + it("sends a Location without a snapshot", () => { + const [m] = buildOptimisticMessages("ch", "b", "me", "", [], CREATION, undefined, undefined, { latitude: 1, longitude: 2 }) + expect(m.message_type).toBe("Location") + expect("file" in m).toBe(false) + }) + + it("keeps an older-site fallback as plain text", () => { + const [m] = buildOptimisticMessages("ch", "b", "me", '

x

', [], CREATION) + expect(m.message_type).toBe("Text") + }) +}) + +describe("retrySend", () => { + it("rebuilds a Location and sends its snapshot as a plain file", async () => { + mocks.getOptimisticMessages.mockReturnValue([ + { message_type: "Location", json: { latitude: 1, longitude: 2 }, file: SNAPSHOT.file_url, file_size: SNAPSHOT.file_size }, + ]) + const post = vi.fn().mockResolvedValue({ message: [] }) + + await retrySend({ post }, "ch", "b") + + expect(post).toHaveBeenCalledWith( + "raven.api.raven_message.send_message_with_attachments", + expect.objectContaining({ + location: { latitude: 1, longitude: 2, maps: "google" }, + files: [SNAPSHOT], + }), + ) + }) + + it("sends a Location without a snapshot as an empty file list", async () => { + mocks.getOptimisticMessages.mockReturnValue([{ message_type: "Location", json: { latitude: 1, longitude: 2 } }]) + const post = vi.fn().mockResolvedValue({ message: [] }) + + await retrySend({ post }, "ch", "b") + + expect(post).toHaveBeenCalledWith( + "raven.api.raven_message.send_message_with_attachments", + expect.objectContaining({ location: { latitude: 1, longitude: 2, maps: "google" }, files: [] }), + ) + }) + + it("falls back to the saved location when the placeholder json is junk", async () => { + mocks.getOptimisticMessages.mockReturnValue([{ message_type: "Location", json: "not-json" }]) + mocks.getOutbox.mockResolvedValue(outboxRecord()) + const post = vi.fn().mockResolvedValue({ message: [] }) + + await retrySend({ post }, "ch", "b") + + expect(post).toHaveBeenCalledWith( + "raven.api.raven_message.send_message_with_attachments", + expect.objectContaining({ location: { latitude: 1, longitude: 2, maps: "google" }, files: [] }), + ) + }) +}) + +describe("injectOutboxRecord", () => { + it("puts the saved location back as a Location placeholder with its snapshot", () => { + injectOutboxRecord(outboxRecord()) + + expect(mocks.addOptimisticMessages).toHaveBeenCalledWith("ch", "b", [ + expect.objectContaining({ message_type: "Location", json: { latitude: 1, longitude: 2 }, file: SNAPSHOT.file_url, file_size: SNAPSHOT.file_size }), + ]) + }) +}) + +describe("retryOutboxRecord", () => { + it("re-sends a saved location with its snapshot", async () => { + const post = vi.fn().mockResolvedValue({ message: [] }) + + await retryOutboxRecord({ post }, outboxRecord()) + + expect(mocks.addOptimisticMessages).toHaveBeenCalledWith("ch", "b", [ + expect.objectContaining({ message_type: "Location", json: { latitude: 1, longitude: 2 }, file: SNAPSHOT.file_url, file_size: SNAPSHOT.file_size }), + ]) + expect(post).toHaveBeenCalledWith( + "raven.api.raven_message.send_message_with_attachments", + expect.objectContaining({ location: { latitude: 1, longitude: 2, maps: "google" }, files: [SNAPSHOT] }), + ) + }) +}) + +describe("enqueueSend", () => { + it("saves the location on the outbox record", () => { + const post = vi.fn().mockResolvedValue({ message: [] }) + + enqueueSend( + { post }, + { channelID: "ch", batchId: "b", owner: "me", content: "", files: [SNAPSHOT], location: { latitude: 1, longitude: 2 } }, + ) + + expect(mocks.putOutbox).toHaveBeenCalledWith(expect.objectContaining({ location: { latitude: 1, longitude: 2 } })) + expect(mocks.addOptimisticMessages).toHaveBeenCalledWith("ch", "b", [ + expect.objectContaining({ message_type: "Location", json: { latitude: 1, longitude: 2 }, file: SNAPSHOT.file_url, file_size: SNAPSHOT.file_size }), + ]) + }) }) diff --git a/apps/web/src/stores/messages/messageSender.ts b/apps/web/src/stores/messages/messageSender.ts index 7450c7ca8..a869e000b 100644 --- a/apps/web/src/stores/messages/messageSender.ts +++ b/apps/web/src/stores/messages/messageSender.ts @@ -5,6 +5,7 @@ import type { Message } from "@raven/types/common/Message" import type { OutboxMessage } from "@db" import { getAttachmentKind } from "@utils/attachmentPreview" import { parseVoiceJson, type VoiceNoteMeta } from "@lib/voiceNote" +import { parseLocationJson, senderMapsProvider, type LocationMeta } from "@lib/location" import _ from "@lib/translate" import { channelMessagesStore } from "./store" import { channelUnreadStore } from "@stores/unread/store" @@ -57,6 +58,33 @@ const optimisticImageThumbnail = (width: number, height: number) => { const optimisticNow = () => dayjs().tz(SYSTEM_TIMEZONE).format("YYYY-MM-DD HH:mm:ss.SSS") + "000" +/** One placeholder per outgoing file, in send order. */ +const filePlaceholders = ( + files: OutgoingFile[], + batchId: string, + base: (extra: Partial) => OptimisticMessage, +): OptimisticMessage[] => + files.map((file, i) => { + const isImage = getAttachmentKind(file.file_url) === "image" + return base({ + name: `${batchId}-${i}`, + message_type: isImage ? "Image" : "File", + file: file.file_url, + file_size: file.file_size, + // The placeholder reserves the same box the server copy will have. + // Videos: the server stores the client dims as-is, so stamp them + // as-is. Images: the server stores a CAPPED thumbnail pair β€” stamp + // the same capped pair, or a portrait placeholder renders taller + // than the ack and shrinks when it lands. + ...(file.width && file.height + ? isImage + ? optimisticImageThumbnail(file.width, file.height) + : { thumbnail_width: file.width, thumbnail_height: file.height } + : {}), + ...(file.voice ? { message_type: "Voice" as const, json: file.voice } : {}), + }) + }) + /** * Builds the messages we show on screen the instant the user hits send β€” before the * server confirms them β€” attachments first, then the text. They all share `batchId` @@ -72,6 +100,7 @@ export const buildOptimisticMessages = ( creation: string = optimisticNow(), reply?: { linkedMessage: string; repliedMessageDetails?: string }, linkedDocument?: { doctype: string; docname: string }, + location?: LocationMeta, ): OptimisticMessage[] => { const base = (extra: Partial): OptimisticMessage => ({ @@ -83,29 +112,20 @@ export const buildOptimisticMessages = ( ...extra, }) as unknown as OptimisticMessage - const messages = files.map((file, i) => { - const isImage = getAttachmentKind(file.file_url) === "image" - return base({ - name: `${batchId}-${i}`, - message_type: isImage ? "Image" : "File", - file: file.file_url, - file_size: file.file_size, - // The placeholder reserves the same box the server copy will have. - // Videos: the server stores the client dims as-is, so stamp them - // as-is. Images: the server stores a CAPPED thumbnail pair β€” stamp - // the same capped pair, or a portrait placeholder renders taller - // than the ack and shrinks when it lands. - ...(file.width && file.height - ? isImage - ? optimisticImageThumbnail(file.width, file.height) - : { thumbnail_width: file.width, thumbnail_height: file.height } - : {}), - ...(file.voice ? { message_type: "Voice" as const, json: file.voice } : {}), - }) - }) + // A location is one message: its snapshot (when present) rides on it, and the + // composer never pairs it with text. + const locationPlaceholder = location + ? base({ + name: `${batchId}-location`, + message_type: "Location", + json: location, + ...(files[0] ? { file: files[0].file_url, file_size: files[0].file_size } : {}), + }) + : undefined + const messages: OptimisticMessage[] = locationPlaceholder ? [locationPlaceholder] : filePlaceholders(files, batchId, base) // Mirror the backend: a document-only send still needs one (empty Text) message // to carry the link β€” the card is the content. - if (content || (linkedDocument && files.length === 0)) { + if (!location && (content || (linkedDocument && files.length === 0))) { messages.push(base({ name: `${batchId}-text`, message_type: "Text", text: content })) } @@ -150,13 +170,15 @@ export const enqueueSend = ( sendSilently?: boolean /** A system document going out with the message (rendered as a card). */ linkedDocument?: { doctype: string; docname: string } + /** A shared location; its snapshot, when present, travels as `files[0]`. */ + location?: LocationMeta }, ) => { - const { channelID, batchId, owner, content, files, linkedMessage, repliedMessageDetails, sendSilently, linkedDocument } = params + const { channelID, batchId, owner, content, files, linkedMessage, repliedMessageDetails, sendSilently, linkedDocument, location } = params const creation = optimisticNow() const reply = linkedMessage ? { linkedMessage, repliedMessageDetails } : undefined - const placeholders = buildOptimisticMessages(channelID, batchId, owner, content, files, creation, reply, linkedDocument) + const placeholders = buildOptimisticMessages(channelID, batchId, owner, content, files, creation, reply, linkedDocument, location) channelMessagesStore.addOptimisticMessages(channelID, batchId, placeholders) // Sending means you're caught up β€” clear the unread badge instantly (the read tracker @@ -176,9 +198,10 @@ export const enqueueSend = ( replied_message_details: repliedMessageDetails, ...(sendSilently ? { send_silently: true } : {}), ...(linkedDocument ? { link_doctype: linkedDocument.doctype, link_document: linkedDocument.docname } : {}), + ...(location ? { location } : {}), }) - submitSend(client, channelID, batchId, content, files, linkedMessage, { sendSilently, linkedDocument }) + submitSend(client, channelID, batchId, content, files, linkedMessage, { sendSilently, linkedDocument, location }) } /** @@ -221,6 +244,8 @@ export const submitSend = ( suppressErrorToast?: boolean sendSilently?: boolean linkedDocument?: { doctype: string; docname: string } + /** The shared point; `maps` picks the server's fallback link provider. */ + location?: LocationMeta }, ) => { inFlight.add(batchId) @@ -237,6 +262,7 @@ export const submitSend = ( // Linked document rides the last message too (the caption when there's text). link_doctype: opts?.linkedDocument?.doctype ?? null, link_document: opts?.linkedDocument?.docname ?? null, + location: opts?.location ? { ...opts.location, maps: senderMapsProvider() } : null, }) .then((res) => { channelMessagesStore.resolveOptimisticSend(channelID, batchId, res.message ?? []) @@ -275,7 +301,7 @@ export const retrySend = async (client: PostClient, channelID: string, batchId: .map((m) => ({ file_url: (m as { file?: string }).file as string, file_size: (m as { file_size?: number }).file_size ?? 0, - voice: m.message_type === "Voice" ? parseVoiceJson(m.json) ?? undefined : undefined, + ...(m.message_type === "Voice" ? { voice: parseVoiceJson(m.json) ?? undefined } : {}), })) // The reply lives on whichever placeholder carries linked_message (the last one). const linkedMessage = placeholders @@ -288,6 +314,9 @@ export const retrySend = async (client: PostClient, channelID: string, batchId: // isSettling β€” if a reconnect flush grabbed it in that window, let that send win. if (inFlight.has(batchId)) return + // Fall back to the saved record when the placeholder json is unusable. + const location = locationFromPlaceholders(placeholders) ?? record?.location + channelMessagesStore.retryOptimisticSend(channelID, batchId) setOutboxStatus(batchId, "sending") // Linked document and the silent flag both come from the outbox record β€” @@ -295,9 +324,16 @@ export const retrySend = async (client: PostClient, channelID: string, batchId: submitSend(client, channelID, batchId, content, files, linkedMessage, { sendSilently: record?.send_silently, linkedDocument: record ? linkedDocumentOf(record) : undefined, + location, }) } +/** A Location placeholder's point, rebuilt for a retry (undefined when unusable). */ +const locationFromPlaceholders = (placeholders: Message[]): LocationMeta | undefined => { + const location = placeholders.find((m) => m.message_type === "Location") + return location ? parseLocationJson(location.json) ?? undefined : undefined +} + /** Discard a failed send: remove it both from the screen and from the saved outbox. */ export const discardSend = (channelID: string, batchId: string) => { channelMessagesStore.discardOptimisticSend(channelID, batchId) @@ -321,6 +357,7 @@ export const injectOutboxRecord = (record: OutboxMessage) => { record.creation, record.linked_message ? { linkedMessage: record.linked_message, repliedMessageDetails: record.replied_message_details } : undefined, linkedDocumentOf(record), + record.location, ) channelMessagesStore.addOptimisticMessages(record.channel_id, record.client_id, placeholders) // failed AND rejected both render as a failed bubble (Retry / Discard). @@ -352,6 +389,7 @@ export const retryOutboxRecord = (client: PostClient, record: OutboxMessage): Pr record.creation, record.linked_message ? { linkedMessage: record.linked_message, repliedMessageDetails: record.replied_message_details } : undefined, linkedDocumentOf(record), + record.location, ) channelMessagesStore.addOptimisticMessages(record.channel_id, record.client_id, placeholders) } else { @@ -364,5 +402,6 @@ export const retryOutboxRecord = (client: PostClient, record: OutboxMessage): Pr suppressErrorToast: true, sendSilently: record.send_silently, linkedDocument: linkedDocumentOf(record), + location: record.location, }) } From 8d24e67cdaa48a4c3e0ac9424df9395ae8fdad91 Mon Sep 17 00:00:00 2001 From: Aditya Patil Date: Mon, 26 Oct 2026 17:14:03 +0530 Subject: [PATCH 04/10] WIP: location card, open-in-maps flow and preview surfaces --- .../searchResultToMessage.test.ts | 5 + .../searchResultToMessage.ts | 3 + .../actions/dialogs/MessagePreview.tsx | 14 +- .../features/message/actions/editTarget.ts | 2 +- .../message/actions/useMessageActions.tsx | 3 +- .../message/renderers/LocationCard.tsx | 63 ++++++ .../message/renderers/MessageContent.tsx | 6 +- .../message/renderers/MessageLocation.tsx | 85 ++++++++ .../message/renderers/OpenInMapsMenu.tsx | 74 +++++++ .../renderers/RepliedMessagePreview.tsx | 9 +- .../features/reminders/RemindersList.tsx | 4 + .../saved-messages/savedRowToMessage.test.ts | 21 ++ .../saved-messages/savedRowToMessage.ts | 6 +- apps/web/src/hooks/use-mobile.ts | 8 +- apps/web/src/lib/location.test.ts | 2 + apps/web/src/lib/location.ts | 3 + apps/web/src/native/openMaps.test.ts | 191 ++++++++++++++++++ apps/web/src/native/openMaps.ts | 130 ++++++++++++ apps/web/src/utils/messageUtils.test.ts | 6 + apps/web/src/utils/messageUtils.ts | 2 + apps/web/src/utils/replySnapshot.test.ts | 5 + packages/types/common/Message.ts | 3 + 22 files changed, 638 insertions(+), 7 deletions(-) create mode 100644 apps/web/src/components/features/message/renderers/LocationCard.tsx create mode 100644 apps/web/src/components/features/message/renderers/MessageLocation.tsx create mode 100644 apps/web/src/components/features/message/renderers/OpenInMapsMenu.tsx create mode 100644 apps/web/src/native/openMaps.test.ts create mode 100644 apps/web/src/native/openMaps.ts diff --git a/apps/web/src/components/common/MessageResultBlock/searchResultToMessage.test.ts b/apps/web/src/components/common/MessageResultBlock/searchResultToMessage.test.ts index b73c08014..72a99457e 100644 --- a/apps/web/src/components/common/MessageResultBlock/searchResultToMessage.test.ts +++ b/apps/web/src/components/common/MessageResultBlock/searchResultToMessage.test.ts @@ -13,4 +13,9 @@ describe("searchResultToMessage", () => { expect(message.message_type).toBe("Text") expect(message.text).toContain("Voice note") }) + + it("shows a location as a labelled placeholder, not its fallback link", () => { + const row = { id: "m2", author: "a@x.com", channel_id: "ch", creation: "2026-10-23 10:00:00", message_type: "Location", content: "https://www.google.com/maps/search/?api=1&query=1,2" } as unknown as SearchResult + expect(searchResultToMessage({ ...row, message_type: "Location" }).text).toBe("

πŸ“ Location

") + }) }) diff --git a/apps/web/src/components/common/MessageResultBlock/searchResultToMessage.ts b/apps/web/src/components/common/MessageResultBlock/searchResultToMessage.ts index 88cc40681..31fce6fa9 100644 --- a/apps/web/src/components/common/MessageResultBlock/searchResultToMessage.ts +++ b/apps/web/src/components/common/MessageResultBlock/searchResultToMessage.ts @@ -47,6 +47,9 @@ export function searchResultToMessage(r: SearchResult): Message { if (messageType === 'Voice') { return { ...base, message_type: 'Text', text: `

🎀 ${_("Voice note")}

` } } + if (messageType === 'Location') { + return { ...base, message_type: 'Text', text: `

πŸ“ ${_("Location")}

` } + } if (messageType === 'Poll') { return { ...base, diff --git a/apps/web/src/components/features/message/actions/dialogs/MessagePreview.tsx b/apps/web/src/components/features/message/actions/dialogs/MessagePreview.tsx index de3ab7c6a..7150106db 100644 --- a/apps/web/src/components/features/message/actions/dialogs/MessagePreview.tsx +++ b/apps/web/src/components/features/message/actions/dialogs/MessagePreview.tsx @@ -1,4 +1,4 @@ -import { BarChart3Icon, File, FileText, Film, MessageSquareText, Mic, Music, type LucideIcon } from "lucide-react" +import { BarChart3Icon, File, FileText, Film, MapPin, MessageSquareText, Mic, Music, type LucideIcon } from "lucide-react" import { getAttachmentKind, type AttachmentKind } from "@utils/attachmentPreview" import { formatBytes, getFileExtension } from "@lib/file" import { getFileName } from "@raven/lib/utils/operations" @@ -68,6 +68,18 @@ export const MessagePreview = ({ message }: { message: Message }) => { ) } + // A location's snapshot is not a downloadable attachment, so it gets its own row. + if (message.message_type === "Location") { + return ( + <> +
+ +
+

{_("Location")}

+ + ) + } + if (file) { const extension = getFileExtension(file) const kind = getAttachmentKind(file) diff --git a/apps/web/src/components/features/message/actions/editTarget.ts b/apps/web/src/components/features/message/actions/editTarget.ts index 355f1db24..15f3cdff8 100644 --- a/apps/web/src/components/features/message/actions/editTarget.ts +++ b/apps/web/src/components/features/message/actions/editTarget.ts @@ -9,7 +9,7 @@ import type { Message } from "@raven/types/common/Message" * file messages aren't editable. */ export const resolveEditTarget = (message: Message): Message | null => { - if (message.message_type === "Poll") return null + if (message.message_type === "Poll" || message.message_type === "Location") return null if (message.message_batch_id) { const members = channelMessagesStore.batchMembers(message.channel_id, message.message_batch_id) return members.find((member) => member.text) ?? null diff --git a/apps/web/src/components/features/message/actions/useMessageActions.tsx b/apps/web/src/components/features/message/actions/useMessageActions.tsx index dd9938258..f6ba58b4a 100644 --- a/apps/web/src/components/features/message/actions/useMessageActions.tsx +++ b/apps/web/src/components/features/message/actions/useMessageActions.tsx @@ -247,7 +247,8 @@ export const useMessageActions = ( onSelect: () => setDialog({ type: "forward", message }), }) } - if (message.text || message.content) { + // A location's text is the fallback link β€” nothing worth copying. + if (message.message_type !== "Location" && (message.text || message.content)) { clipboard.push({ id: "copy", label: _("Copy"), diff --git a/apps/web/src/components/features/message/renderers/LocationCard.tsx b/apps/web/src/components/features/message/renderers/LocationCard.tsx new file mode 100644 index 000000000..741cc2c8b --- /dev/null +++ b/apps/web/src/components/features/message/renderers/LocationCard.tsx @@ -0,0 +1,63 @@ +import { forwardRef, useState, type ComponentPropsWithoutRef, type Ref } from "react" +import { ExternalLink, MapPin } from "lucide-react" +import type { LocationMeta } from "@lib/location" +import { cn } from "@lib/utils" +import _ from "@lib/translate" + +const cardClass = + "flex h-[76px] w-full max-w-[420px] items-center gap-3 rounded-xl border border-outline-gray-2 bg-surface-base p-1.5" + +type LocationCardProps = { + meta: LocationMeta | null + snapshotUrl?: string + onOpen?: () => void +} & Omit, "onClick" | "type" | "children"> + +/** The compact Location row: snapshot or pin, "Location", and an open-in-maps affordance. */ +export const LocationCard = forwardRef(function LocationCard( + { meta, snapshotUrl, onOpen, ...triggerProps }, + ref, +) { + const [failedSnapshot, setFailedSnapshot] = useState(null) + const showSnapshot = meta !== null && !!snapshotUrl && snapshotUrl !== failedSnapshot + const interactive = meta !== null && !!onOpen + const card = ( + <> +
+ {showSnapshot ? ( + setFailedSnapshot(snapshotUrl)} + /> + ) : ( +
+ + {_("Location")} + +