[ENG-8400] Feature/notification metrics

.github/workflows/test-build.yml

@@ -11,6 +11,7 @@ env:
   ELASTICSEARCH6_ARCHIVE: elasticsearch-6.3.1.tar.gz
   OSF_DB_PORT: 5432
   OSF_DB_PASSWORD: postgres
+  GITHUB_ACTIONS: true
 
 jobs:
   build-cache:
@@ -167,3 +168,36 @@ jobs:
     - name: Upload report
       if: (success() || failure())    # run this step even if previous step failed
       uses: ./.github/actions/gen-report
+
+  mailhog:
+    runs-on: ubuntu-22.04
+    permissions:
+      checks: write
+    needs: build-cache
+    services:
+      postgres:
+        image: postgres
+
+        env:
+          POSTGRES_PASSWORD: ${{ env.OSF_DB_PASSWORD }}
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+        ports:
+          # Maps tcp port 5432 on service container to the host
+          - 5432:5432
+      mailhog:
+        image: mailhog/mailhog
+        ports:
+          - 1025:1025
+          - 8025:8025
+    steps:
+    - uses: actions/checkout@v2
+    - uses: ./.github/actions/start-build
+    - name: Run tests
+      run: poetry run python3 -m invoke test-ci-mailhog -n 1 --junit
+    - name: Upload report
+      if: (github.event_name != 'pull_request') && (success() || failure())    # run this step even if previous step failed
+      uses: ./.github/actions/gen-report

README-docker-compose.md

@@ -271,10 +271,8 @@
       docker compose run --rm web python3 -m scripts.parse_citation_styles
       ```
 - Start ember_osf_web
-  - Needed for quickfiles feature:
-      ```bash
-      docker compose up -d ember_osf_web
-      ```
+  - Needed for ember app:
+    - `docker-compose up -d ember_osf_web`
 - OPTIONAL: Register OAuth Scopes
   - Needed for things such as the ember-osf dummy app
       ```bash

addons/base/views.py

@@ -21,7 +21,6 @@
 
 from addons.base import exceptions as addon_errors
 from addons.base.models import BaseStorageAddon
-from addons.osfstorage.models import OsfStorageFile
 from addons.osfstorage.models import OsfStorageFileNode
 from addons.osfstorage.utils import enqueue_update_analytics
 
@@ -34,9 +33,7 @@
 from framework.exceptions import HTTPError
 from framework.flask import redirect
 from framework.sentry import log_exception
-from framework.routing import proxy_url
 from framework.transactions.handlers import no_auto_transaction
-from website import mails
 from website import settings
 from addons.base import signals as file_signals
 from addons.base.utils import format_last_known_metadata, get_mfr_url
@@ -54,11 +51,12 @@
     DraftRegistration,
     Guid,
     FileVersionUserMetadata,
-    FileVersion
+    FileVersion, NotificationType
 )
 from osf.metrics import PreprintView, PreprintDownload
 from osf.utils import permissions
 from osf.external.gravy_valet import request_helpers
+from website.notifications.emails import localize_timestamp
 from website.profile.utils import get_profile_image_url
 from website.project import decorators
 from website.project.decorators import must_be_contributor_or_public, must_be_valid_project, check_contributor_auth
@@ -483,7 +481,7 @@ def _construct_payload(auth, resource, credentials, waterbutler_settings):
 
 @must_be_signed
 @no_auto_transaction
-@must_be_valid_project(quickfiles_valid=True, preprints_valid=True)
+@must_be_valid_project(preprints_valid=True)
 def create_waterbutler_log(payload, **kwargs):
     with transaction.atomic():
         try:
@@ -578,20 +576,24 @@ def create_waterbutler_log(payload, **kwargs):
                     params=payload
                 )
 
