Use an ARRAY for edit log entries

Using a ```-separated string is *very* strange and would break when some change line contained that string. To convert an existing database without recreating it, use this query: ALTER TABLE reservation_edit_logs ALTER COLUMN info TYPE varchar[] USING string_to_array(info, '```');
javfg · Jun 12, 2014 · 2459728 · 2459728
1 parent 751908c
commit 2459728
Show file tree

Hide file tree

Showing 7 changed files with 16 additions and 13 deletions.
diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md
@@ -8,3 +8,5 @@ For the moment, we are focusing on _PostgreSQL_. These are some of the specific
 * `array_agg()`, since MySQL doesn't have ARRAY type. You can find occurrences of them within:
     - _indico/modules/rb/models/locations.py_
     - _indico/modules/rb/models/rooms.py_
+
+* ReservationEditLog uses a PostgreSQL ARRAY to store the changes for a single log entry
diff --git a/bin/migration/migrate_to_sqlalchemy.py b/bin/migration/migrate_to_sqlalchemy.py
@@ -406,7 +406,7 @@ def migrate_reservations(rb_root):
                 el = ReservationEditLog(
                     timestamp=ts,
                     user_name=h._responsibleUser,
-                    info=convert_to_unicode('```'.join(h._info))  # TODO: store this properly!
+                    info=map(convert_to_unicode, h._info)
                 )
                 r.edit_logs.append(el)
 

diff --git a/indico/MaKaC/webinterface/tpls/RoomBookingDetails.tpl b/indico/MaKaC/webinterface/tpls/RoomBookingDetails.tpl
@@ -417,15 +417,14 @@
                                       </dl>
                                       <dl class="booking-history old" style="display: none;">
                                   % endif
-                                  <% info = entry.info.split('```') %>
                                   <dt>${ formatDateTime(entry.timestamp) }</dt>
                                   <dd>
-                                      <strong>${ info[0] }</strong> ${ _('by') } ${ entry.user_name }
-                                      % if len(info) > 1:
+                                      <strong>${ entry.info[0] }</strong> ${ _('by') } ${ entry.user_name }
+                                      % if len(entry.info) > 1:
                                           (<a class="js-history-show-details" href="#" data-other-text="${ _('Hide Details') }">${ _('Show Details') }</a>)
                                       % endif
                                   </dd>
-                                  % for detail in info[1:]:
+                                  % for detail in entry.info[1:]:
                                       <dd style="display: none;">
                                           ${ detail }
                                       </dd>

diff --git a/indico/htdocs/js/indico/RoomBooking/validation.js b/indico/htdocs/js/indico/RoomBooking/validation.js
@@ -148,7 +148,7 @@
             }
         });
 
-        $('#submit_check, #submit_prebook, #submit_book').on('click', function(e) {
+        $('.js-submit-booking, #submit_check').on('click', function(e) {
             if ($(this).data('validation') == 'check') {
                 validatingConflicts = true;
                 validatingBooking = false;

diff --git a/indico/modules/rb/models/reservation_edit_logs.py b/indico/modules/rb/models/reservation_edit_logs.py
@@ -21,6 +21,8 @@
 Schema of modifications done on a reservation
 """
 
+from sqlalchemy.dialects.postgresql import ARRAY
+
 from indico.core.db import db
 from indico.core.db.sqlalchemy.custom.utcdatetime import UTCDateTime
 from indico.util.date_time import now_utc
@@ -40,7 +42,7 @@ class ReservationEditLog(db.Model):
         default=now_utc
     )
     info = db.Column(
-        db.String,
+        ARRAY(db.String),
         nullable=False
     )
     user_name = db.Column(

diff --git a/indico/modules/rb/models/reservations.py b/indico/modules/rb/models/reservations.py
@@ -618,13 +618,13 @@ def modify(self, data, user):
             old = converter(change['old'])
             new = converter(change['new'])
             if not old:
-                log.append("The {} was set to '{}'".format(field_title, new))
+                log.append(u"The {} was set to '{}'".format(field_title, new))
             elif not new:
-                log.append("The {} was cleared".format(field_title))
+                log.append(u"The {} was cleared".format(field_title))
             else:
-                log.append("The {} was changed from '{}' to '{}'".format(field_title, old, new))
+                log.append(u"The {} was changed from '{}' to '{}'".format(field_title, old, new))
 
-        self.edit_logs.append(ReservationEditLog(user_name=user.getFullName(), info='```'.join(log)))
+        self.edit_logs.append(ReservationEditLog(user_name=user.getFullName(), info=log))
 
         # Recreate all occurrence if necessary
         if update_occurrences:

diff --git a/indico/tests/db/data.py b/indico/tests/db/data.py
@@ -91,12 +91,12 @@ def _create_utc_dt(*args):
     {
         'timestamp': _create_utc_dt(2013, 12, 2, 11, 33, 57),
         'user_name': 'johhny',
-        'info': 'name updated```flag is cleared'
+        'info': ['name updated', 'flag is cleared']
     },
     {
         'timestamp': _create_utc_dt(2013, 12, 2, 11, 34, 17),
         'user_name': 'admin',
-        'info': 'removed bad words from reason```some value changed```set flag'
+        'info': ['removed bad words from reason', 'some value changed', 'set flag']
     }
 ]