Simplify feature flag removal process

The production and staging environments now use separate databases, so we don't
need the two-step removal process any more.

h/models/feature.py

@@ -18,26 +18,6 @@
     "export_formats": "Allow users to select the format for their annotations export file",
 }
 
-# Once a feature has been fully deployed, we remove the flag from the codebase.
-# We can't do this in one step, because removing it entirely will cause stage
-# to remove the flag data from the database on boot, which will in turn disable
-# the feature in prod.
-#
-# Instead, the procedure for removing a feature is as follows:
-#
-# 1. Remove all feature lookups for the named feature throughout the code.
-#
-# 2. Move the feature to FEATURES_PENDING_REMOVAL. This ensures that the
-#    feature won't show up in the admin panel, and any uses of the feature will
-#    provoke UnknownFeatureErrors (server-side) or console warnings
-#    (client-side).
-#
-# 3. Deploy these changes all the way out to production.
-#
-# 4. Finally, remove the feature from FEATURES_PENDING_REMOVAL.
-#
-FEATURES_PENDING_REMOVAL = {}
-
 
 class Feature(Base):
     """A feature flag for the application."""
@@ -106,9 +86,7 @@ def remove_old_flags(cls, session):
 
         This function removes unknown feature flags from the database.
         """
-        # N.B. We remove only those features we know absolutely nothing about,
-        # which means that FEATURES_PENDING_REMOVAL are left alone.
-        known = set(FEATURES) | set(FEATURES_PENDING_REMOVAL)
+        known = set(FEATURES)
         unknown_flags = session.query(cls).filter(sa.not_(cls.name.in_(known)))
         count = unknown_flags.delete(synchronize_session=False)
         if count > 0:  # pragma: no cover

tests/unit/h/models/feature_test.py

@@ -5,7 +5,7 @@
 from h.models import Feature
 
 
-@pytest.mark.usefixtures("features_override", "features_pending_removal_override")
+@pytest.mark.usefixtures("features_override")
 class TestFeature:
     def test_description_returns_hardcoded_description(self):
         feat = Feature(name="notification")
@@ -20,12 +20,11 @@ def test_all_creates_annotations_that_dont_exist(self, db_session):
 
     def test_all_only_returns_current_flags(self, db_session):
         """The .all() method should only return named current feature flags."""
-        new, pending, old = [
+        new, old = [
             Feature(name="notification"),
-            Feature(name="abouttoberemoved"),
             Feature(name="somethingelse"),
         ]
-        db_session.add_all([new, pending, old])
+        db_session.add_all([new, old])
         db_session.flush()
 
         features = Feature.all(db_session)
@@ -37,21 +36,20 @@ def test_remove_old_flag_removes_old_flags(self, db_session):
         """
         The remove_old_flags function should remove unknown flags.
 
-        New flags and flags pending removal should be left alone, but completely
+        New flags should be left alone, but completely
         unknown flags should be removed.
         """
-        new, pending, old = [
+        new, old = [
             Feature(name="notification"),
-            Feature(name="abouttoberemoved"),
             Feature(name="somethingelse"),
         ]
-        db_session.add_all([new, pending, old])
+        db_session.add_all([new, old])
         db_session.flush()
 
         Feature.remove_old_flags(db_session)
 
         remaining = {f.name for f in db_session.query(Feature).all()}
-        assert remaining == {"abouttoberemoved", "notification"}
+        assert remaining == {"notification"}
 
     @pytest.fixture
     def features_override(self, request):
@@ -63,14 +61,3 @@ def features_override(self, request):
         )
         patcher.start()
         request.addfinalizer(patcher.stop)
-
-    @pytest.fixture
-    def features_pending_removal_override(self, request):
-        # And configure 'abouttoberemoved' as a feature pending removal...
-        patcher = mock.patch.dict(
-            "h.models.feature.FEATURES_PENDING_REMOVAL",
-            {"abouttoberemoved": "A test flag that's about to be removed."},
-            clear=True,
-        )
-        patcher.start()
-        request.addfinalizer(patcher.stop)