config: update with bmo data again after parsing modules (bug 1797102)

src/mots/config.py

@@ -16,7 +16,7 @@
 from ruamel.yaml import YAML
 
 from mots.bmo import get_bmo_data
-from mots.directory import Directory, People
+from mots.directory import Directory, People, Person
 from mots.export import export_to_format
 from mots.module import Module
 from mots.utils import generate_machine_readable_name
@@ -160,6 +160,7 @@ def clean(file_config: FileConfig, write: bool = True):
             if key not in module or not module[key]:
                 continue
             for i, person in enumerate(module[key]):
+                person = Person(**person).serialize()
                 if person["bmo_id"] not in directory.people.by_bmo_id:
                     file_config.config["people"].append(person)
                     module[key][i] = person
@@ -189,6 +190,11 @@ def clean(file_config: FileConfig, write: bool = True):
                     submodule["machine_name"] = generate_machine_readable_name(
                         submodule["name"]
                     )
+    # Update people again from BMO in case new people were added.
+    people = file_config.config["people"]
+    bmo_data = get_bmo_data(people)
+    updated_people = People(people, bmo_data)
+    file_config.config["people"] = updated_people.serialized
 
     file_config.config["modules"].sort(key=lambda x: x["machine_name"])
     if write:

src/mots/directory.py

@@ -5,6 +5,8 @@
 """Directory classes for mots."""
 from __future__ import annotations
 
+from __future__ import annotations
+
 from collections import defaultdict
 from dataclasses import asdict
 from dataclasses import dataclass
@@ -174,25 +176,28 @@ class Person:
     """A class representing a person."""
 
     bmo_id: int
-    name: str
-    nick: str
+    name: str | None = ""
+    nick: str | None = ""
 
     def __hash__(self):
         """Return a unique identifier for this person."""
         return self.bmo_id
 
+    def serialize(self):
+        """Return dictionary representation of Person."""
+        return asdict(self)
+
 
 class People:
     """A people directory searchable by name, email, or BMO ID."""
 
     def __init__(self, people, bmo_data: dict):
         logger.debug(f"Initializing people directory with {len(people)} people...")
 
-        self.people = []
+        self.people = set()
         self.by_bmo_id = {}
 
-        people = list(people)
-        for i, person in enumerate(people):
+        for i, person in enumerate(list(people)):
             logger.debug(f"Adding person {person} to roster...")
 
             bmo_id = person["bmo_id"] = int(person["bmo_id"])
@@ -207,9 +212,11 @@ def __init__(self, people, bmo_data: dict):
                 person["name"] = person.get("name", "")
                 person["nick"] = person.get("nick", "")
 
-            self.people.append(Person(**person))
+            self.people.add(Person(**person))
             self.by_bmo_id[person["bmo_id"]] = i
             logger.debug(f"Person {person} added to position {i}.")
+
+        self.people = list(self.people)
         self.serialized = [asdict(person) for person in self.people]
 
     def refresh_by_bmo_id(self):

tests/conftest.py

@@ -121,3 +121,31 @@ def config():
             },
         ],
     }
+
+
+@pytest.fixture
+def config_with_bmo_ids_only():
+    return {
+        "repo": "test_repo",
+        "created_at": "2021-09-10 12:53:22.383393",
+        "updated_at": "2021-09-10 12:53:22.383393",
+        "people": [],
+        "export": {"format": "rst", "path": "mots.rst"},
+        "modules": [
+            {
+                "machine_name": "domesticated_animals",
+                "exclude_submodule_paths": True,
+                "exclude_module_paths": True,
+                "includes": [
+                    "canines/**/*",
+                    "felines/**/*",
+                    "bovines/**/*",
+                    "birds/**/*",
+                    "pigs/**/*",
+                ],
+                "excludes": ["canines/red_fox"],
+                "owners": [{"bmo_id": 1}],
+                "peers": [{"bmo_id": 1}],
+            }
+        ],
+    }

tests/test_config.py

@@ -4,7 +4,9 @@
 
 """Test config module."""
 
-from mots.config import calculate_hashes, FileConfig
+from unittest import mock
+
+from mots.config import calculate_hashes, FileConfig, clean
 
 
 def test_calculate_hashes(config):
@@ -32,3 +34,20 @@ def test_FileConfig__check_hashes(repo):
         "da39a3ee5e6b4b0d3255bfef95601890afd80709 does not match ghjk",
         "export file is out of date.",
     ]
+
+
+@mock.patch("mots.config.get_bmo_data")
+def test_clean_with_no_nick(get_bmo_data, repo, config_with_bmo_ids_only):
+    """Ensures clean function runs without any errors if nick is missing."""
+    get_bmo_data.return_value = {
+        1: {"bmo_id": 1, "nick": "somenick", "real_name": "Some Name"}
+    }
+    file_config = FileConfig(repo / "mots.yml")
+    file_config.config = config_with_bmo_ids_only
+    file_config.write()
+
+    assert file_config.config["people"] == []
+    clean(file_config)
+    assert file_config.config["people"] == [
+        {"bmo_id": 1, "nick": "somenick", "name": "Some Name"}
+    ]

tests/test_directory.py

@@ -4,8 +4,11 @@
 
 """Tests for directory module."""
 
+import pytest
+
 from mots.directory import Directory, Person, QueryResult
 from mots.module import Module
+
 from mots.config import FileConfig
 
 
@@ -184,3 +187,18 @@ def test_directory__QueryResult_empty_addition():
     empty_result = QueryResult()
     other_empty_result = QueryResult()
     assert not (empty_result + other_empty_result)
+
+
+def test_Person():
+    with pytest.raises(TypeError):
+        Person()
+
+    person = Person(0)
+    assert person.bmo_id == 0
+    assert person.nick == ""
+    assert person.name == ""
+
+    person = Person(0, name="Tester", nick="tester")
+    assert person.bmo_id == 0
+    assert person.name == "Tester"
+    assert person.nick == "tester"