-            if payload.get('email') is True or payload.get('errors'):
-                mails.send_mail(
-                    user.username,
-                    mails.FILE_OPERATION_FAILED if payload.get('errors')
-                    else mails.FILE_OPERATION_SUCCESS,
-                    action=payload['action'],
-                    source_node=source_node,
-                    destination_node=destination_node,
-                    source_path=payload['source']['materialized'],
-                    source_addon=payload['source']['addon'],
-                    destination_addon=payload['destination']['addon'],
-                    osf_support_email=settings.OSF_SUPPORT_EMAIL
+            if payload.get('email') or payload.get('errors'):
+                if payload.get('email'):
+                    notification_type = NotificationType.Type.FILE_OPERATION_SUCCESS
+                if payload.get('errors'):
+                    notification_type = NotificationType.Type.FILE_OPERATION_FAILED
+
+                NotificationType.objects.get(name=notification_type.value).emit(
+                    user=user,
+                    event_context={
+                        'action': payload['action'],
+                        'source_node': source_node,
+                        'destination_node': destination_node,
+                        'source_path': payload['source']['materialized'],
+                        'source_addon': payload['source']['addon'],
+                        'destination_addon': payload['destination']['addon'],
+                        'osf_support_email': settings.OSF_SUPPORT_EMAIL
+                    }
                 )
-
             if payload.get('errors'):
                 # Action failed but our function succeeded
                 # Bail out to avoid file_signals
@@ -603,11 +605,38 @@ def create_waterbutler_log(payload, **kwargs):
     metadata = payload.get('metadata') or payload.get('destination')
 
     target_node = AbstractNode.load(metadata.get('nid'))
-    if target_node and not target_node.is_quickfiles and payload['action'] != 'download_file':
+    if target_node and payload['action'] != 'download_file':
         update_storage_usage_with_size(payload)
 
-    with transaction.atomic():
-        file_signals.file_updated.send(target=node, user=user, event_type=action, payload=payload)
+    file_signals.file_updated.send(target=node, user=user, event_type=action, payload=payload)
+
+    match f'node_{action}':
+        case NotificationType.Type.NODE_FILE_ADDED:
+            notification = NotificationType.objects.get(name=NotificationType.Type.NODE_FILE_ADDED)
+        case NotificationType.Type.NODE_FILE_REMOVED:
+            notification = NotificationType.objects.get(name=NotificationType.Type.NODE_FILE_REMOVED)
+        case NotificationType.Type.NODE_FILE_UPDATED:
+            notification = NotificationType.objects.get(name=NotificationType.Type.NODE_FILE_UPDATED)
+        case NotificationType.Type.NODE_ADDON_FILE_RENAMED:
+            notification = NotificationType.objects.get(name=NotificationType.Type.NODE_ADDON_FILE_RENAMED)
+        case NotificationType.Type.NODE_ADDON_FILE_COPIED:
+            notification = NotificationType.objects.get(name=NotificationType.Type.NODE_ADDON_FILE_COPIED)
+        case NotificationType.Type.NODE_ADDON_FILE_REMOVED:
+            notification = NotificationType.objects.get(name=NotificationType.Type.NODE_ADDON_FILE_REMOVED)
+        case NotificationType.Type.NODE_ADDON_FILE_MOVED:
+            notification = NotificationType.objects.get(name=NotificationType.Type.NODE_ADDON_FILE_MOVED)
+        case _:
+            raise NotImplementedError(f'action {action} not implemented')
+
+    notification.emit(
+        user=user,
+        event_context={
+            'profile_image_url': user.profile_image_url(),
+            'localized_timestamp': localize_timestamp(timezone.now(), user),
+            'user_fullname': user.fullname,
+            'url': node.absolute_url,
+        }
+    )
 
     return {'status': 'success'}
 
@@ -1032,16 +1061,6 @@ def persistent_file_download(auth, **kwargs):
     )
 
 
-def addon_view_or_download_quickfile(**kwargs):
-    fid = kwargs.get('fid', 'NOT_AN_FID')
-    file_ = OsfStorageFile.load(fid)
-    if not file_:
-        raise HTTPError(http_status.HTTP_404_NOT_FOUND, data={
-            'message_short': 'File Not Found',
-            'message_long': 'The requested file could not be found.'
-        })
-    return proxy_url(f'/project/{file_.target._id}/files/osfstorage/{fid}/')
-
 def addon_view_file(auth, node, file_node, version):
     # TODO: resolve circular import issue
     from addons.wiki import settings as wiki_settings

addons/boa/tests/test_tasks.py

@@ -38,6 +38,9 @@ def setUp(self):
         self.output_file_name = 'fake_boa_script_results.txt'
         self.job_id = '1a2b3c4d5e6f7g8'
 
+        from conftest import start_mock_send_grid
+        self.mock_send_grid = start_mock_send_grid(self)
+
     def tearDown(self):
         super().tearDown()
 
