Resolve "Use primary row value in row comment mention notification email"

backend/src/baserow/contrib/database/fields/notification_types.py

@@ -32,6 +32,7 @@ class CollaboratorAddedToRowNotificationData:
     field_id: int
     field_name: str
     row_id: int
+    row_name: str
 
 
 class CollaboratorAddedToRowNotificationType(
@@ -42,11 +43,11 @@ class CollaboratorAddedToRowNotificationType(
     @classmethod
     def get_notification_title_for_email(cls, notification, context):
         return _(
-            "%(sender)s assigned you to %(field_name)s in row %(row_id)s in %(table_name)s."
+            "%(sender)s assigned you to %(field_name)s in row %(row_name)s in %(table_name)s."
         ) % {
             "sender": notification.sender.first_name,
             "field_name": notification.data["field_name"],
-            "row_id": notification.data["row_id"],
+            "row_name": notification.data.get("row_name", notification.data["row_id"]),
             "table_name": notification.data["table_name"],
         }
 
@@ -71,6 +72,7 @@ class CollaboratorAddedToRowNotificationType(
         workspace = table.database.workspace
         data = CollaboratorAddedToRowNotificationData(
             row_id=row.id,
+            row_name=row.name_or_id,
             field_id=field.id,
             field_name=field.name,
             table_id=table.id,

backend/src/baserow/contrib/database/table/models.py

@@ -595,6 +595,28 @@ class GeneratedTableModel(HierarchicalModelMixin, models.Model):
         except StopIteration:
             raise ValueError(f"Field {field_name} not found.")
 
+    @classmethod
+    def get_primary_field_object(cls):
+        field_objects = cls.get_field_objects()
+        for field_obj in field_objects:
+            if field_obj["field"].primary:
+                return field_obj
+
+    @property
+    def name(self):
+        primary_field_object = self.__class__.get_primary_field_object()
+        if primary_field_object is None:
+            return None
+        primary_field_name = primary_field_object["name"]
+        return getattr(self, primary_field_name)
+
+    @property
+    def name_or_id(self):
+        name = self.name
+        if name is None or name == "":
+            name = str(self.id)
+        return name
+
     @classmethod
     def get_field_objects(cls, include_trash: bool = False):
         field_objects = cls._field_objects.values()

backend/tests/baserow/contrib/database/field/test_field_notification_types.py

@@ -72,6 +72,7 @@ def test_notification_creation_on_adding_users_on_collaborator_fields(
                 "workspace": {"id": workspace.id},
                 "data": {
                     "row_id": row.id,
+                    "row_name": row.name_or_id,
                     "field_id": collaborator_field.id,
                     "field_name": collaborator_field.name,
                     "table_id": table.id,
@@ -101,6 +102,7 @@ def test_notification_creation_on_adding_users_on_collaborator_fields(
                     "created_on": "2023-07-06T12:00:00Z",
                     "data": {
                         "row_id": row.id,
+                        "row_name": row.name_or_id,
                         "field_id": collaborator_field.id,
                         "field_name": collaborator_field.name,
                         "table_id": table.id,
@@ -160,6 +162,7 @@ def test_notification_creation_on_adding_users_on_collaborator_fields(
                     "created_on": "2023-07-06T12:00:00Z",
                     "data": {
                         "row_id": row.id,
+                        "row_name": row.name_or_id,
                         "field_id": collaborator_field.id,
                         "field_name": collaborator_field.name,
                         "table_id": table.id,
@@ -199,6 +202,7 @@ def test_notification_creation_on_adding_users_on_collaborator_fields(
                 "workspace": {"id": workspace.id},
                 "data": {
                     "row_id": row.id,
+                    "row_name": row.name_or_id,
                     "field_id": collaborator_field.id,
                     "field_name": collaborator_field.name,
                     "table_id": table.id,
@@ -270,6 +274,7 @@ def test_notifications_are_grouped_when_user_is_added_to_multiple_rows(
                 "workspace": {"id": workspace.id},
                 "data": {
                     "row_id": row_1_id,
+                    "row_name": str(row_1_id),
                     "field_id": collaborator_field.id,
                     "field_name": collaborator_field.name,
                     "table_id": table.id,
@@ -291,6 +296,7 @@ def test_notifications_are_grouped_when_user_is_added_to_multiple_rows(
                 "workspace": {"id": workspace.id},
                 "data": {
                     "row_id": row_2_id,
+                    "row_name": str(row_2_id),
                     "field_id": collaborator_field.id,
                     "field_name": collaborator_field.name,
                     "table_id": table.id,
@@ -320,6 +326,7 @@ def test_notifications_are_grouped_when_user_is_added_to_multiple_rows(
                     "created_on": "2023-07-06T12:00:00Z",
                     "data": {
                         "row_id": row_1_id,
+                        "row_name": str(row_1_id),
                         "field_id": collaborator_field.id,
                         "field_name": collaborator_field.name,
                         "table_id": table.id,
@@ -341,6 +348,7 @@ def test_notifications_are_grouped_when_user_is_added_to_multiple_rows(
                     "created_on": "2023-07-06T12:00:00Z",
                     "data": {
                         "row_id": row_2_id,
+                        "row_name": str(row_2_id),
                         "field_id": collaborator_field.id,
                         "field_name": collaborator_field.name,
                         "table_id": table.id,

backend/tests/baserow/contrib/database/table/test_table_models.py

@@ -216,6 +216,69 @@ def test_get_table_model_to_str(data_fixture):
     assert str(instance) == f"unnamed row {instance.id}"
 
 
+@pytest.mark.django_db
+def test_get_primary_field_object(data_fixture):
+    table = data_fixture.create_database_table()
+    table_without_primary = data_fixture.create_database_table()
+    text_field = data_fixture.create_text_field(table=table, primary=True)
+    number_field = data_fixture.create_number_field(table=table)
+
+    model = table.get_model()
+    assert model.get_primary_field_object()["field"] == text_field
+    assert model.get_primary_field_object()["name"] == f"field_{text_field.id}"
+
+    model_without_primary = table_without_primary.get_model()
+    assert model_without_primary.get_primary_field_object() is None
+
+
+@pytest.mark.django_db
+def test_name_property(data_fixture):
+    table = data_fixture.create_database_table()
+    text_field = data_fixture.create_text_field(table=table, primary=True)
+    number_field = data_fixture.create_number_field(table=table)
+    model = table.get_model()
+    table_without_primary = data_fixture.create_database_table()
+    text_field_no_primary = data_fixture.create_text_field(table=table_without_primary)
+    model_without_primary = table_without_primary.get_model()
+
+    row_no_name = model.objects.create(**{f"field_{text_field.id}": None})
+    row_empty_name = model.objects.create(**{f"field_{text_field.id}": ""})
+    row_john = model.objects.create(**{f"field_{text_field.id}": "John"})
+
+    assert row_no_name.name is None
+    assert row_empty_name.name == ""
+    assert row_john.name == "John"
+
+    row_no_name = model_without_primary.objects.create(
+        **{f"field_{text_field_no_primary.id}": None}
+    )
+    row_empty_name = model_without_primary.objects.create(
+        **{f"field_{text_field_no_primary.id}": ""}
+    )
+    row_john = model_without_primary.objects.create(
+        **{f"field_{text_field_no_primary.id}": "John"}
+    )
+
+    assert row_no_name.name is None
+    assert row_empty_name.name is None
+    assert row_john.name is None
+
+
+@pytest.mark.django_db
+def test_name_or_id_property(data_fixture):
+    table = data_fixture.create_database_table()
+    text_field = data_fixture.create_text_field(table=table, primary=True)
+    model = table.get_model()
+
+    row_no_name = model.objects.create(**{f"field_{text_field.id}": None})
+    row_empty_name = model.objects.create(**{f"field_{text_field.id}": ""})
+    row_john = model.objects.create(**{f"field_{text_field.id}": "John"})
+
+    assert row_no_name.name_or_id == str(row_no_name.id)
+    assert row_empty_name.name_or_id == str(row_empty_name.id)
+    assert row_john.name_or_id == "John"
+
+
 @pytest.mark.django_db
 def test_enhance_by_fields_queryset(data_fixture):
     table = data_fixture.create_database_table(name="Cars")

changelog/entries/unreleased/feature/2293_use_primary_row_value_in_email_notifications_instead_of_just.json

@@ -0,0 +1,7 @@
+{
+    "type": "feature",
+    "message": "Use primary row value in email notifications instead of just row ids",
+    "issue_number": 2293,
+    "bullet_points": [],
+    "created_at": "2024-01-25"
+}

premium/backend/src/baserow_premium/row_comments/handler.py

@@ -176,7 +176,7 @@ class RowCommentHandler:
             context=table,
         )
 
-        RowHandler().has_row(requesting_user, table, row_id, raise_error=True)
+        row = RowHandler().get_row(requesting_user, table, row_id)
 
         if not is_valid_prosemirror_document(message):
             raise InvalidRowCommentException()
@@ -198,7 +198,11 @@ class RowCommentHandler:
             row_comment.mentions.set(mentions)
 
         row_comment_created.send(
-            cls, row_comment=row_comment, user=requesting_user, mentions=list(mentions)
+            cls,
+            row_comment=row_comment,
+            row=row,
+            user=requesting_user,
+            mentions=list(mentions),
         )
         return row_comment
 
@@ -251,9 +255,12 @@ class RowCommentHandler:
         if new_mentions:
             row_comment.mentions.set(new_mentions)
 
+        row = RowHandler().get_row(requesting_user, table, row_comment.row_id)
+
         row_comment_updated.send(
             cls,
             row_comment=row_comment,
+            row=row,
             user=requesting_user,
             mentions=list(set(new_mentions) - set(old_mentions)),
         )

premium/backend/src/baserow_premium/row_comments/notification_types.py

@@ -28,17 +28,19 @@ class RowCommentNotificationData:
     table_id: int
     table_name: str
     row_id: int
+    row_name: str
     comment_id: int
     message: str
 
     @classmethod
-    def from_row_comment(cls, row_comment):
+    def from_row_comment(cls, row_comment, row):
         return cls(
             database_id=row_comment.table.database.id,
             database_name=row_comment.table.database.name,
             table_id=row_comment.table_id,
             table_name=row_comment.table.name,
             row_id=int(row_comment.row_id),
+            row_name=row.name_or_id,
             comment_id=row_comment.id,
             message=row_comment.message,
         )
@@ -48,7 +50,7 @@ class RowCommentMentionNotificationType(EmailNotificationTypeMixin, Notification
     type = "row_comment_mention"
 
     @classmethod
-    def notify_mentioned_users(cls, row_comment, mentions):
+    def notify_mentioned_users(cls, row_comment, row, mentions):
         """
         Creates a notification for each user that is mentioned in the comment.
 
@@ -57,7 +59,9 @@ class RowCommentMentionNotificationType(EmailNotificationTypeMixin, Notification
         :return: The list of created notifications.
         """
 
-        notification_data = RowCommentNotificationData.from_row_comment(row_comment)
+        notification_data = RowCommentNotificationData.from_row_comment(
+            row_comment, row
+        )
         NotificationHandler.create_direct_notification_for_users(
             notification_type=cls.type,
             recipients=mentions,
@@ -68,9 +72,9 @@ class RowCommentMentionNotificationType(EmailNotificationTypeMixin, Notification
 
     @classmethod
     def get_notification_title_for_email(cls, notification, context):
-        return _("%(user)s mentioned you in row %(row_id)s in %(table_name)s.") % {
+        return _("%(user)s mentioned you in row %(row_name)s in %(table_name)s.") % {
             "user": notification.sender.first_name,
-            "row_id": notification.data["row_id"],
+            "row_name": notification.data.get("row_name", notification.data["row_id"]),
             "table_name": notification.data["table_name"],
         }
 
@@ -92,7 +96,7 @@ class RowCommentNotificationType(EmailNotificationTypeMixin, NotificationType):
 
     @classmethod
     def notify_subscribed_users(
-        cls, row_comment, users_to_notify
+        cls, row_comment, row, users_to_notify
     ) -> Optional[List[NotificationRecipient]]:
         """
         Creates a notification for each user that is subscribed to receive comments on
@@ -107,7 +111,9 @@ class RowCommentNotificationType(EmailNotificationTypeMixin, NotificationType):
         if not users_to_notify:
             return
 
-        notification_data = RowCommentNotificationData.from_row_comment(row_comment)
+        notification_data = RowCommentNotificationData.from_row_comment(
+            row_comment, row
+        )
         return NotificationHandler.create_direct_notification_for_users(
             notification_type=cls.type,
             recipients=users_to_notify,
@@ -118,9 +124,9 @@ class RowCommentNotificationType(EmailNotificationTypeMixin, NotificationType):
 
     @classmethod
     def get_notification_title_for_email(cls, notification, context):
-        return _("%(user)s posted a comment in row %(row_id)s in %(table_name)s.") % {
+        return _("%(user)s posted a comment in row %(row_name)s in %(table_name)s.") % {
             "user": notification.sender.first_name,
-            "row_id": notification.data["row_id"],
+            "row_name": notification.data.get("row_name", notification.data["row_id"]),
             "table_name": notification.data["table_name"],
         }
 
@@ -130,16 +136,20 @@ class RowCommentNotificationType(EmailNotificationTypeMixin, NotificationType):
 
 
 @receiver(row_comment_created)
-def on_row_comment_created(sender, row_comment, user, mentions, **kwargs):
+def on_row_comment_created(sender, row_comment, row, user, mentions, **kwargs):
     if mentions:
-        RowCommentMentionNotificationType.notify_mentioned_users(row_comment, mentions)
+        RowCommentMentionNotificationType.notify_mentioned_users(
+            row_comment, row, mentions
+        )
 
     user_ids_to_exclude = [mention.id for mention in mentions]
     users_to_notify = RowCommentHandler.get_users_to_notify_for_comment(
         row_comment, user_ids_to_exclude
     )
     if users_to_notify:
-        RowCommentNotificationType.notify_subscribed_users(row_comment, users_to_notify)
+        RowCommentNotificationType.notify_subscribed_users(
+            row_comment, row, users_to_notify
+        )
 
     # The sender will probably wants to receive the notification about all
     # future comments posted on this row if this is the first comment, so change
@@ -160,6 +170,8 @@ def on_row_comment_created(sender, row_comment, user, mentions, **kwargs):
 
 
 @receiver(row_comment_updated)
-def on_row_comment_updated(sender, row_comment, user, mentions, **kwargs):
+def on_row_comment_updated(sender, row_comment, row, user, mentions, **kwargs):
     if mentions:
-        RowCommentMentionNotificationType.notify_mentioned_users(row_comment, mentions)
+        RowCommentMentionNotificationType.notify_mentioned_users(
+            row_comment, row, mentions
+        )

premium/backend/src/baserow_premium/ws/row_comments/signals.py

@@ -9,7 +9,7 @@ from baserow.ws.tasks import broadcast_to_users
 
 
 @receiver(row_comment_signals.row_comment_created)
-def row_comment_created(sender, row_comment, user, **kwargs):
+def row_comment_created(sender, row_comment, row, user, **kwargs):
     table_page_type = page_registry.get("table")
     transaction.on_commit(
         lambda: table_page_type.broadcast(
@@ -24,7 +24,7 @@ def row_comment_created(sender, row_comment, user, **kwargs):
 
 
 @receiver(row_comment_signals.row_comment_updated)
-def row_comment_updated(sender, row_comment, user, **kwargs):
+def row_comment_updated(sender, row_comment, row, user, **kwargs):
     table_page_type = page_registry.get("table")
     transaction.on_commit(
         lambda: table_page_type.broadcast(

premium/backend/tests/baserow_premium_tests/row_comments/test_row_comments_handler.py

@@ -103,8 +103,10 @@ def test_row_comment_created_signal_called(
 
     mock_row_comment_created.assert_called_once()
     args = mock_row_comment_created.call_args
-
-    assert args == call(RowCommentHandler, row_comment=c, user=user, mentions=[])
+    assert args.kwargs["mentions"] == []
+    assert args.kwargs["row_comment"] == c
+    assert args.kwargs["user"] == user
+    assert args.kwargs["row"].id == rows[0].id
 
 
 @pytest.mark.django_db
@@ -168,7 +170,10 @@ def test_row_comment_updated_signal_called(
     mock_row_comment_updated.assert_called_once()
     args = mock_row_comment_updated.call_args
 
-    assert args == call(RowCommentHandler, row_comment=c, user=user, mentions=[])
+    assert args.kwargs["mentions"] == []
+    assert args.kwargs["row_comment"] == c
+    assert args.kwargs["user"] == user
+    assert args.kwargs["row"].id == rows[0].id
 
 
 @pytest.mark.django_db

premium/backend/tests/baserow_premium_tests/row_comments/test_row_comments_notification_types.py

@@ -97,6 +97,7 @@ def test_notification_creation_on_creating_row_comment_mention(
                     "table_id": table.id,
                     "table_name": table.name,
                     "row_id": rows[0].id,
+                    "row_name": rows[0].name_or_id,
                     "comment_id": comment_id,
                     "message": message,
                 },
@@ -186,6 +187,7 @@ def test_notify_only_new_mentions_when_updating_a_comment(
                     "table_id": table.id,
                     "table_name": table.name,
                     "row_id": rows[0].id,
+                    "row_name": rows[0].name_or_id,
                     "comment_id": comment_id,
                     "message": new_message,
                 },
@@ -248,7 +250,7 @@ def test_email_notifications_are_created_correctly(
     expected_context = {
         "notifications": [
             {
-                "title": f"User 1 mentioned you in row {row.id} in {table.name}.",
+                "title": f"User 1 mentioned you in row {row.name_or_id} in {table.name}.",
                 "description": "@User 2",
             }
         ],
@@ -358,6 +360,7 @@ def test_user_receive_notification_if_subscribed_for_comments_on_a_row(
         "read": False,
         "data": {
             "row_id": rows[0].id,
+            "row_name": rows[0].name_or_id,
             "table_id": table.id,
             "table_name": table.name,
             "database_id": database.id,
@@ -620,7 +623,7 @@ def test_row_comment_notification_type_can_be_rendered_as_email(
     )
 
     notification_recipients = RowCommentNotificationType.notify_subscribed_users(
-        row_comment, [user]
+        row_comment, row_comment.get_parent(), [user]
     )
 
     assert len(notification_recipients) == 1
@@ -628,7 +631,7 @@ def test_row_comment_notification_type_can_be_rendered_as_email(
 
     assert (
         RowCommentNotificationType.get_notification_title_for_email(notification, {})
-        == f"{commenter.first_name} posted a comment in row {rows[0].id} in {table.name}."
+        == f"{commenter.first_name} posted a comment in row {rows[0].name_or_id} in {table.name}."
     )
     assert (
         RowCommentNotificationType.get_notification_description_for_email(

premium/web-frontend/modules/baserow_premium/components/row_comments/RowCommentMentionNotification.vue

@@ -15,7 +15,9 @@
           >
         </template>
         <template #row>
-          <strong>{{ notification.data.row_id }}</strong>
+          <strong>{{
+            notification.data.row_name ?? notification.data.row_id
+          }}</strong>
         </template>
         <template #table>
           <strong>{{ notification.data.table_name }}</strong>

premium/web-frontend/modules/baserow_premium/components/row_comments/RowCommentNotification.vue

@@ -13,7 +13,9 @@
           >
         </template>
         <template #row>
-          <strong>{{ notification.data.row_id }}</strong>
+          <strong>{{
+            notification.data.row_name ?? notification.data.row_id
+          }}</strong>
         </template>
         <template #table>
           <strong>{{ notification.data.table_name }}</strong>

web-frontend/modules/database/components/notifications/CollaboratorAddedToRowNotification.vue

@@ -18,7 +18,9 @@
           <strong>{{ notification.data.field_name }}</strong>
         </template>
         <template #rowId>
-          <strong>{{ notification.data.row_id }}</strong>
+          <strong>{{
+            notification.data.row_name ?? notification.data.row_id
+          }}</strong>
         </template>
         <template #tableName>
           <strong>{{ notification.data.table_name }}</strong>