diff --git a/backend/src/baserow/contrib/database/fields/notification_types.py b/backend/src/baserow/contrib/database/fields/notification_types.py index 32e5e304e..9cd941918 100644 --- a/backend/src/baserow/contrib/database/fields/notification_types.py +++ b/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, diff --git a/backend/src/baserow/contrib/database/table/models.py b/backend/src/baserow/contrib/database/table/models.py index 5be0f9da2..2e8c5c6fa 100644 --- a/backend/src/baserow/contrib/database/table/models.py +++ b/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() diff --git a/backend/tests/baserow/contrib/database/field/test_field_notification_types.py b/backend/tests/baserow/contrib/database/field/test_field_notification_types.py index 2ad0b5907..b19ebe4d9 100644 --- a/backend/tests/baserow/contrib/database/field/test_field_notification_types.py +++ b/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, diff --git a/backend/tests/baserow/contrib/database/table/test_table_models.py b/backend/tests/baserow/contrib/database/table/test_table_models.py index 49e191b93..f6066f7f9 100644 --- a/backend/tests/baserow/contrib/database/table/test_table_models.py +++ b/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") diff --git a/changelog/entries/unreleased/feature/2293_use_primary_row_value_in_email_notifications_instead_of_just.json b/changelog/entries/unreleased/feature/2293_use_primary_row_value_in_email_notifications_instead_of_just.json new file mode 100644 index 000000000..056da0774 --- /dev/null +++ b/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" +} \ No newline at end of file diff --git a/premium/backend/src/baserow_premium/row_comments/handler.py b/premium/backend/src/baserow_premium/row_comments/handler.py index 094f64fbd..0d6dd2c3a 100644 --- a/premium/backend/src/baserow_premium/row_comments/handler.py +++ b/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)), ) diff --git a/premium/backend/src/baserow_premium/row_comments/notification_types.py b/premium/backend/src/baserow_premium/row_comments/notification_types.py index 68601b888..341348dd0 100644 --- a/premium/backend/src/baserow_premium/row_comments/notification_types.py +++ b/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 + ) diff --git a/premium/backend/src/baserow_premium/ws/row_comments/signals.py b/premium/backend/src/baserow_premium/ws/row_comments/signals.py index 9af65ad08..7d4c71ea0 100644 --- a/premium/backend/src/baserow_premium/ws/row_comments/signals.py +++ b/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( diff --git a/premium/backend/tests/baserow_premium_tests/row_comments/test_row_comments_handler.py b/premium/backend/tests/baserow_premium_tests/row_comments/test_row_comments_handler.py index 67549781c..035a04adc 100644 --- a/premium/backend/tests/baserow_premium_tests/row_comments/test_row_comments_handler.py +++ b/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 diff --git a/premium/backend/tests/baserow_premium_tests/row_comments/test_row_comments_notification_types.py b/premium/backend/tests/baserow_premium_tests/row_comments/test_row_comments_notification_types.py index 15f5445e4..9602a1b7c 100644 --- a/premium/backend/tests/baserow_premium_tests/row_comments/test_row_comments_notification_types.py +++ b/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( diff --git a/premium/web-frontend/modules/baserow_premium/components/row_comments/RowCommentMentionNotification.vue b/premium/web-frontend/modules/baserow_premium/components/row_comments/RowCommentMentionNotification.vue index 17dbcc1e1..d691f4989 100644 --- a/premium/web-frontend/modules/baserow_premium/components/row_comments/RowCommentMentionNotification.vue +++ b/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> diff --git a/premium/web-frontend/modules/baserow_premium/components/row_comments/RowCommentNotification.vue b/premium/web-frontend/modules/baserow_premium/components/row_comments/RowCommentNotification.vue index 81fa6ea1e..4cc88006f 100644 --- a/premium/web-frontend/modules/baserow_premium/components/row_comments/RowCommentNotification.vue +++ b/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> diff --git a/web-frontend/modules/database/components/notifications/CollaboratorAddedToRowNotification.vue b/web-frontend/modules/database/components/notifications/CollaboratorAddedToRowNotification.vue index b612684b2..45a4f6b0f 100644 --- a/web-frontend/modules/database/components/notifications/CollaboratorAddedToRowNotification.vue +++ b/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>