feat: add title field and metadata handling for ZIM file creation

Author: nicolapace

wp1-frontend/cypress/e2e/zimFile.cy.js

@@ -31,6 +31,21 @@ describe('the zim file creation page', () => {
           cy.get('#longdesc').should('be.visible');
         });
 
+        it('validates the title input on losing focus', () => {
+          cy.get('#zimtitle').click();
+          cy.get('#zimtitle').clear();
+          cy.get('#longdesc').click();
+          cy.get('#zimtitle-group > .invalid-feedback').should('be.visible');
+        });
+
+        it('validates the title input to have max 30 characters', () => {
+            const longTitle = 'A'.repeat(31);
+          cy.get('#zimtitle').click();
+          cy.get('#zimtitle').clear();
+          cy.get('#zimtitle').type(longTitle);
+          cy.get('#zimtitle').should('have.value', longTitle.substring(0, 30));
+        });
+
         it('validates the description input on losing focus', () => {
           cy.get('#desc').click();
           cy.get('#longdesc').click();

wp1-frontend/src/components/ZimFile.vue

@@ -20,7 +20,7 @@
           <h2>Create ZIM file</h2>
           <p v-if="status === 'NOT_REQUESTED'">
             Use this form to create a ZIM file from your selection, so that you
-            can browse the articles it contains offline. The
+            can browse the articles it contains offline. The &quot;Title&quot;,
             &quot;Description&quot; and &quot;Long Description&quot; fields are
             required, but generic defaults will be used if they're not provided.
           </p>
@@ -66,6 +66,21 @@
             class="needs-validation"
             novalidate
           >
+            <div id="zimtitle-group" ref="zimtitle_form_group" class="form-group">
+              <label for="zimtitle">Title</label>
+              <input
+                id="zimtitle"
+                ref="zimtitle"
+                v-on:blur="validationOnBlur"
+                v-model="zimTitle"
+                class="form-control"
+                maxlength="30"
+                placeholder="ZIM title"
+                required
+              />
+              <small class="form-text text-muted">{{ zimTitle.length }}/30 characters</small>
+              <div class="invalid-feedback">Please provide a title</div>
+            </div>
             <div id="desc-group" ref="form_group" class="form-group">
               <label for="desc">Description</label>
               <input
@@ -78,6 +93,7 @@
                 placeholder="ZIM file created from a WP1 Selection"
                 required
               />
+              <small class="form-text text-muted">{{ description.length }}/80 characters</small>
               <div class="invalid-feedback">Please provide a description</div>
             </div>
             <div class="form-group">
@@ -160,6 +176,7 @@ export default {
   components: { SecondaryNav, LoginRequired, PulseLoader },
   data: function () {
     return {
+      zimTitle: '',
       description: '',
       errors: [],
       errorMessages: [],
@@ -198,6 +215,10 @@ export default {
       } else {
         this.notFound = false;
         this.builder = await response.json();
+        // Set default ZIM title from builder name, truncated to 30 chars if necessary
+        if (this.builder && this.builder.name) {
+          this.zimTitle = this.builder.name.substring(0, 30);
+        }
         this.$emit('onBuilderLoaded', this.builder);
       }
     },
@@ -214,8 +235,6 @@ export default {
 
       const data = await response.json();
       this.status = data.status;
-      this.description = data.description;
-      this.longDescription = data.long_description;
       this.isDeleted = data.is_deleted;
       if (this.status === 'FILE_READY') {
         this.stopProgressPolling();
@@ -229,6 +248,7 @@ export default {
       const form = this.$refs.form;
       if (!form.checkValidity()) {
         this.$refs.form_group.classList.add('was-validated');
+        this.$refs.zimtitle_form_group.classList.add('was-validated');
         return;
       }
 
@@ -242,6 +262,7 @@ export default {
         method: 'post',
         credentials: 'include',
         body: JSON.stringify({
+          title: this.zimTitle,
           description: this.description,
           long_description: this.longDescription,
         }),

wp1/exceptions.py

@@ -25,5 +25,8 @@ class ObjectNotFoundError(Wp1Error):
 class UserNotAuthorizedError(Wp1Error):
   pass
 
+class InvalidZimMetadataError(Wp1Error):
+  pass
+
 class Wp1ScoreProcessingError(Wp1Error):
   pass

wp1/logic/builder.py

@@ -393,6 +393,7 @@ def schedule_zim_file(s3,
                       wp10db,
                       builder_id,
                       user_id=None,
+                      title='',
                       description='',
                       long_description=''):
   if isinstance(builder_id, str):
@@ -414,6 +415,7 @@ def schedule_zim_file(s3,
                                       redis,
                                       wp10db,
                                       builder,
+                                      title=title,
                                       description=description,
                                       long_description=long_description)
   selection = latest_selection_for(wp10db, builder_id,

wp1/logic/builder_test.py

@@ -4,7 +4,7 @@
 import attr
 
 from wp1.base_db_test import BaseWpOneDbTest
-from wp1.exceptions import ObjectNotFoundError, UserNotAuthorizedError, ZimFarmError
+from wp1.exceptions import ObjectNotFoundError, UserNotAuthorizedError, ZimFarmError, InvalidZimMetadataError
 from wp1.logic import builder as logic_builder
 from wp1.models.wp10.builder import Builder
 
@@ -844,13 +844,15 @@ def test_schedule_zim_file(self, patched_utcnow, patched_schedule_zim_file):
                                     self.wp10db,
                                     builder_id,
                                     user_id=1234,
+                                    title='test_title',
                                     description='a',
                                     long_description='z')
 
     patched_schedule_zim_file.assert_called_once_with(s3,
                                                       redis,
                                                       self.wp10db,
                                                       self.builder,
+                                                      title='test_title',
                                                       description='a',
                                                       long_description='z')
     with self.wp10db.cursor() as cursor:
@@ -866,6 +868,26 @@ def test_schedule_zim_file(self, patched_utcnow, patched_schedule_zim_file):
     self.assertEqual(b'a', data['z_description'])
     self.assertEqual(b'z', data['z_long_description'])
 
+  @patch('wp1.logic.builder.utcnow',
+         return_value=datetime.datetime(2022, 12, 25, 0, 1, 2))
+  @patch('wp1.logic.builder.zimfarm.get_zimfarm_token',
+         return_value="test_token")
+  def test_schedule_zim_file_long_name(self, patched_utcnow, patched_get_zimfarm_token):
+    redis = MagicMock()
+    s3 = MagicMock()
+
+    builder_id = self._insert_builder()
+ 
+    with self.assertRaises(InvalidZimMetadataError):
+      logic_builder.schedule_zim_file(s3,
+                                      redis,
+                                      self.wp10db,
+                                      builder_id,
+                                      user_id=1234,
+                                      title='A'*31,
+                                      description='a',
+                                      long_description='z')
+
   @patch('wp1.logic.builder.zimfarm.schedule_zim_file')
   def test_schedule_zim_file_404(self, patched_schedule_zim_file):
     redis = MagicMock()

wp1/web/builders.py

@@ -148,10 +148,20 @@ def create_zim_file_for_builder(builder_id):
   user_id = flask.session['user']['identity']['sub']
 
   data = flask.request.get_json()
+
+  title = data.get('title')
   desc = data.get('description')
+  
+  error_messages = []
+  if not title:
+    error_messages.append('Title is required for ZIM file')
+  
   if not desc:
-    return flask.jsonify(
-        {'error_messages': 'Description is required for ZIM file'}), 400
+    error_messages.append('Description is required for ZIM file')
+  
+  if error_messages:
+    return flask.jsonify({'error_messages': error_messages}), 400
+  
   long_desc = data.get('long_description')
 
   try:
@@ -160,6 +170,7 @@ def create_zim_file_for_builder(builder_id):
                                               wp10db,
                                               builder_id,
                                               user_id=user_id,
+                                              title=title,
                                               description=desc,
                                               long_description=long_desc)
   except ObjectNotFoundError:

wp1/web/builders_test.py

@@ -445,7 +445,7 @@ def test_create_zim_file_for_builder(self, patched_schedule_zim_file):
       with client.session_transaction() as sess:
         sess['user'] = self.USER
       rv = client.post('/v1/builders/%s/zim' % builder_id,
-                       json={'description': 'Test description'})
+                       json={'title': 'Test title', 'description': 'Test description'})
       self.assertEqual('204 NO CONTENT', rv.status)
 
     patched_schedule_zim_file.assert_called_once()
@@ -468,7 +468,7 @@ def test_create_zim_file_for_builder_not_found(self,
       with client.session_transaction() as sess:
         sess['user'] = self.USER
       rv = client.post('/v1/builders/1234-not-found/zim',
-                       json={'description': 'Test description'})
+                       json={'title': 'Test title', 'description': 'Test description'})
       self.assertEqual('404 NOT FOUND', rv.status)
 
   @patch('wp1.zimfarm.schedule_zim_file')
@@ -482,7 +482,7 @@ def test_create_zim_file_for_builder_unauthorized(self,
       with client.session_transaction() as sess:
         sess['user'] = self.UNAUTHORIZED_USER
       rv = client.post('/v1/builders/%s/zim' % builder_id,
-                       json={'description': 'Test description'})
+                       json={'title': 'Test title', 'description': 'Test description'})
       self.assertEqual('403 FORBIDDEN', rv.status)
 
   @patch('wp1.zimfarm.schedule_zim_file')
@@ -497,7 +497,7 @@ def test_create_zim_file_for_builder_500(self, patched_schedule_zim_file):
       with client.session_transaction() as sess:
         sess['user'] = self.USER
       rv = client.post('/v1/builders/%s/zim' % builder_id,
-                       json={'description': 'Test description'})
+                       json={'title': 'Test title', 'description': 'Test description'})
       self.assertEqual('500 INTERNAL SERVER ERROR', rv.status)
 
   @patch('wp1.zimfarm.schedule_zim_file')
@@ -512,6 +512,18 @@ def test_create_zim_file_for_builder_400(self, patched_schedule_zim_file):
       rv = client.post('/v1/builders/%s/zim' % builder_id, json={})
       self.assertEqual('400 BAD REQUEST', rv.status)
 
+  @patch('wp1.zimfarm.schedule_zim_file')
+  def test_create_zim_file_for_builder_no_title(self, patched_schedule_zim_file):
+    builder_id = self._insert_builder()
+    self._insert_selections(builder_id)
+
+    self.app = create_app()
+    with self.override_db(self.app), self.app.test_client() as client:
+      with client.session_transaction() as sess:
+        sess['user'] = self.USER
+      rv = client.post('/v1/builders/%s/zim' % builder_id, json={'description': 'Test description'})
+      self.assertEqual('400 BAD REQUEST', rv.status)
+
   @patch('wp1.web.builders.queues.poll_for_zim_file_status')
   def test_update_zimfarm_status(self, patched_poll):
     builder_id = self._insert_builder()

wp1/zimfarm.py

@@ -10,14 +10,18 @@
 import wp1.logic.selection as logic_selection
 from wp1.constants import WP1_USER_AGENT
 from wp1.credentials import CREDENTIALS, ENV
-from wp1.exceptions import ObjectNotFoundError, ZimFarmError
+from wp1.exceptions import ObjectNotFoundError, ZimFarmError, InvalidZimMetadataError
 from wp1.logic import util
 from wp1.time import get_current_datetime
 from wp1.timestamp import utcnow
 
 MWOFFLINER_IMAGE = 'ghcr.io/openzim/mwoffliner:latest'
 REDIS_AUTH_KEY = 'zimfarm.auth'
 
+# ZIM metadata limits as per https://wiki.openzim.org/wiki/Metadata
+ZIM_TITLE_MAX_LENGTH = 30
+ZIM_DESCRIPTION_MAX_LENGTH = 80
+
 logger = logging.getLogger(__name__)
 
 
@@ -121,7 +125,23 @@ def get_webhook_url():
                                                  urllib.parse.quote(token))
 
 
-def _get_params(s3, wp10db, builder, description='', long_description=''):
+def validate_zim_metadata(title=None, description=None):
+  """Validate ZIM metadata fields against length limits."""
+  errors = []
+  print(f"Validating ZIM metadata: title='{title}', description='{description}'")
+  if title and len(title) > ZIM_TITLE_MAX_LENGTH:
+    errors.append(f"Title exceeds maximum length: {ZIM_TITLE_MAX_LENGTH} characters.")
+  
+  if description and len(description) > ZIM_DESCRIPTION_MAX_LENGTH:
+    errors.append(f"Description exceeds maximum length: {ZIM_DESCRIPTION_MAX_LENGTH} characters.")
+    
+  if errors:
+    return False, " ".join(errors)
+  
+  return True, None
+
+
+def _get_params(s3, wp10db, builder, title='', description='', long_description=''):
   if builder is None:
     raise ObjectNotFoundError('Given builder was None: %r' % builder)
 
@@ -150,7 +170,7 @@ def _get_params(s3, wp10db, builder, description='', long_description=''):
           'articleList':
               logic_selection.url_for_selection(selection),
           'customZimTitle':
-              builder.b_name.decode('utf-8'),
+              title,
           'customZimDescription':
               description
               if description else 'ZIM file created from a WP1 Selection',
@@ -193,6 +213,7 @@ def schedule_zim_file(s3,
                       redis,
                       wp10db,
                       builder,
+                      title='',
                       description='',
                       long_description=''):
   token = get_zimfarm_token(redis)
@@ -201,10 +222,18 @@ def schedule_zim_file(s3,
 
   if builder is None:
     raise ObjectNotFoundError('Cannot schedule for None builder')
+  
+  valid_metadata, error_message = validate_zim_metadata(
+      title=title,
+      description=description,
+  )
+  if not valid_metadata:
+    raise InvalidZimMetadataError(error_message)
 
   params = _get_params(s3,
                        wp10db,
                        builder,
+                       title=title,
                        description=description,
                        long_description=long_description)
   base_url = get_zimfarm_url()
@@ -314,7 +343,7 @@ def cancel_zim_by_task_id(redis, task_id):
 
   token = get_zimfarm_token(redis)
   if token is None:
-    raise ZimfarmError('Error retrieving auth token for request')
+    raise ZimFarmError('Error retrieving auth token for request')
   base_url = get_zimfarm_url()
   headers = _get_zimfarm_headers(token)