@@ -52,9 +55,10 @@ def test_boa_error_code(self):
         assert BoaErrorCode.FILE_TOO_LARGE_ERROR == 6
         assert BoaErrorCode.JOB_TIME_OUT_ERROR == 7
 
+    @mock.patch('website.mails.settings.USE_EMAIL', True)
+    @mock.patch('website.mails.settings.USE_CELERY', False)
     def test_handle_boa_error(self):
-        with mock.patch('addons.boa.tasks.send_mail', return_value=None) as mock_send_mail, \
-                mock.patch('addons.boa.tasks.sentry.log_message', return_value=None) as mock_sentry_log_message, \
+        with mock.patch('addons.boa.tasks.sentry.log_message', return_value=None) as mock_sentry_log_message, \
                 mock.patch('addons.boa.tasks.logger.error', return_value=None) as mock_logger_error:
             return_value = handle_boa_error(
                 self.error_message,
@@ -68,24 +72,7 @@ def test_handle_boa_error(self):
                 output_file_name=self.output_file_name,
                 job_id=self.job_id
             )
-            mock_send_mail.assert_called_with(
-                to_addr=self.user_username,
-                mail=ADDONS_BOA_JOB_FAILURE,
-                fullname=self.user_fullname,
-                code=BoaErrorCode.UNKNOWN,
-                message=self.error_message,
-                query_file_name=self.query_file_name,
-                file_size=self.file_size,
-                max_file_size=boa_settings.MAX_SUBMISSION_SIZE,
-                query_file_full_path=self.file_full_path,
-                output_file_name=self.output_file_name,
-                job_id=self.job_id,
-                max_job_wait_hours=self.max_job_wait_hours,
-                project_url=self.project_url,
-                boa_job_list_url=boa_settings.BOA_JOB_LIST_URL,
-                boa_support_email=boa_settings.BOA_SUPPORT_EMAIL,
-                osf_support_email=osf_settings.OSF_SUPPORT_EMAIL,
-            )
+            self.mock_send_grid.assert_called()
             mock_sentry_log_message.assert_called_with(self.error_message, skip_session=True)
             mock_logger_error.assert_called_with(self.error_message)
             assert return_value == BoaErrorCode.UNKNOWN
@@ -167,9 +154,14 @@ def setUp(self):
         boa_settings.REFRESH_JOB_INTERVAL = DEFAULT_REFRESH_JOB_INTERVAL
         boa_settings.MAX_JOB_WAITING_TIME = DEFAULT_MAX_JOB_WAITING_TIME
 
+        from conftest import start_mock_send_grid
+        self.mock_send_grid = start_mock_send_grid(self)
+
     def tearDown(self):
         super().tearDown()
 
+    @mock.patch('website.mails.settings.USE_EMAIL', True)
+    @mock.patch('website.mails.settings.USE_CELERY', False)
     async def test_submit_success(self):
         with mock.patch('osf.models.user.OSFUser.objects.get', return_value=self.user), \
                 mock.patch('osf.models.user.OSFUser.get_or_create_cookie', return_value=self.user_cookie), \
@@ -179,7 +171,6 @@ async def test_submit_success(self):
                 mock.patch('boaapi.boa_client.BoaClient.query', return_value=self.mock_job), \
                 mock.patch('boaapi.boa_client.BoaClient.close', return_value=None) as mock_close, \
                 mock.patch('asyncio.sleep', new_callable=AsyncMock, return_value=None) as mock_async_sleep, \
