fix(file): write the metadata onto the page the upload created

- store_entity saw the page the upload had just made and kept its empty content
- pass overwrite='replace remote' when the file page did not exist beforehand
- an already existing page keeps whatever policy the caller asked for

Author: LukasGold

src/osw/controller/file/wiki.py

@@ -64,6 +64,22 @@ def assert_upload_success(result: Any, title: str, host: str) -> None:
     )
 
 
+def store_params_for_upload(
+    se_params: Dict[str, Any], page_existed: bool
+) -> Dict[str, Any]:
+    """Picks the overwrite policy for the entity that accompanies an upload
+
+    An upload creates the file page when it was not there yet. store_entity
+    would then see an existing page and, under the default 'keep existing',
+    leave the metadata unwritten. So the entity has to replace what the upload
+    put there. A page that was already there keeps whatever the caller asked
+    for.
+    """
+    if page_existed:
+        return se_params
+    return {**se_params, "overwrite": "replace remote"}
+
+
 class WikiFileController(model.WikiFile, RemoteFileController):
     """File controller for wiki files"""
 
@@ -190,6 +206,7 @@ def put(self, file: IO, **kwargs: Dict[str, Any]):
         # across the two writes, so one of them can be left standing. A file
         # page without metadata is visibly incomplete, while metadata without a
         # file looks like a valid entity until someone tries to download it.
+        page_existed = self.osw.mw_site.pages[f"{self.namespace}:{self.title}"].exists
         try:
             result = self.osw.mw_site.upload(
                 file=file,
@@ -205,7 +222,7 @@ def put(self, file: IO, **kwargs: Dict[str, Any]):
             OSW.StoreEntityParam(
                 entities=[self.cast(model.WikiFile, **wf_params)],
                 namespace=self.namespace,
-                **se_params,
+                **store_params_for_upload(se_params, page_existed),
             )
         )
 

tests/test_wiki_file_upload_errors.py

@@ -15,6 +15,7 @@
     assert_upload_success,
     get_allowed_file_extensions,
     reraise_upload_error,
+    store_params_for_upload,
 )
 
 
@@ -109,6 +110,27 @@ def test_upload_without_success_raises(result):
     assert "wiki.example.org" in str(exc_info.value)
 
 
+def test_a_page_the_upload_created_gets_its_metadata_written():
+    """Otherwise store_entity keeps the empty page the upload just left behind."""
+    assert store_params_for_upload({}, page_existed=False) == {
+        "overwrite": "replace remote"
+    }
+
+
+def test_an_existing_page_keeps_the_callers_overwrite_policy():
+    se_params = {"overwrite": "keep existing", "edit_comment": "hi"}
+
+    assert store_params_for_upload(se_params, page_existed=True) == se_params
+
+
+def test_store_params_are_not_mutated():
+    se_params = {"edit_comment": "hi"}
+
+    store_params_for_upload(se_params, page_existed=False)
+
+    assert se_params == {"edit_comment": "hi"}
+
+
 def test_warned_upload_reports_the_warnings():
     result = {"result": "Warning", "warnings": {"badfilename": "a_png"}}