export: Fix broken URLs in UI with S3 backend.

Apparently, the Zulip notifications (and resulting emails) were
correct, but the download links inside the Zulip UI were incorrectly
not including S3 prefix on the URL, making them not work.

While we're at this, we rewrite the somewhat convoluted previous
system for formatting the data export output.

static/js/settings_exports.js

@@ -33,7 +33,7 @@ exports.populate_exports_table = function (exports) {
     var exports_list = list_render.create(exports_table, Object.values(exports), {
         name: "admin_exports_list",
         modifier: function (data) {
-            if (data.export_data.deleted_timestamp === undefined) {
+            if (data.deleted_timestamp === null) {
                 return render_admin_export_list({
                     realm_export: {
                         id: data.id,
@@ -42,7 +42,7 @@ exports.populate_exports_table = function (exports) {
                         event_time: timerender.last_seen_status_from_date(
                             new XDate(data.export_time * 1000)
                         ),
-                        path: data.export_data.export_path,
+                        url: data.export_url,
                     },
                 });
             }

static/templates/admin_export_list.hbs

@@ -7,8 +7,8 @@
         <span class="export_time">{{event_time}}</span>
     </td>
     <td>
-        {{#if path}}
-        <span class="export_url"><a href="{{path}}" download>{{t 'Download' }}</a></span>
+        {{#if url}}
+        <span class="export_url"><a href="{{url}}" download>{{t 'Download' }}</a></span>
         {{else}}
         <span class="export_url">{{t 'The export URL is not yet available... Check back soon.' }}</span>
         {{/if}}

zerver/lib/export.py

@@ -1707,10 +1707,14 @@ def get_realm_exports_serialized(user: UserProfile) -> List[Dict[str, Any]]:
                                                event_type=RealmAuditLog.REALM_EXPORTED)
     exports_dict = {}
     for export in all_exports:
+        export_data = ujson.loads(export.extra_data)
+        export_url = zerver.lib.upload.upload_backend.get_export_tarball_url(
+            user.realm, export_data['export_path'])
         exports_dict[export.id] = dict(
             id=export.id,
             export_time=export.event_time.timestamp(),
             acting_user_id=export.acting_user.id,
-            export_data=ujson.loads(export.extra_data)
+            export_url=export_url,
+            deleted_timestamp=export_data.get('deleted_timestamp'),
         )
     return sorted(exports_dict.values(), key=lambda export_dict: export_dict['id'])

zerver/lib/upload.py

@@ -259,6 +259,10 @@ class ZulipUploadBackend:
     def delete_export_tarball(self, path_id: str) -> Optional[str]:
         raise NotImplementedError()
 
+    def get_export_tarball_url(self, realm: Realm, export_path: str) -> str:
+        raise NotImplementedError()
+
+
 ### S3
 
 def get_bucket(conn: S3Connection, bucket_name: str) -> Bucket:
@@ -453,6 +457,11 @@ class S3UploadBackend(ZulipUploadBackend):
         # ?x=x allows templates to append additional parameters with &s
         return "https://%s.s3.amazonaws.com/%s%s?x=x" % (bucket, hash_key, medium_suffix)
 
+    def get_export_tarball_url(self, realm: Realm, export_path: str) -> str:
+        bucket = settings.S3_AVATAR_BUCKET
+        # export_path has a leading /
+        return "https://%s.s3.amazonaws.com%s" % (bucket, export_path)
+
     def upload_realm_icon_image(self, icon_file: File, user_profile: UserProfile) -> None:
         content_type = guess_type(icon_file.name)[0]
         bucket_name = settings.S3_AVATAR_BUCKET
@@ -815,6 +824,10 @@ class LocalUploadBackend(ZulipUploadBackend):
             return path_id
         return None
 
+    def get_export_tarball_url(self, realm: Realm, export_path: str) -> str:
+        # export_path has a leading `/`
+        return realm.uri + export_path
+
 # Common and wrappers
 if settings.LOCAL_UPLOADS_DIR is not None:
     upload_backend = LocalUploadBackend()  # type: ZulipUploadBackend

zerver/tests/test_events.py

@@ -2773,9 +2773,8 @@ class EventsRegisterTest(ZulipTestCase):
                 ('id', check_int),
                 ('export_time', check_float),
                 ('acting_user_id', check_int),
-                ('export_data', check_dict_only([
-                    ('export_path', check_string),
-                ])),
+                ('export_url', check_string),
+                ('deleted_timestamp', equals(None)),
             ]))),
         ])
 
@@ -2800,10 +2799,8 @@ class EventsRegisterTest(ZulipTestCase):
                 ('id', check_int),
                 ('export_time', check_float),
                 ('acting_user_id', check_int),
-                ('export_data', check_dict_only([
-                    ('export_path', check_string),
-                    ('deleted_timestamp', check_float),
-                ])),
+                ('export_url', check_string),
+                ('deleted_timestamp', check_float),
             ]))),
         ])
 

zerver/tests/test_realm_export.py

@@ -68,7 +68,8 @@ class RealmExportTest(ZulipTestCase):
         # Test that the export we have is the export we created.
         export_dict = result.json()['exports']
         self.assertEqual(export_dict[0]['id'], audit_log_entry.id)
-        self.assertEqual(export_dict[0]['export_data'].get('export_path'), path_id)
+        self.assertEqual(export_dict[0]['export_url'],
+                         'https://test-avatar-bucket.s3.amazonaws.com' + path_id)
         self.assertEqual(export_dict[0]['acting_user_id'], admin.id)
         self.assert_length(export_dict,
                            RealmAuditLog.objects.filter(
@@ -126,7 +127,7 @@ class RealmExportTest(ZulipTestCase):
         # Test that the export we have is the export we created.
         export_dict = result.json()['exports']
         self.assertEqual(export_dict[0]['id'], audit_log_entry.id)
-        self.assertEqual(export_dict[0]['export_data'].get('export_path'), path_id)
+        self.assertEqual(export_dict[0]['export_url'], admin.realm.uri + path_id)
         self.assertEqual(export_dict[0]['acting_user_id'], admin.id)
         self.assert_length(export_dict,
                            RealmAuditLog.objects.filter(