-                mock.patch('addons.boa.tasks.send_mail', return_value=None) as mock_send_mail, \
                 mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
             return_value = await submit_to_boa_async(
                 self.host,
@@ -199,19 +190,7 @@ async def test_submit_success(self):
             assert self.mock_job.refresh.call_count == 4
             assert mock_async_sleep.call_count == 4
             mock_close.assert_called()
-            mock_send_mail.assert_called_with(
-                to_addr=self.user.username,
-                mail=ADDONS_BOA_JOB_COMPLETE,
-                fullname=self.user.fullname,
-                query_file_name=self.query_file_name,
-                query_file_full_path=self.file_full_path,
-                output_file_name=self.output_file_name,
-                job_id=self.mock_job.id,
-                project_url=self.project_url,
-                boa_job_list_url=boa_settings.BOA_JOB_LIST_URL,
-                boa_support_email=boa_settings.BOA_SUPPORT_EMAIL,
-                osf_support_email=osf_settings.OSF_SUPPORT_EMAIL,
-            )
+            self.mock_send_grid.assert_called()
             mock_handle_boa_error.assert_not_called()
 
     async def test_download_error(self):

addons/osfstorage/tests/test_views.py

@@ -24,7 +24,7 @@
 from framework.auth import cas
 
 from osf import features
-from osf.models import Tag, QuickFilesNode
+from osf.models import Tag
 from osf.models import files as models
 from addons.osfstorage.apps import osf_storage_root
 from addons.osfstorage import utils

addons/osfstorage/views.py

@@ -314,9 +314,6 @@ def osfstorage_create_child(file_node, payload, **kwargs):
     if not (name or user) or '/' in name:
         raise HTTPError(http_status.HTTP_400_BAD_REQUEST)
 
-    if getattr(file_node.target, 'is_quickfiles', False) and is_folder:
-        raise HTTPError(http_status.HTTP_400_BAD_REQUEST, data={'message_long': 'You may not create a folder for QuickFiles'})
-
     try:
         # Create a save point so that we can rollback and unlock
         # the parent record

admin/base/urls.py

@@ -30,7 +30,6 @@
             re_path(r'^maintenance/', include('admin.maintenance.urls', namespace='maintenance')),
             re_path(r'^meetings/', include('admin.meetings.urls', namespace='meetings')),
             re_path(r'^metrics/', include('admin.metrics.urls', namespace='metrics')),
-            re_path(r'^osf_groups/', include('admin.osf_groups.urls', namespace='osf_groups')),
             re_path(r'^management/', include('admin.management.urls', namespace='management')),
             re_path(r'^internet_archive/', include('admin.internet_archive.urls', namespace='internet_archive')),
             re_path(r'^schema_responses/', include('admin.schema_responses.urls', namespace='schema_responses')),

admin/common_auth/forms.py

@@ -22,7 +22,7 @@ class UserRegistrationForm(forms.Form):
 
     # TODO: Moving to guardian, find a better way to distinguish "admin-like" groups from object permission groups
     group_perms = forms.ModelMultipleChoiceField(
-        queryset=Group.objects.exclude(Q(name__startswith='collections_') | Q(name__startswith='reviews_') | Q(name__startswith='preprint_') | Q(name__startswith='node_') | Q(name__startswith='osfgroup_') | Q(name__startswith='draft_registration_')),
+        queryset=Group.objects.exclude(Q(name__startswith='collections_') | Q(name__startswith='reviews_') | Q(name__startswith='preprint_') | Q(name__startswith='node_') | Q(name__startswith='draft_registration_')),
         required=False,
         widget=forms.CheckboxSelectMultiple
     )

admin/nodes/templatetags/node_extras.py

@@ -39,11 +39,6 @@ def reverse_user(user):
     return reverse('users:user', kwargs={'guid': user._id})
 
 
-@register.filter
-def reverse_osf_group(value):
-    return reverse('osf_groups:osf_group', kwargs={'id': value._id})
-
-
 @register.filter
 def reverse_registration_provider(value):
     return reverse('registration_providers:detail', kwargs={'registration_provider_id': value.provider.id})

admin/notifications/views.py

@@ -1,17 +1,17 @@
-from osf.models.notifications import NotificationSubscription
+from osf.models.notifications import NotificationSubscriptionLegacy
 from django.db.models import Count
 
 def delete_selected_notifications(selected_ids):
-    NotificationSubscription.objects.filter(id__in=selected_ids).delete()
+    NotificationSubscriptionLegacy.objects.filter(id__in=selected_ids).delete()
 
 def detect_duplicate_notifications(node_id=None):
-    query = NotificationSubscription.objects.values('_id').annotate(count=Count('_id')).filter(count__gt=1)
+    query = NotificationSubscriptionLegacy.objects.values('_id').annotate(count=Count('_id')).filter(count__gt=1)
     if node_id:
         query = query.filter(node_id=node_id)
 
     detailed_duplicates = []
     for dup in query:
-        notifications = NotificationSubscription.objects.filter(
+        notifications = NotificationSubscriptionLegacy.objects.filter(
             _id=dup['_id']
         ).order_by('created')