common/tests/factories/notifications.py

@@ -1,11 +1,19 @@
 from django.contrib.contenttypes.models import ContentType
 from factory.django import DjangoModelFactory
+from faker import Faker
 import actstream.models
 import factory
 
+from common.tests.factories.extensions import ExtensionFactory, RatingFactory
+from common.tests.factories.reviewers import ApprovalActivityFactory
+from common.tests.factories.users import UserFactory
+from constants.activity import Verb
 import notifications.models
+import reviewers.models
+
 
 RELATION_ALLOWED_MODELS = []
+fake = Faker()
 
 
 def generic_foreign_key_id_for_type_factory(generic_relation_type_field):
@@ -32,3 +40,46 @@ class NotificationFactory(DjangoModelFactory):
         model = notifications.models.Notification
 
     action = factory.SubFactory(ActionFactory)
+
+
+def construct_fake_notifications() -> list['NotificationFactory']:
+    """Construct notifications of known types without persisting them in the DB."""
+    fake_extension = ExtensionFactory.build(slug='test')
+    verb_to_action_object = {
+        Verb.APPROVED: ApprovalActivityFactory.build(
+            extension=fake_extension,
+            type=reviewers.models.ApprovalActivity.ActivityType.APPROVED,
+            message=fake.paragraph(nb_sentences=1),
+        ),
+        Verb.COMMENTED: ApprovalActivityFactory.build(
+            extension=fake_extension,
+            type=reviewers.models.ApprovalActivity.ActivityType.COMMENT,
+            message=fake.paragraph(nb_sentences=1),
+        ),
+        Verb.RATED_EXTENSION: RatingFactory.build(
+            text=fake.paragraph(nb_sentences=2),
+        ),
+        Verb.REPORTED_EXTENSION: None,  # TODO: fake action_object
+        Verb.REPORTED_RATING: None,  # TODO: fake action_object
+        Verb.REQUESTED_CHANGES: ApprovalActivityFactory.build(
+            extension=fake_extension,
+            type=reviewers.models.ApprovalActivity.ActivityType.AWAITING_CHANGES,
+            message=fake.paragraph(nb_sentences=1),
+        ),
+        Verb.REQUESTED_REVIEW: ApprovalActivityFactory.build(
+            extension=fake_extension,
+            type=reviewers.models.ApprovalActivity.ActivityType.AWAITING_REVIEW,
+            message=fake.paragraph(nb_sentences=1),
+        ),
+    }
+    fake_notifications = [
+        NotificationFactory.build(
+            recipient=UserFactory.build(),
+            action__actor=UserFactory.build(),
+            action__target=fake_extension,
+            action__verb=verb,
+            action__action_object=action_object,
+        )
+        for verb, action_object in verb_to_action_object.items()
+    ]
+    return fake_notifications

emails/admin.py

@@ -8,14 +8,9 @@ from django.utils.html import format_html
 from django.utils.safestring import mark_safe
 import django.core.mail
 
-from common.tests.factories.extensions import ExtensionFactory, RatingFactory
-from common.tests.factories.notifications import NotificationFactory
-from common.tests.factories.reviewers import ApprovalActivityFactory
-from common.tests.factories.users import UserFactory
-from constants.activity import Verb
 from emails.models import Email
 from emails.util import construct_email
-import reviewers.models
+from common.tests.factories.notifications import construct_fake_notifications
 
 logger = logging.getLogger(__name__)
 User = get_user_model()
@@ -117,44 +112,8 @@ class EmailPreviewAdmin(NoAddDeleteMixin, EmailAdmin):
     def _get_fake_context(self, request):
         fake_context = {'feedback': {}}
 
