From 7bcf168ae6c9022fcce697871041e7ad3c2401c8 Mon Sep 17 00:00:00 2001
From: Jan Kaluza <jkaluza@redhat.com>
Date: Jun 27 2019 05:29:02 +0000
Subject: Accept all possible `state` representation in `transition()` methods.


One can for example pass EventState.COMPLETE, EventState.COMPLETE.value
or "complete" as a `state` in `transition` method.
We so far supported just passing the `EventState.COMPLETE.value`, but
some parts of code used `EventState.COMPLETE`.

In this commit, the `transition` methods are changed to support all
three ways.

---

diff --git a/freshmaker/models.py b/freshmaker/models.py
index 6e7d2ac..a5c83cc 100644
--- a/freshmaker/models.py
+++ b/freshmaker/models.py
@@ -331,10 +331,12 @@ class Event(FreshmakerBase):
         :param state_reason: Reason why this state has been set.
         :return: True/False, whether state was changed
         """
+        # Convert state from its possible representation to number.
+        state = self.validate_state("state", state)
 
-        # Log the time done
-        if state == EventState.FAILED.value or state == EventState.COMPLETE.value:
-            self.time_done = datetime.utcnow()
+        # Update the state reason.
+        if state_reason is not None:
+            self.state_reason = state_reason
 
         # Log the state and state_reason
         if state == EventState.FAILED.value:
@@ -344,14 +346,19 @@ class Event(FreshmakerBase):
         log_fnc("Event %r moved to state %s, %r" % (
             self, EventState(state).name, state_reason))
 
+        # In case Event is already in the state, return False.
         if self.state == state:
             return False
 
         self.state = state
+
+        # Log the time done
+        if state in [EventState.FAILED.value, EventState.COMPLETE.value,
+                     EventState.SKIPPED.value]:
+            self.time_done = datetime.utcnow()
+
         if EventState(state).counter:
             EventState(state).counter.inc()
-        if state_reason is not None:
-            self.state_reason = state_reason
 
         db.session.commit()
         messaging.publish('event.state.changed', self.json())
@@ -572,6 +579,8 @@ class ArtifactBuild(FreshmakerBase):
         :param state_reason: Reason why this state has been set.
         :return: True/False, whether state was changed
         """
+        # Convert state from its possible representation to number.
+        state = self.validate_state("state", state)
 
         # Log the state and state_reason
         if state == ArtifactBuildState.FAILED.value:
diff --git a/tests/test_models.py b/tests/test_models.py
index 8f07208..c210f40 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -95,6 +95,14 @@ class TestModels(helpers.ModelsTestCase):
         deps = set(parent.depending_artifact_builds())
         self.assertEqual(deps, set([build2, build3]))
 
+    def test_event_transition(self):
+        for i, state in enumerate([
+                EventState.COMPLETE, EventState.COMPLETE.value, "complete"]):
+            event = Event.create(db.session, "test_msg_id_{}".format(i), "test", events.TestingEvent)
+            event.transition(state, "reason")
+            self.assertEqual(event.state, EventState.COMPLETE.value)
+            self.assertTrue(event.time_done is not None)
+
     def test_build_transition_recursion(self):
         for i, state in enumerate([ArtifactBuildState.FAILED.value,
                                    ArtifactBuildState.CANCELED.value]):
diff --git a/tests/test_views.py b/tests/test_views.py
index c21b57e..5e9c354 100644
--- a/tests/test_views.py
+++ b/tests/test_views.py
@@ -678,7 +678,7 @@ class TestManualTriggerRebuild(helpers.ModelsTestCase):
         # Other fields are predictible.
         self.assertEqual(data['requested_rebuilds'], ["foo-1-1"])
         assert add_dependency.call_count == 1
-        assert "RHSA-2018-103" == add_dependency.call_args.args[1].search_key
+        assert "RHSA-2018-103" == add_dependency.call_args[0][1].search_key
         publish.assert_called_once_with(
             'manual.rebuild',
             {'msg_id': 'manual_rebuild_123', u'errata_id': 1,