Bug 1310213 - enable scheduled deletions (mozilla-releng#182). r=bhea…

…rsum

auslib/admin/views/forms.py

@@ -213,17 +213,27 @@ class EditRuleForm(DbEditableForm):
 
 
 class ScheduledChangeNewRuleForm(ScheduledChangeForm, RuleForm):
-    pass
+    change_type = SelectField("Change Type", choices=[('insert', 'insert'), ('update', 'update'), ('delete'), ('delete')])
 
 
 class ScheduledChangeExistingRuleForm(ScheduledChangeForm, EditRuleForm):
     # EditRuleForm doesn't have rule_id in it because rules are edited through
     # URLs that contain them. Scheduled changes, on the other hand, are edited
     # through URLs that contain scheduled change IDs, so we need to include
     # the rule_id in the form when editing scheduled changes for rules.
+    change_type = SelectField("Change Type", choices=[('insert', 'insert'), ('update', 'update'), ('delete'), ('delete')])
     rule_id = IntegerField('Rule ID', validators=[Required()])
 
 
+class ScheduledChangeDeleteRuleForm(ScheduledChangeForm):
+    """
+    ScheduledChangeDeletionForm includes all the PK columns ,ScheduledChangeForm columns and data version
+    """
+    change_type = SelectField("Change Type", choices=[('insert', 'insert'), ('update', 'update'), ('delete', 'delete')])
+    rule_id = IntegerField('Rule ID', validators=[Required()])
+    data_version = IntegerField('data_version', widget=HiddenInput())
+
+
 class EditScheduledChangeNewRuleForm(ScheduledChangeForm, RuleForm):
     sc_data_version = IntegerField('sc_data_version', validators=[Required()], widget=HiddenInput())
 

auslib/admin/views/rules.py

@@ -11,7 +11,8 @@
 from auslib.admin.views.csrf import get_csrf_headers
 from auslib.admin.views.forms import EditRuleForm, RuleForm, DbEditableForm, \
     ScheduledChangeNewRuleForm, ScheduledChangeExistingRuleForm, \
-    EditScheduledChangeNewRuleForm, EditScheduledChangeExistingRuleForm
+    ScheduledChangeDeleteRuleForm, EditScheduledChangeNewRuleForm, \
+    EditScheduledChangeExistingRuleForm
 from auslib.admin.views.scheduled_changes import ScheduledChangesView, \
     ScheduledChangeView, EnactScheduledChangeView, ScheduledChangeHistoryView, \
     SignoffsView
@@ -299,14 +300,36 @@ def __init__(self):
 
     @requirelogin
     def _post(self, transaction, changed_by):
-        if request.json and request.json.get("data_version"):
+
+        change_type = request.json.get("change_type")
+
+        if change_type == "update":
             form = ScheduledChangeExistingRuleForm()
-        else:
+
+            releaseNames = dbo.releases.getReleaseNames(transaction=transaction)
+
+            self.log.debug("releaseNames: %s" % (releaseNames))
+            self.log.debug("transaction: %s" % (transaction))
+
+            form.mapping.choices = [(item['name'], item['name']) for item in releaseNames]
+            form.mapping.choices.insert(0, ('', 'NULL'))
+
+        elif change_type == "insert":
             form = ScheduledChangeNewRuleForm()
 
-        releaseNames = dbo.releases.getReleaseNames(transaction=transaction)
-        form.mapping.choices = [(item['name'], item['name']) for item in releaseNames]
-        form.mapping.choices.insert(0, ('', 'NULL'))
+            releaseNames = dbo.releases.getReleaseNames(transaction=transaction)
+
+            self.log.debug("releaseNames: %s" % (releaseNames))
+            self.log.debug("transaction: %s" % (transaction))
+
+            form.mapping.choices = [(item['name'], item['name']) for item in releaseNames]
+            form.mapping.choices.insert(0, ('', 'NULL'))
+
+        elif change_type == "delete":
+            form = ScheduledChangeDeleteRuleForm()
+
+        else:
+            return Response(status=400, response="Change Type invalid or not entered")
 
         return super(RuleScheduledChangesView, self)._post(form, transaction, changed_by)
 
@@ -323,6 +346,7 @@ def _post(self, sc_id, transaction, changed_by):
             form = EditScheduledChangeNewRuleForm()
 
         releaseNames = dbo.releases.getReleaseNames(transaction=transaction)
+
         form.mapping.choices = [(item['name'], item['name']) for item in releaseNames]
         form.mapping.choices.insert(0, ('', 'NULL'))
 

auslib/db.py

@@ -871,6 +871,7 @@ def __init__(self, db, dialect, metadata, baseTable, conditions=("time", "uptake
                            Column("sc_id", Integer, primary_key=True, autoincrement=True),
                            Column("scheduled_by", String(100), nullable=False),
                            Column("complete", Boolean, default=False),
+                           Column("change_type", String(50), nullable=False),
                            )
         self.conditions = ConditionsTable(db, dialect, metadata, table_name, conditions)
         # Signoffs are configurable at runtime, which means that we always need
@@ -935,10 +936,17 @@ def _splitColumns(self, columns):
         return base_columns, condition_columns
 
     def _checkBaseTablePermissions(self, base_table_where, new_row, changed_by, transaction):
-        if new_row.get("data_version"):
+        if "change_type" not in new_row:
+            raise ValueError("change_type needed to check Permission")
+
+        if new_row.get("change_type") == "update":
             self.baseTable.update(base_table_where, new_row, changed_by, new_row["data_version"], transaction=transaction, dryrun=True)
-        else:
+        elif new_row.get("change_type") == "insert":
             self.baseTable.insert(changed_by, transaction=transaction, dryrun=True, **new_row)
+        elif new_row.get("change_type") == "delete":
+            self.baseTable.delete(base_table_where, changed_by, new_row["data_version"], transaction=transaction, dryrun=True)
+        else:
+            raise ValueError("Unknown Change Type")
 
     def _dataVersionsAreSynced(self, sc_id, transaction):
         sc_row = super(ScheduledChangeTable, self).select(where=[self.sc_id == sc_id], transaction=transaction, columns=[self.data_version])
@@ -959,6 +967,14 @@ def validate(self, base_columns, condition_columns, changed_by, sc_id=None, tran
         # it easy to do the extra checks conditionally afterwards.
         base_table_where = []
         sc_table_where = []
+
+        if base_columns["change_type"] == "delete":
+            for pk in self.base_primary_key:
+                if pk not in base_columns:
+                    raise ValueError("Missing primary key column %s. PK values needed for deletion" % (pk))
+                if base_columns[pk] is None:
+                    raise ValueError("%s value found to be None. PK value can not be None for deletion" % (pk))
+
         for pk in self.base_primary_key:
             base_column = getattr(self.baseTable, pk)
             if pk in base_columns:
@@ -996,6 +1012,7 @@ def validate(self, base_columns, condition_columns, changed_by, sc_id=None, tran
             sc_table_where.append(self.complete == False) # noqa because we need to use == for sqlalchemy operator overloading to work
             if len(self.select(columns=[self.sc_id], where=sc_table_where)) > 0:
                 raise ChangeScheduledError("Cannot scheduled a change for a row with one already scheduled")
+        self.log.debug("base_columns: %s" % (base_columns))
 
         self.conditions.validate(condition_columns)
         self._checkBaseTablePermissions(base_table_where, base_columns, changed_by, transaction)
@@ -1017,11 +1034,14 @@ def select(self, where=None, transaction=None, **kwargs):
 
     def insert(self, changed_by, transaction=None, dryrun=False, **columns):
         base_columns, condition_columns = self._splitColumns(columns)
+        if "change_type" not in base_columns:
+            raise ValueError("Change type is required")
 
-        self.validate(base_columns, condition_columns, changed_by, transaction)
+        self.validate(base_columns=base_columns, condition_columns=condition_columns, changed_by=changed_by, transaction=transaction)
 
         base_columns = self._prefixColumns(base_columns)
         base_columns["scheduled_by"] = changed_by
+
         ret = super(ScheduledChangeTable, self).insert(changed_by=changed_by, transaction=transaction, dryrun=dryrun, **base_columns)
         if not dryrun:
             sc_id = ret.inserted_primary_key[0]
@@ -1055,6 +1075,9 @@ def update(self, where, what, changed_by, old_data_version, transaction=None, dr
                 elif sc_columns.get(col):
                     base_columns[base_col] = sc_columns[col]
 
+            # As we need change_type in base_columns and it does not start with "base_". We assign it outside the loop
+            base_columns["change_type"] = sc_columns["change_type"]
+
             # Similarly, we need to integrate the new values for any conditions
             # with the existing ones.
             condition_columns.update(condition_what)
@@ -1077,6 +1100,8 @@ def delete(self, where, changed_by=None, old_data_version=None, transaction=None
         for row in self.select(where=where, transaction=transaction):
             conditions_where.append(self.conditions.sc_id == row["sc_id"])
             base_row = {col[5:]: row[col] for col in row if col.startswith("base_")}
+            # we also need change_type in base_row to check permission
+            base_row["change_type"] = row["change_type"]
             base_table_where = {pk: row["base_%s" % pk] for pk in self.base_primary_key}
             # TODO: What permissions *should* be required to delete a scheduled change?
             # It seems a bit odd to be checking base table update/insert here. Maybe
@@ -1095,6 +1120,7 @@ def enactChange(self, sc_id, enacted_by, transaction=None):
 
         sc = self.select(where=[self.sc_id == sc_id], transaction=transaction)[0]
         what = {}
+        change_type = sc["change_type"]
         for col in sc:
             if col.startswith("base_"):
                 what[col[5:]] = sc[col]
@@ -1114,13 +1140,20 @@ def enactChange(self, sc_id, enacted_by, transaction=None):
 
         # If the scheduled change had a data version, it means the row already
         # exists, and we need to use update() to enact it.
-        if what["data_version"]:
+        if change_type == "delete":
+            where = []
+            for col in self.base_primary_key:
+                where.append((getattr(self.baseTable, col) == sc["base_%s" % col]))
+            self.baseTable.delete(where, sc["scheduled_by"], sc["base_data_version"], transaction=transaction)
+        elif change_type == "update":
             where = []
             for col in self.base_primary_key:
                 where.append((getattr(self.baseTable, col) == sc["base_%s" % col]))
             self.baseTable.update(where, what, sc["scheduled_by"], sc["base_data_version"], transaction=transaction)
-        else:
+        elif change_type == "insert":
             self.baseTable.insert(sc["scheduled_by"], transaction=transaction, **what)
+        else:
+            raise ValueError("Unknown Change Type")
 
     def mergeUpdate(self, old_row, what, changed_by, transaction=None):
         """Merges an update to the base table into any changes that may be
@@ -1136,6 +1169,7 @@ def mergeUpdate(self, old_row, what, changed_by, transaction=None):
             where.append((getattr(self, "base_%s" % col) == old_row[col]))
 
         scheduled_changes = self.select(where=where, transaction=transaction)
+
         if not scheduled_changes:
             self.log.debug("No scheduled changes found for update; nothing to do")
             return

auslib/migrate/versions/019_add_change_type.py

@@ -0,0 +1,50 @@
+from sqlalchemy import Column, String, MetaData, Table
+
+
+def upgrade(migrate_engine):
+    metadata = MetaData(bind=migrate_engine)
+
+    # In order to add a column a nullable=False:
+    # 1) Add the column with nullable=True
+    change_type = Column("change_type", String(50))
+    change_type.create(Table("rules_scheduled_changes", metadata, autoload=True))
+
+    # 2) Update the values of change_type depending on base_data_version
+
+    migrate_engine.execute("""
+        UPDATE rules_scheduled_changes
+        SET change_type = "insert"
+        WHERE base_data_version is NULL;
+        """)
+    migrate_engine.execute("""
+        UPDATE rules_scheduled_changes
+        SET change_type = "update"
+        WHERE base_data_version is not NULL;
+        """)
+
+    # 3) Alter the column and set nullable=False
+    change_type.alter(nullable=False)
+
+    change_type = Column("change_type", String(50))
+    change_type.create(Table("rules_scheduled_changes_history", metadata,
+                             autoload=True))
+
+    migrate_engine.execute("""
+        UPDATE rules_scheduled_changes_history
+        SET change_type = "insert"
+        WHERE base_data_version is NULL;
+        """)
+    migrate_engine.execute("""
+        UPDATE rules_scheduled_changes_history
+        SET change_type = "update"
+        WHERE base_data_version is not NULL;
+        """)
+    rules_scheduled_changes = Table("rules_scheduled_changes", metadata, autoload=True)
+    rules_scheduled_changes.c.base_update_type.alter(nullable=True)
+
+
+def downgrade(migrate_engine):
+    metadata = MetaData(bind=migrate_engine)
+    Table("rules_scheduled_changes", metadata, autoload=True).c.change_type.drop()
+    Table("rules_scheduled_changes_history", metadata,
+          autoload=True).c.change_type.drop()