-        # Make previews for all known notification types
+        fake_notifications = construct_fake_notifications()
-        fake_extension = ExtensionFactory.build(slug='test')
+        for fake_notification in fake_notifications:
-        verb_to_action_object = {
-            Verb.APPROVED: ApprovalActivityFactory.build(
-                extension=fake_extension,
-                type=reviewers.models.ApprovalActivity.ActivityType.APPROVED,
-                message='Amazing add-on, approved!',
-            ),
-            Verb.COMMENTED: ApprovalActivityFactory.build(
-                extension=fake_extension,
-                type=reviewers.models.ApprovalActivity.ActivityType.COMMENT,
-                message='This is an important albeit somewhat inconclusive note',
-            ),
-            Verb.RATED_EXTENSION: RatingFactory.build(text='Amazing, 7/10!'),
-            Verb.REPORTED_EXTENSION: None,  # TODO: fake action_object
-            Verb.REPORTED_RATING: None,  # TODO: fake action_object
-            Verb.REQUESTED_CHANGES: ApprovalActivityFactory.build(
-                extension=fake_extension,
-                type=reviewers.models.ApprovalActivity.ActivityType.AWAITING_CHANGES,
-                message=(
-                    'Latest uploaded version does not appear to work, '
-                    'a change is necessary before this can be approved and publicly listed'
-                ),
-            ),
-            Verb.REQUESTED_REVIEW: ApprovalActivityFactory.build(
-                extension=fake_extension,
-                type=reviewers.models.ApprovalActivity.ActivityType.AWAITING_REVIEW,
-                message='This add-on is ready to be reviewed',
-            ),
-        }
-        for verb, action_object in verb_to_action_object.items():
-            fake_notification = NotificationFactory.build(
-                recipient=UserFactory.build(),
-                action__actor=UserFactory.build(),
-                action__target=fake_extension,
-                action__verb=verb,
-                action__action_object=action_object,
-            )
             mail_name = fake_notification.full_template_name
             fake_context[mail_name] = {
                 'template': fake_notification.template_name,

emails/templates/emails/base.html

@@ -1,16 +1,10 @@
 {% extends "emails/email_base.html" %}
 
-{% block header_logo %}
+{% block header_logo %}{% spaceless %}
   {# have a title instead of the logo with the remote image #}
   <div style="text-align: center; font-weight: bold;">{{ subject }}</div>
-{% endblock header_logo %}
+{% endspaceless %}{% endblock header_logo %}
 
-{% block body %}
+{% block body %}{% spaceless %}
-  <p>Dear {% firstof user.full_name user.email %},</p>
   {% block content %}{% endblock content %}
-  <p>
+{% endspaceless %}{% endblock body %}
-    --<br />
-    Kind regards,<br />
-    Blender Extensions Team
-  </p>
-{% endblock body %}

emails/templates/emails/base.txt

@@ -1,9 +1,3 @@
-Dear {% firstof user.full_name user.email %},
 {% block content %}{% endblock content %}
 
 Manage your profile: {{ profile_url }}
-
---
-Kind regards,
-
-Blender Extensions Team

emails/templates/emails/email_base.html

@@ -1,3 +1,4 @@
+{% spaceless %}
 <html style="
   height: 100%;
   width: 100%;
@@ -93,3 +94,4 @@
     </div>
   </body>
 </html>
+{% endspaceless %}

emails/templates/emails/new_activity.html

@@ -1,5 +1,5 @@
 {% extends "emails/base.html" %}
-{% block content %}
+{% block content %}{% spaceless %}
   <div>
     {% include "emails/components/new_activity_action" %}{% if quoted_message %}:{% endif %}
     {% if quoted_message %}
@@ -17,8 +17,8 @@
     You are receiving this email because you are a moderator subscribed to notification emails.
     You are receiving this email because you are subscribed to notifications on this extension.
   {% endcomment %}
-{% endblock content %}
+{% endspaceless %}{% endblock content %}
 
-{% block footer_links %}
+{% block footer_links %}{% spaceless %}
   Unsubscribe by adjusting your preferences at  {{ profile_url }}
-{% endblock footer_links %}
+{% endspaceless %}{% endblock footer_links %}

emails/util.py

@@ -34,7 +34,7 @@ def construct_email(email_name: str, context: Dict[str, Any]) -> Tuple[str, str,
 
     :return: tuple (html, text, subject)
     """
-    context = {**get_template_context(), **context}
+    context.update(**get_template_context())
     base_path = 'emails'
     subj_tmpl, html_tmpl, txt_tmpl = (
         f'{base_path}/{email_name}_subject.txt',

notifications/models.py

@@ -78,7 +78,6 @@ class Notification(models.Model):
             Verb.APPROVED,
             Verb.COMMENTED,
             Verb.RATED_EXTENSION,
-            Verb.REPORTED_EXTENSION,
             Verb.REPORTED_RATING,
             Verb.REQUESTED_CHANGES,
             Verb.REQUESTED_REVIEW,

notifications/tests/test_follow_logic.py

@@ -1,7 +1,10 @@
 from pathlib import Path
 
+from django.core import mail
+from django.core.management import call_command
 from django.test import TestCase
 from django.urls import reverse
+from django.utils import timezone
 
 from common.tests.factories.extensions import create_approved_version, create_version
 from common.tests.factories.files import FileFactory
@@ -10,6 +13,7 @@ from files.models import File
 from notifications.models import Notification
 from reviewers.models import ApprovalActivity
 
+
 TEST_FILES_DIR = Path(__file__).resolve().parent / '../../extensions/tests/files'
 
 
@@ -17,7 +21,9 @@ class TestTasks(TestCase):
     fixtures = ['dev', 'licenses']
 
     def test_ratings(self):
-        extension = create_approved_version(ratings=[]).extension
+        extension = create_approved_version(
+            ratings=[], file__user__confirmed_email_at=timezone.now()
+        ).extension
         author = extension.authors.first()
         notification_nr = Notification.objects.filter(recipient=author).count()
         some_user = UserFactory()
@@ -29,14 +35,35 @@ class TestTasks(TestCase):
         new_notification_nr = Notification.objects.filter(recipient=author).count()
         self.assertEqual(new_notification_nr, notification_nr + 1)
 
+        # Call the command that sends notification emails
+        call_command('send_notification_emails')
+
+        # Test that one message has been sent.
+        self.assertEqual(len(mail.outbox), 1)
+
+        email = mail.outbox[0]
+        self.assertEqual(email.subject, f'Add-on rated: "{extension.name}"')
+        self.assertEqual(email.to, [author.email])
+        email_text = email.body
+        self.maxDiff = None
+        expected_text = expected_rated_text.format(**locals())
+        self.assertEqual(email_text, expected_text)
+
+        # Check that most important action and relevant URL are in the HTML version
+        email_html = email.alternatives[0][0]
+        self.assertIn(' rated ', email_html)
+        self.assertIn(
+            f'https://extensions.local:8111/add-ons/{extension.slug}/reviews/', email_html
+        )
+
     def test_abuse(self):
         extension = create_approved_version(ratings=[]).extension
-        moderator = create_moderator()
+        moderator = create_moderator(confirmed_email_at=timezone.now())
         notification_nr = Notification.objects.filter(recipient=moderator).count()
         some_user = UserFactory()
         self.client.force_login(some_user)
         url = extension.get_report_url()
-        self.client.post(
+        response = self.client.post(
             url,
             {
                 'message': 'test message',
@@ -44,11 +71,31 @@ class TestTasks(TestCase):
                 'version': '',
             },
         )
+        self.assertEqual(response.status_code, 302)
+        report_url = response['Location']
         new_notification_nr = Notification.objects.filter(recipient=moderator).count()
         self.assertEqual(new_notification_nr, notification_nr + 1)
 
+        call_command('send_notification_emails')
+
+        # Test that one message has been sent.
+        self.assertEqual(len(mail.outbox), 1)
+
+        email = mail.outbox[0]
+        self.assertEqual(email.subject, f'Add-on reported: "{extension.name}"')
+        self.assertEqual(email.to, [moderator.email])
+        email_text = email.body
+        self.maxDiff = None
+        expected_text = expected_abuse_report_text.format(**locals())
+        self.assertEqual(email_text, expected_text)
+
+        # Check that most important action and relevant URL are in the HTML version
+        email_html = email.alternatives[0][0]
+        self.assertIn(report_url, email_html)
+        self.assertIn(' reported ', email_html)
+
     def test_new_extension_submitted(self):
-        moderator = create_moderator()
+        moderator = create_moderator(confirmed_email_at=timezone.now())
         notification_nr = Notification.objects.filter(recipient=moderator).count()
         some_user = UserFactory()
         file_data = {
@@ -117,10 +164,29 @@ class TestTasks(TestCase):
         new_notification_nr = Notification.objects.filter(recipient=moderator).count()
         self.assertEqual(new_notification_nr, notification_nr + 1)
 
+        call_command('send_notification_emails')
+
+        # Test that one message has been sent.
+        self.assertEqual(len(mail.outbox), 1)
+
+        email = mail.outbox[0]
+        extension = file.version.extension
+        self.assertEqual(email.subject, 'Add-on review requested: "Edit Breakdown"')
+        self.assertEqual(email.to, [moderator.email])
+        email_text = email.body
+        self.maxDiff = None
+        expected_text = expected_review_requested_text.format(**locals())
+        self.assertEqual(email_text, expected_text)
+
+        # Check that most important action and relevant URL are in the HTML version
+        email_html = email.alternatives[0][0]
+        self.assertIn(' requested review of ', email_html)
+        self.assertIn(f'https://extensions.local:8111/approval-queue/{extension.slug}/', email_html)
+
     def test_approval_queue_activity(self):
         extension = create_approved_version(ratings=[]).extension
         author = extension.authors.first()
-        moderator = create_moderator()
+        moderator = create_moderator(confirmed_email_at=timezone.now())
         some_user = UserFactory()
         notification_nrs = {}
         for user in [author, moderator, some_user]:
@@ -136,7 +202,88 @@ class TestTasks(TestCase):
         self.assertEqual(new_notification_nrs[moderator.pk], notification_nrs[moderator.pk] + 1)
         self.assertEqual(new_notification_nrs[some_user.pk], notification_nrs[some_user.pk] + 1)
 
+        call_command('send_notification_emails')
+
+        # Test that one message has been sent (only moderator has email confirmed here).
+        self.assertEqual(len(mail.outbox), 1)
+
+        email = mail.outbox[0]
+        self.assertEqual(email.subject, f'New comment on Add-on "{extension.name}"')
+        email_text = email.body
+        expected_text = expected_new_comment_text.format(**locals())
+        self.maxDiff = None
+        self.assertEqual(email_text, expected_text)
+
+        # Check that most important action and relevant URL are in the HTML version
+        email_html = email.alternatives[0][0]
+        self.assertIn(' commented on ', email_html)
+        self.assertIn(f'https://extensions.local:8111/approval-queue/{extension.slug}/', email_html)
+
     def _leave_a_comment(self, user, extension, text):
         self.client.force_login(user)
         url = reverse('reviewers:approval-comment', args=[extension.slug])
         self.client.post(url, {'type': ApprovalActivity.ActivityType.COMMENT, 'message': text})
+
+    # TODO: test for notifications about a reported rating
+    # TODO: test for notifications about extension approved by moderators
+
+
+expected_abuse_report_text = """Add-on reported: "{extension.name}"
+{some_user.full_name} reported Add-on "{extension.name}"
+:
+
+“test message”
+
+https://extensions.local:8111{report_url}
+Read all notifications at https://extensions.local:8111/notifications/
+
+
+Unsubscribe by adjusting your preferences at  https://extensions.local:8111/settings/profile/
+
+https://extensions.local:8111/
+"""
+
+expected_new_comment_text = """New comment on Add-on "{extension.name}"
+{some_user.full_name} commented on Add-on "{extension.name}"
+:
+
+“this is bad”
+
+https://extensions.local:8111/approval-queue/{extension.slug}/
+Read all notifications at https://extensions.local:8111/notifications/
+
+
+Unsubscribe by adjusting your preferences at  https://extensions.local:8111/settings/profile/
+
+https://extensions.local:8111/
+"""
+
+expected_rated_text = """Add-on rated: "{extension.name}"
+{some_user.full_name} rated extension Add-on "{extension.name}"
+:
+
+“rating text”
+
+https://extensions.local:8111/add-ons/{extension.slug}/reviews/
+Read all notifications at https://extensions.local:8111/notifications/
+
+
+Unsubscribe by adjusting your preferences at  https://extensions.local:8111/settings/profile/
+
+https://extensions.local:8111/
+"""
+
+expected_review_requested_text = """Add-on review requested: "Edit Breakdown"
+{some_user.full_name} requested review of Add-on "Edit Breakdown"
+:
+
+“Extension is ready for initial review”
+
+https://extensions.local:8111/approval-queue/edit-breakdown/
+Read all notifications at https://extensions.local:8111/notifications/
+
+
+Unsubscribe by adjusting your preferences at  https://extensions.local:8111/settings/profile/
+
+https://extensions.local:8111/
+"""

notifications/tests/test_send_emails.py

@@ -1,161 +0,0 @@
-from io import StringIO
-from pathlib import Path
-
-from django.core.management import call_command
-from django.test import TestCase
-from django.urls import reverse
-from django.utils import timezone
-
-from common.tests.factories.extensions import create_approved_version, create_version
-from common.tests.factories.files import FileFactory
-from common.tests.factories.users import UserFactory, create_moderator
-from files.models import File
-from notifications.models import Notification
-from reviewers.models import ApprovalActivity
-
-TEST_FILES_DIR = Path(__file__).resolve().parent / '../../extensions/tests/files'
-
-
-class TestSendNotificationEmails(TestCase):
-    fixtures = ['dev', 'licenses']
-
-    def test_ratings(self):
-        extension = create_approved_version(
-            ratings=[], file__user__confirmed_email_at=timezone.now()
-        ).extension
-        author = extension.authors.first()
-        notification_nr = Notification.objects.filter(recipient=author).count()
-        some_user = UserFactory()
-        self.client.force_login(some_user)
-        url = extension.get_rate_url()
-        response = self.client.post(url, {'score': 3, 'text': 'rating text'})
-        self.assertEqual(response.status_code, 302)
-        self.assertEqual(extension.ratings.count(), 1)
-        new_notification_nr = Notification.objects.filter(recipient=author).count()
-        self.assertEqual(new_notification_nr, notification_nr + 1)
-
-        out = StringIO()
-        call_command('send_notification_emails', stdout=out, stderr=out)
-
-    def test_abuse(self):
-        extension = create_approved_version(
-            ratings=[], file__user__confirmed_email_at=timezone.now()
-        ).extension
-        moderator = create_moderator(confirmed_email_at=timezone.now())
-        notification_nr = Notification.objects.filter(recipient=moderator).count()
-        some_user = UserFactory()
-        self.client.force_login(some_user)
-        url = extension.get_report_url()
-        self.client.post(
-            url,
-            {
-                'message': 'test message',
-                'reason': '127',
-                'version': '',
-            },
-        )
-        new_notification_nr = Notification.objects.filter(recipient=moderator).count()
-        self.assertEqual(new_notification_nr, notification_nr + 1)
-
-        out = StringIO()
-        call_command('send_notification_emails', stdout=out, stderr=out)
-
-    def test_new_extension_submitted(self):
-        moderator = create_moderator(confirmed_email_at=timezone.now())
-        notification_nr = Notification.objects.filter(recipient=moderator).count()
-        some_user = UserFactory()
-        file_data = {
-            'metadata': {
-                'tagline': 'Get insight on the complexity of an edit',
-                'id': 'edit_breakdown',
-                'name': 'Edit Breakdown',
-                'version': '0.1.0',
-                'blender_version_min': '4.2.0',
-                'type': 'add-on',
-                'schema_version': "1.0.0",
-            },
-            'file_hash': 'sha256:4f3664940fc41641c7136a909270a024bbcfb2f8523a06a0d22f85c459b0b1ae',
-            'size_bytes': 53959,
-            'tags': ['Sequencer'],
-            'version_str': '0.1.0',
-            'slug': 'edit-breakdown',
-        }
-        file = FileFactory(
-            type=File.TYPES.BPY,
-            user=some_user,
-            original_hash=file_data['file_hash'],
-            hash=file_data['file_hash'],
-            metadata=file_data['metadata'],
-        )
-        create_version(
-            file=file,
-            extension__name=file_data['metadata']['name'],
-            extension__slug=file_data['metadata']['id'].replace("_", "-"),
-            extension__website=None,
-            tagline=file_data['metadata']['tagline'],
-            version=file_data['metadata']['version'],
-            blender_version_min=file_data['metadata']['blender_version_min'],
-            schema_version=file_data['metadata']['schema_version'],
-        )
-        self.client.force_login(some_user)
-        data = {
-            # Most of these values should come from the form's initial values, set in the template
-            # Version fields
-            'release_notes': 'initial release',
-            # Extension fields
-            'description': 'Rather long and verbose description',
-            'support': 'https://example.com/issues',
-            # Previews
-            'form-TOTAL_FORMS': ['2'],
-            'form-INITIAL_FORMS': ['0'],
-            'form-MIN_NUM_FORMS': ['0'],
-            'form-MAX_NUM_FORMS': ['1000'],
-            'form-0-id': '',
-            'form-0-caption': ['First Preview Caption Text'],
-            'form-1-id': '',
-            'form-1-caption': ['Second Preview Caption Text'],
-            # Submit for Approval.
-            'submit_draft': '',
-        }
-        file_name1 = 'test_preview_image_0001.png'
-        file_name2 = 'test_preview_image_0002.png'
-        with open(TEST_FILES_DIR / file_name1, 'rb') as fp1, open(
-            TEST_FILES_DIR / file_name2, 'rb'
-        ) as fp2:
-            files = {
-                'form-0-source': fp1,
-                'form-1-source': fp2,
-            }
-            self.client.post(file.get_submit_url(), {**data, **files})
-        new_notification_nr = Notification.objects.filter(recipient=moderator).count()
-        self.assertEqual(new_notification_nr, notification_nr + 1)
-
-        out = StringIO()
-        call_command('send_notification_emails', stdout=out, stderr=out)
-
-    def test_approval_queue_activity(self):
-        extension = create_approved_version(ratings=[]).extension
-        author = extension.authors.first()
-        moderator = create_moderator()
-        some_user = UserFactory()
-        notification_nrs = {}
-        for user in [author, moderator, some_user]:
-            notification_nrs[user.pk] = Notification.objects.filter(recipient=user).count()
-        # both moderator and some_user start following only after their first comment
-        self._leave_a_comment(moderator, extension, 'need to check this')
-        self._leave_a_comment(some_user, extension, 'this is bad')
-        self._leave_a_comment(moderator, extension, 'thanks for the heads up')
-        new_notification_nrs = {}
-        for user in [author, moderator, some_user]:
-            new_notification_nrs[user.pk] = Notification.objects.filter(recipient=user).count()
-        self.assertEqual(new_notification_nrs[author.pk], notification_nrs[author.pk] + 3)
-        self.assertEqual(new_notification_nrs[moderator.pk], notification_nrs[moderator.pk] + 1)
-        self.assertEqual(new_notification_nrs[some_user.pk], notification_nrs[some_user.pk] + 1)
-
-        out = StringIO()
-        call_command('send_notification_emails', stdout=out, stderr=out)
-
-    def _leave_a_comment(self, user, extension, text):
-        self.client.force_login(user)
-        url = reverse('reviewers:approval-comment', args=[extension.slug])
-        self.client.post(url, {'type': ApprovalActivity.ActivityType.COMMENT, 'message': text})

utils.py

@@ -196,41 +196,45 @@ def absolutify(url: str, request=None) -> str:
 def absolute_url(
     view_name: str, args: Optional[tuple] = None, kwargs: Optional[dict] = None
 ) -> str:
-    """Same as django.urls.reverse() but then as absolute URL."""
+    """Same as django.urls.reverse() but returned as an absolute URL."""
     relative_url = reverse(view_name, args=args, kwargs=kwargs)
     return absolutify(relative_url)
 
 
 class HTMLFilter(HTMLParser):
+    skip_text_of = ('a', 'style')
     text = ''
-    skip = False
+    skip_tag_text = False
-    skip_one = False
 
     def handle_starttag(self, tag, attrs):
-        if tag == 'style':
+        if tag in self.skip_text_of:
-            self.skip = True
+            self.skip_tag_text = True
         for name, value in attrs:
             if name == 'href':
-                self.skip_one = True
+                self.skip_tag_text = True
                 self.text += value
+        if tag in ('quote', 'q'):
+            self.text += '“'
 
     def handle_endtag(self, tag):
-        if tag == 'style':
+        if tag in self.skip_text_of:
-            self.skip = False
+            self.skip_tag_text = False
+        if tag in ('quote', 'q'):
+            self.text += '”\n\n'
 
     def handle_data(self, data):
-        if self.skip:
+        if self.skip_tag_text:
             return
-        if self.skip_one:
-            self.skip_one = False
-            return
-        data = data.strip()
         self.text += data
-        if not data.endswith('\n') and len(data) > 1:
-            self.text += '\n'
 
 
 def html_to_text(data: str) -> str:
     f = HTMLFilter()
     f.feed(data)
-    return f.text
+    lines = [_.lstrip(' \t') for _ in f.text.split('\n')]
+    skip_empty = 0
+    for line in lines:
+        if not re.match(r'^\s*$', line):
+            break
+        skip_empty += 1
+    return '\n'.join(lines[skip_empty:])