From 7f02741dc6e8f9a80851e1ef470dbb59f9b27f57 Mon Sep 17 00:00:00 2001 From: James Richardson Date: Feb 13 2023 15:42:47 +0000 Subject: Conform to Fedora Messaging conventions Services should send an 'agent_name' consistently in messages so FMN can process them easily. This renames the property accordingly and adds 'agent' as an alias which emits a warning when accessed. Signed-off-by: james02135 Error in base.py, self.agent_name was wrapped in a list Signed-off-by: james02135 --- diff --git a/fedora_elections_messages/base.py b/fedora_elections_messages/base.py index 7209415..bcb6b9d 100644 --- a/fedora_elections_messages/base.py +++ b/fedora_elections_messages/base.py @@ -13,6 +13,7 @@ # You should have received a copy of the GNU General Public License along # with this program; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +import warnings from fedora_messaging import message from fedora_messaging.schema_utils import user_avatar_url @@ -72,15 +73,24 @@ class ElectionsMessage(message.Message): @property def agent(self): - return self.body.get("agent") + warnings.warn( + "agent property is deprecated, please use agent_name instead", + DeprecationWarning, + stacklevel=2, + ) + return self.body.get("agent_name") + + @property + def agent_name(self): + return self.body.get("agent_name") @property def agent_avatar(self): - return user_avatar_url(self.agent) + return user_avatar_url(self.agent_name) @property def usernames(self): - return [self.agent] + return [self.agent_name] @property def url(self): diff --git a/fedora_elections_messages/candidate_delete.py b/fedora_elections_messages/candidate_delete.py index 97e52cc..16b5db2 100644 --- a/fedora_elections_messages/candidate_delete.py +++ b/fedora_elections_messages/candidate_delete.py @@ -32,19 +32,21 @@ class DeleteCandidateV1(CandidateMessage): "type": "object", "properties": { "agent": {"type": "string"}, + "agent_name": {"type": "string"}, "election": ELECTION_SCHEMA, "candidate": CANDIDATE_SCHEMA, }, - "required": ["agent", "election", "candidate"], + "required": ["agent_name", "election", "candidate"], } def __str__(self): """Return a complete human-readable representation of the message.""" return ( - "Candidate for Election {election} was deleted\nBy: {agent}\nCandidate: {candidate}\n" + "Candidate for Election {election} was deleted" + "\nBy: {agent_name}\nCandidate: {candidate}\n" ).format( election=self.body["election"]["shortdesc"], - agent=self.body["agent"], + agent_name=self.body["agent_name"], candidate=self.body["candidate"]["name"], ) @@ -53,13 +55,16 @@ class DeleteCandidateV1(CandidateMessage): """Return a summary of the message.""" if self.body["candidate"].get("fas_name"): msg = ( - '{agent} deleted candidate "{candidate_name}" ({candidate_fas_name}) ' + '{agent_name} deleted candidate "{candidate_name}" ({candidate_fas_name}) ' 'for election "{title}" ({alias})' ) else: - msg = '{agent} deleted candidate "{candidate_name}" for election "{title}" ({alias})' + msg = ( + '{agent_name} deleted candidate "{candidate_name}"' + ' for election "{title}" ({alias})' + ) return msg.format( - agent=self.body["agent"], + agent_name=self.body["agent_name"], candidate_name=self.body["candidate"]["name"], candidate_fas_name=self.body["candidate"].get("fas_name"), title=self.body["election"]["shortdesc"], diff --git a/fedora_elections_messages/candidate_edit.py b/fedora_elections_messages/candidate_edit.py index a082763..412427f 100644 --- a/fedora_elections_messages/candidate_edit.py +++ b/fedora_elections_messages/candidate_edit.py @@ -32,19 +32,21 @@ class EditCandidateV1(CandidateMessage): "type": "object", "properties": { "agent": {"type": "string"}, + "agent_name": {"type": "string"}, "election": ELECTION_SCHEMA, "candidate": CANDIDATE_SCHEMA, }, - "required": ["agent", "election", "candidate"], + "required": ["agent_name", "election", "candidate"], } def __str__(self): """Return a complete human-readable representation of the message.""" return ( - "Candidate for Election {election} was edited\nBy: {agent}\nCandidate: {candidate}\n" + "Candidate for Election {election} was edited" + "\nBy: {agent_name}\nCandidate: {candidate}\n" ).format( election=self.body["election"]["shortdesc"], - agent=self.body["agent"], + agent_name=self.body["agent_name"], candidate=self.body["candidate"]["name"], ) @@ -53,13 +55,16 @@ class EditCandidateV1(CandidateMessage): """Return a summary of the message.""" if self.body["candidate"].get("fas_name"): msg = ( - '{agent} edited candidate "{candidate_name}" ({candidate_fas_name}) ' + '{agent_name} edited candidate "{candidate_name}" ({candidate_fas_name}) ' 'for election "{title}" ({alias})' ) else: - msg = '{agent} edited candidate "{candidate_name}" for election "{title}" ({alias})' + msg = ( + '{agent_name} edited candidate "{candidate_name}"' + ' for election "{title}" ({alias})' + ) return msg.format( - agent=self.body["agent"], + agent_name=self.body["agent_name"], candidate_name=self.body["candidate"]["name"], candidate_fas_name=self.body["candidate"].get("fas_name"), title=self.body["election"]["shortdesc"], diff --git a/fedora_elections_messages/candidate_new.py b/fedora_elections_messages/candidate_new.py index e50f8f1..d883fc2 100644 --- a/fedora_elections_messages/candidate_new.py +++ b/fedora_elections_messages/candidate_new.py @@ -32,19 +32,20 @@ class NewCandidateV1(CandidateMessage): "type": "object", "properties": { "agent": {"type": "string"}, + "agent_name": {"type": "string"}, "election": ELECTION_SCHEMA, "candidate": CANDIDATE_SCHEMA, }, - "required": ["agent", "election", "candidate"], + "required": ["agent_name", "election", "candidate"], } def __str__(self): """Return a complete human-readable representation of the message.""" return ( - "New Candidate for Election: {election}\nBy: {agent}\nCandidate: {candidate}\n" + "New Candidate for Election: {election}\nBy: {agent_name}\nCandidate: {candidate}\n" ).format( election=self.body["election"]["shortdesc"], - agent=self.body["agent"], + agent_name=self.body["agent_name"], candidate=self.body["candidate"]["name"], ) @@ -53,13 +54,13 @@ class NewCandidateV1(CandidateMessage): """Return a summary of the message.""" if self.body["candidate"].get("fas_name"): msg = ( - '{agent} added candidate "{candidate_name}" ({candidate_fas_name}) ' + '{agent_name} added candidate "{candidate_name}" ({candidate_fas_name}) ' 'for election "{title}" ({alias})' ) else: - msg = '{agent} added candidate "{candidate_name}" for election "{title}" ({alias})' + msg = '{agent_name} added candidate "{candidate_name}" for election "{title}" ({alias})' return msg.format( - agent=self.body["agent"], + agent_name=self.body["agent_name"], candidate_name=self.body["candidate"]["name"], candidate_fas_name=self.body["candidate"].get("fas_name"), title=self.body["election"]["shortdesc"], diff --git a/fedora_elections_messages/election_edit.py b/fedora_elections_messages/election_edit.py index b368f81..73e46b1 100644 --- a/fedora_elections_messages/election_edit.py +++ b/fedora_elections_messages/election_edit.py @@ -30,21 +30,26 @@ class EditElectionV1(ElectionsMessage): "$schema": "http://json-schema.org/draft-04/schema#", "description": "Schema for messages sent when an election is edited", "type": "object", - "properties": {"agent": {"type": "string"}, "election": ELECTION_SCHEMA}, - "required": ["agent", "election"], + "properties": { + "agent": {"type": "string"}, + "agent_name": {"type": "string"}, + "election": ELECTION_SCHEMA, + }, + "required": ["agent_name", "election"], } def __str__(self): """Return a complete human-readable representation of the message.""" - return "Election edited: {election}\nBy: {agent}\n".format( - election=self.body["election"]["shortdesc"], agent=self.body["agent"], + return "Election edited: {election}\nBy: {agent_name}\n".format( + election=self.body["election"]["shortdesc"], + agent_name=self.body["agent_name"], ) @property def summary(self): """Return a summary of the message.""" - return '{agent} edited election "{title}" ({alias})'.format( - agent=self.body["agent"], + return '{agent_name} edited election "{title}" ({alias})'.format( + agent_name=self.body["agent_name"], title=self.body["election"]["shortdesc"], alias=self.body["election"]["alias"], ) diff --git a/fedora_elections_messages/election_new.py b/fedora_elections_messages/election_new.py index 91efdf1..855715e 100644 --- a/fedora_elections_messages/election_new.py +++ b/fedora_elections_messages/election_new.py @@ -30,21 +30,26 @@ class NewElectionV1(ElectionsMessage): "$schema": "http://json-schema.org/draft-04/schema#", "description": "Schema for messages sent when a new election is created", "type": "object", - "properties": {"agent": {"type": "string"}, "election": ELECTION_SCHEMA}, - "required": ["agent", "election"], + "properties": { + "agent": {"type": "string"}, + "agent_name": {"type": "string"}, + "election": ELECTION_SCHEMA, + }, + "required": ["agent_name", "election"], } def __str__(self): """Return a complete human-readable representation of the message.""" - return "New Election: {election}\nBy: {agent}\n".format( - election=self.body["election"]["shortdesc"], agent=self.body["agent"], + return "New Election: {election}\nBy: {agent_name}\n".format( + election=self.body["election"]["shortdesc"], + agent_name=self.body["agent_name"], ) @property def summary(self): """Return a summary of the message.""" - return '{agent} created election "{title}" ({alias})'.format( - agent=self.body["agent"], + return '{agent_name} created election "{title}" ({alias})'.format( + agent_name=self.body["agent_name"], title=self.body["election"]["shortdesc"], alias=self.body["election"]["alias"], ) diff --git a/fedora_elections_messages/tests/test_candidate.py b/fedora_elections_messages/tests/test_candidate.py index adcd124..a27f323 100644 --- a/fedora_elections_messages/tests/test_candidate.py +++ b/fedora_elections_messages/tests/test_candidate.py @@ -31,7 +31,7 @@ def test_message(message_class): Assert the message schema validates a message with the required fields. """ body = { - "agent": "dummy-user", + "agent_name": "dummy-user", "election": DUMMY_ELECTION, "candidate": DUMMY_CANDIDATE, } @@ -45,7 +45,7 @@ def test_message(message_class): def test_missing_fields(message_class): """Assert an exception is actually raised on validation failure.""" minimal_message = { - "agent": "dummy-user", + "agent_name": "dummy-user", "election": DUMMY_ELECTION, "candidate": {"url": "http://elections.test/candidate"}, } @@ -65,7 +65,7 @@ def test_missing_fields(message_class): def test_str(message_class, expected_header): """Assert __str__ produces a human-readable message.""" body = { - "agent": "dummy-user", + "agent_name": "dummy-user", "election": DUMMY_ELECTION, "candidate": DUMMY_CANDIDATE, } @@ -89,7 +89,7 @@ def test_str(message_class, expected_header): def test_summary(message_class, action): """Assert the summary is correct.""" body = { - "agent": "dummy-user", + "agent_name": "dummy-user", "election": DUMMY_ELECTION, "candidate": DUMMY_CANDIDATE, } @@ -114,7 +114,7 @@ def test_summary_fasname(message_class, action): candidate = DUMMY_CANDIDATE.copy() candidate["fas_name"] = "dc" body = { - "agent": "dummy-user", + "agent_name": "dummy-user", "election": DUMMY_ELECTION, "candidate": candidate, } diff --git a/fedora_elections_messages/tests/test_common.py b/fedora_elections_messages/tests/test_common.py index 981deb2..804791b 100644 --- a/fedora_elections_messages/tests/test_common.py +++ b/fedora_elections_messages/tests/test_common.py @@ -1,4 +1,5 @@ import unittest +import pytest from ..election_new import NewElectionV1 from ..candidate_new import NewCandidateV1 @@ -9,7 +10,7 @@ class CommonTests(unittest.TestCase): def test_election_properties(self): """Assert some properties are correct.""" body = { - "agent": "dummy-user", + "agent_name": "dummy-user", "election": DUMMY_ELECTION, } message = NewElectionV1(body=body) @@ -18,7 +19,7 @@ class CommonTests(unittest.TestCase): self.assertEqual( message.app_icon, "https://apps.fedoraproject.org/img/icons/elections.png" ) - self.assertEqual(message.agent, "dummy-user") + self.assertEqual(message.agent_name, "dummy-user") self.assertEqual( message.agent_avatar, "https://seccdn.libravatar.org/avatar/" @@ -28,13 +29,22 @@ class CommonTests(unittest.TestCase): self.assertEqual(message.usernames, ["dummy-user"]) self.assertEqual(message.url, "http://elections.test/dummy/") + with pytest.warns(DeprecationWarning) as w: + self.assertEqual(message.agent, "dummy-user") + + assert len(w) == 1 + assert ( + w[0].message.args[0] + == "agent property is deprecated, please use agent_name instead" + ) + def test_candidate_properties(self): """Assert some properties are correct.""" candidate = DUMMY_CANDIDATE.copy() candidate["fas_name"] = "dc" candidate["url"] = "http://elections.test/candidate" body = { - "agent": "dummy-user", + "agent_name": "dummy-user", "election": DUMMY_ELECTION, "candidate": candidate, } diff --git a/fedora_elections_messages/tests/test_election.py b/fedora_elections_messages/tests/test_election.py index 6d41bb5..ee0a0ef 100644 --- a/fedora_elections_messages/tests/test_election.py +++ b/fedora_elections_messages/tests/test_election.py @@ -28,7 +28,7 @@ def test_message(message_class): Assert the message schema validates a message with the required fields. """ body = { - "agent": "dummy-user", + "agent_name": "dummy-user", "election": DUMMY_ELECTION, } message = message_class(body=body) @@ -39,7 +39,7 @@ def test_message(message_class): def test_missing_fields(message_class): """Assert an exception is actually raised on validation failure.""" minimal_message = { - "agent": "dummy-user", + "agent_name": "dummy-user", "election": {"shortdesc": "a dummy election", "alias": "dummy"}, } message = message_class(body=minimal_message) @@ -57,7 +57,7 @@ def test_missing_fields(message_class): def test_str(message_class, expected_header): """Assert __str__ produces a human-readable message.""" body = { - "agent": "dummy-user", + "agent_name": "dummy-user", "election": DUMMY_ELECTION, } expected_str = "{}\nBy: dummy-user\n".format(expected_header) @@ -73,7 +73,7 @@ def test_str(message_class, expected_header): def test_summary(message_class, action): """Assert the summary is correct.""" body = { - "agent": "dummy-user", + "agent_name": "dummy-user", "election": DUMMY_ELECTION, } expected_summary = 'dummy-user {} election "a dummy election" (dummy)'.format( diff --git a/fedora_elections_messages/tests/utils.py b/fedora_elections_messages/tests/utils.py index b0fbc53..08b763d 100644 --- a/fedora_elections_messages/tests/utils.py +++ b/fedora_elections_messages/tests/utils.py @@ -1,3 +1,7 @@ +"""Utilities to do unit testing of message schemas.""" + +DUMMY_CANDIDATE = {"name": "Dummy Candidate", "fas_name": None, "url": None} + DUMMY_ELECTION = { "shortdesc": "a dummy election", "alias": "dummy", @@ -8,6 +12,3 @@ DUMMY_ELECTION = { "embargoed": 0, "voting_type": "range", } - - -DUMMY_CANDIDATE = {"name": "Dummy Candidate", "fas_name": None, "url": None}