Update Readme: first draft, log error instead of raising exception on failed upload, cleanup tests a bit

README.md

@@ -6,14 +6,24 @@ A utility to generate and upload automatic progress reports for NGI Sweden.
 
 ## How it works
 
-- The script first fetches data from the appropriate NGI source, i.e. statusdb for Stockholm.
+- The script first fetches data from the appropriate NGI source, i.e. statusdb for Stockholm. Only projects with orders with a signed contract wil be fetched
 - The data corresponding to each project will then be saved in a small data file (json, yaml or csv perhaps) on disk.
 - Git will be used to track the directory where these files are kept (between runs of the script).
 - Git status (inside python) will be used to check which projects has changes in their data since the last run and those projects will be selected.
-- For each orderer, fetch all their recent projects from Order Portal. A report will be generated with potentially several projects.
-- Reports are uploaded to each project.
-- We need to make sure the reports are transparent about timestamps when it was last updated - Javascript?
-- If problems to upload to a project?
+- From the projects modified, a list of their associated orderers is created.
+- For each unique orderer in the list, all projects are fetched from Order Portal. These are then filtered on the following criteria
+
+  - All orders which are not in the data fetched from the NGI source are skipped.
+  - All closed orders with close dates within 5 days of the cutoff date (default 30 days) will have their reports hidden.
+  - All closed orders closed more than 5 days before the cutoff date are assumed to have their reports hidden and will be skipped.
+  - All orders which are associated to a project NGI started to process but then aborted will have their reports hidden.
+
+- A report with all active projects is generated for each unique orderer.
+- Reports are uploaded to all orders accepted by NGI which have active projects or are marked to have their reports hidden.
+- As each report is uploaded, the file of the corresponding project is staged for commit in the git repo.
+- After all reports in the current run are uploaded, all staged files are committed in the git repo.
+- The reports have timestamps which indicate when it was last updated
+- If there are problems to upload to an order
   - Report to error log (cron will email this)
   - Do not stage these changes, will make sure that the orderer is re-tried next time.
   - Continue with next project
@@ -26,14 +36,15 @@ Also see diagram below:
 ## Usage
 
 ```bash
-# Generate reports and save in a local git repository (location is given by configuration variable) and commit changes with a timestamp message
+# Generate reports for all orderers and save them in a local directory. Project changes are saved in a local git repository (location is given by configuration variable) and committed with a timestamp message
 daily_read generate all
 
-# Generate report for single orderer,
+# Generate and upload reports for all orderers to order portal. They will not be saved locally
+daily_read generate all --upload
+
+# Generate report for single orderer and save it to local directory
 daily_read generate single --project <OrderID>
 
-# Generate and upload
-daily_read generate all --upload
 ```
 
 To generate and upload reports for a single user(or a list of users), their name(s) can be entered in a text file and provided to the environment variable `DAILY_READ_USERS_LIST_LOCATION`.
@@ -42,6 +53,18 @@ To generate and upload reports for a single user(or a list of users), their name
 
 Configuration is dealt with via environment variables. Simplest way to set it up is to retrieve a `.env` file based on the `.env.example` provided in the repo. Environment variables which are not set have default variables in `daily_read/config.py`.
 
+- `DAILY_READ_ORDER_PORTAL_URL` (str) : Order portal URL
+- `DAILY_READ_ORDER_PORTAL_API_KEY` (str) : Order portal API key
+- `DAILY_READ_REPORTS_LOCATION` (str) : Local disk location to save generated reports
+- `DAILY_READ_DATA_LOCATION` (str) : Local disk location for data git repository
+- `DAILY_READ_LOG_LOCATION` (str) : Local disk location to save log output
+- `DAILY_READ_STHLM_STATUSDB_URL` (str) : NGI STHLM Data source URL
+- `DAILY_READ_STHLM_STATUSDB_USERNAME` (str) : NGI STHLM Data source credentials
+- `DAILY_READ_STHLM_STATUSDB_PASSWORD` (str) : NGI STHLM Data source credentials
+- `DAILY_READ_FETCH_FROM_NGIS` (str) : Flag to turn on data fetching from NGI STHLM
+- `DAILY_READ_SNPSEQ_URL` (str) : NGI STHLM Data source URL
+- `DAILY_READ_USERS_LIST_LOCATION` (str) : Path on local disk to orderer list file (.txt) to send reports to, can be empty. The file would contain a single column of orderer email addresses.
+
 ## Developer note
 
 ### Formatting with Black and Prettier

daily_read/__main__.py

@@ -96,13 +96,18 @@
             # Publish reports
             for status in modified_orders[owner]["projects"].keys():
                 for project in modified_orders[owner]["projects"][status]:
-                    op.upload_report_to_order_portal(report, project, "published")
-                    op.projects_data.stage_data_for_project(project)
+                    uploaded = False
+                    uploaded = op.upload_report_to_order_portal(report, project, "published")
+                    if uploaded:
+                        op.projects_data.stage_data_for_project(project)
             # Hide old reports
             for status in modified_orders[owner]["delete_report_for"].keys():
                 for project in modified_orders[owner]["delete_report_for"][status]:
-                    op.upload_report_to_order_portal("", project, "review")
-                    op.projects_data.stage_data_for_project(project)
+                    uploaded = False
+                    uploaded = op.upload_report_to_order_portal("", project, "review")
+                    if uploaded:
+                        op.projects_data.stage_data_for_project(project)
+            # Commit all uploaded projects
             op.projects_data.commit_staged_data(f"Commit reports updates for {datetime.datetime.now()}")
 
         else:

daily_read/order_portal.py

@@ -161,6 +161,11 @@ def upload_report_to_order_portal(self, report, project, status):
         # TODO: check Encoded to utf-8 to display special characters properly
         response = requests.post(url, headers=self.headers, json=indata)
 
-        assert response.status_code == 200, (response.status_code, response.reason)
-
-        log.info(f"Updated report for order with project id: {project.project_id}")
+        if response.status_code == 200:
+            log.info(f"Updated report for order with project id: {project.project_id}")
+            return True
+        else:
+            log.error(
+                f"Report not uploaded for order with project id: {project.project_id}\nReason: {response.status_code} {response.reason}"
+            )
+            return False

tests/conftest.py

@@ -7,7 +7,7 @@
 
 from datetime import date, timedelta
 
-from daily_read import ngi_data, config
+from daily_read import ngi_data, config, order_portal
 
 dummy_order_open = {
     "orderer": "[email protected]",
@@ -376,30 +376,26 @@ def create_report_path(tmp_path):
     return create_report_path
 
 
-def mocked_requests_get(*args, **kwargs):
-    class MockResponse:
-        def __init__(self, json_data, status_code):
-            self.json_data = json_data
-            self.status_code = status_code
+@pytest.fixture(autouse=True)
+def mocked_requests_get(monkeypatch):
+    """order_portal.OrderPortal._get() mocked to return {'items': [order list]}."""
 
+    class MockResponse:
         def json(self):
-            return self.json_data
-
-    if args[0] == "api/v1/orders":
-        return MockResponse(
-            {
+            return {
                 "items": [
                     order_portal_resp_order_processing,
                     order_portal_resp_order_closed,
                     order_portal_resp_order_processing_mult_reports,
                     order_portal_resp_order_processing_single_report,
                     order_portal_resp_order_processing_to_aborted,
                 ]
-            },
-            200,
-        )
+            }
+
+    def mock_get(*args, **kwargs):
+        return MockResponse()
 
-    return MockResponse(None, 404)
+    monkeypatch.setattr(order_portal.OrderPortal, "_get", mock_get)
 
 
 @pytest.fixture
@@ -414,10 +410,7 @@ def mocked_statusdb_conn_rows():
             "order_year": "2023",
             "project_id": "P123457",
             "project_name": "D.Dummysson_23_03",
-            "proj_dates": {
-                "2023-06-15": ["Samples Received"],
-                "2023-06-28": ["Reception Control finished", "Library QC finished"],
-            },
+            "proj_dates": dummy_order_open["project_dates"],
             "status": "Ongoing",
         },
     )
@@ -430,12 +423,7 @@ def mocked_statusdb_conn_rows():
             "order_year": "2023",
             "project_id": "P123458",
             "project_name": "T.Dummysson_23_04",
-            "proj_dates": {
-                "2023-06-15": ["Samples Received"],
-                "2023-06-28": ["Reception Control finished", "Library QC finished"],
-                "2023-07-28": ["All Samples Sequenced"],
-                "2023-07-29": ["All Raw data Delivered"],
-            },
+            "proj_dates": dummy_order_closed["project_dates"],
             "status": "Closed",
         },
     )

tests/test_daily_report.py

@@ -1,7 +1,6 @@
 import datetime
 import os
 
-from conftest import mocked_requests_get
 from unittest import mock
 
 from daily_read import daily_report, config, ngi_data, order_portal
@@ -19,8 +18,7 @@ def test_write_report_to_out_dir(data_repo_full, mock_project_data_record, creat
     data_master.data = {order_id: mock_project_data_record("open")}
 
     op = order_portal.OrderPortal(config_values, data_master)
-    with mock.patch("daily_read.order_portal.OrderPortal._get", side_effect=mocked_requests_get):
-        op.get_orders(orderer=orderer)
+    op.get_orders(orderer=orderer)
 
     assert op.all_orders[0]["identifier"] == order_id
     modified_orders = op.process_orders(config_values.STATUS_PRIORITY_REV)

tests/test_ngi_data.py

@@ -6,8 +6,6 @@
 
 from daily_read import ngi_data, config
 
-LOGGER = logging.getLogger(__name__)
-
 ####################################################### TESTS #########################################################
 
 

tests/test_order_portal.py

@@ -1,12 +1,37 @@
 import base64
+import logging
 import pytest
 
-from conftest import mocked_requests_get
 from unittest import mock
 
 from daily_read import order_portal, config, ngi_data
 
 
+def test_get_and_process_orders_open_upload_fail(data_repo_full, mock_project_data_record, caplog):
+    """Test getting and processing an open order and upload to Order portal failing"""
+    orderer = "[email protected]"
+    order_id = "NGI123456"
+    config_values = config.Config()
+    with mock.patch("daily_read.statusdb.StatusDBSession"):
+        data_master = ngi_data.ProjectDataMaster(config_values)
+
+    data_master.data = {order_id: mock_project_data_record("open")}
+
+    op = order_portal.OrderPortal(config_values, data_master)
+    op.get_orders(orderer=orderer)
+
+    assert op.all_orders[0]["identifier"] == order_id
+    modified_orders = op.process_orders(config_values.STATUS_PRIORITY_REV)
+    assert modified_orders[orderer]["projects"]["Library QC finished"][0] == data_master.data[order_id]
+    with mock.patch("daily_read.order_portal.requests.post") as mock_post:
+        mock_post.return_value.status_code = 404
+        uploaded = op.upload_report_to_order_portal(
+            "<html>test data</html>", modified_orders[orderer]["projects"]["Library QC finished"][0], "published"
+        )
+        assert not uploaded
+        assert f"Report not uploaded for order with project id: {order_id}\nReason: 404" in caplog.text
+
+
 def test_get_and_process_orders_open_and_upload(data_repo_full, mock_project_data_record):
     """Test getting and processing an open order and uploading its Project progress report and uploading the report to the Order portal"""
     orderer = "[email protected]"
@@ -18,8 +43,7 @@ def test_get_and_process_orders_open_and_upload(data_repo_full, mock_project_dat
     data_master.data = {order_id: mock_project_data_record("open")}
 
     op = order_portal.OrderPortal(config_values, data_master)
-    with mock.patch("daily_read.order_portal.OrderPortal._get", side_effect=mocked_requests_get):
-        op.get_orders(orderer=orderer)
+    op.get_orders(orderer=orderer)
 
     assert op.all_orders[0]["identifier"] == order_id
     modified_orders = op.process_orders(config_values.STATUS_PRIORITY_REV)
@@ -45,7 +69,7 @@ def test_get_and_process_orders_open_and_upload(data_repo_full, mock_project_dat
         )
 
 
-def test_get_and_process_orders_open_with_report_and_upload(data_repo_full, mock_project_data_record):
+def test_get_and_process_orders_open_with_report_and_upload(data_repo_full, mock_project_data_record, caplog):
     """Test getting, processing an open order with an existing Project progress report and uploading the report to the Order portal"""
     orderer = "[email protected]"
     order_id = "NGI123453"
@@ -56,31 +80,33 @@ def test_get_and_process_orders_open_with_report_and_upload(data_repo_full, mock
     data_master.data = {order_id: mock_project_data_record("open_with_report")}
 
     op = order_portal.OrderPortal(config_values, data_master)
-    with mock.patch("daily_read.order_portal.OrderPortal._get", side_effect=mocked_requests_get):
-        op.get_orders(orderer=orderer)
+    op.get_orders(orderer=orderer)
 
     assert op.all_orders[3]["identifier"] == order_id
     modified_orders = op.process_orders(config_values.STATUS_PRIORITY_REV)
     assert modified_orders[orderer]["projects"]["Library QC finished"][0] == data_master.data[order_id]
     with mock.patch("daily_read.order_portal.requests.post") as mock_post:
         mock_post.return_value.status_code = 200
-        op.upload_report_to_order_portal(
-            "<html>test data</html>", modified_orders[orderer]["projects"]["Library QC finished"][0], "published"
-        )
-        url = f"{config_values.ORDER_PORTAL_URL}/api/v1/report/{op.all_orders[3]['reports'][0]['iuid']}"
-        indata = dict(
-            order=order_id,
-            name="Project Progress",
-            status="published",
-            file=dict(
-                data=base64.b64encode("<html>test data</html>".encode()).decode("utf-8"),
-                filename="project_progress.html",
-                content_type="text/html",
-            ),
-        )
-        mock_post.assert_called_once_with(
-            url, headers={"X-OrderPortal-API-key": config_values.ORDER_PORTAL_API_KEY}, json=indata
-        )
+        with caplog.at_level(logging.INFO):
+            uploaded = op.upload_report_to_order_portal(
+                "<html>test data</html>", modified_orders[orderer]["projects"]["Library QC finished"][0], "published"
+            )
+            url = f"{config_values.ORDER_PORTAL_URL}/api/v1/report/{op.all_orders[3]['reports'][0]['iuid']}"
+            indata = dict(
+                order=order_id,
+                name="Project Progress",
+                status="published",
+                file=dict(
+                    data=base64.b64encode("<html>test data</html>".encode()).decode("utf-8"),
+                    filename="project_progress.html",
+                    content_type="text/html",
+                ),
+            )
+            assert uploaded
+            mock_post.assert_called_once_with(
+                url, headers={"X-OrderPortal-API-key": config_values.ORDER_PORTAL_API_KEY}, json=indata
+            )
+            assert f"Updated report for order with project id: {order_id}" in caplog.text
 
 
 def test_get_and_process_orders_open_to_aborted_with_report_and_upload(data_repo_full, mock_project_data_record):
@@ -94,8 +120,7 @@ def test_get_and_process_orders_open_to_aborted_with_report_and_upload(data_repo
     data_master.data = {order_id: mock_project_data_record("open_to_aborted_with_report")}
 
     op = order_portal.OrderPortal(config_values, data_master)
-    with mock.patch("daily_read.order_portal.OrderPortal._get", side_effect=mocked_requests_get):
-        op.get_orders(orderer=orderer)
+    op.get_orders(orderer=orderer)
 
     assert op.all_orders[4]["identifier"] == order_id
     modified_orders = op.process_orders(config_values.STATUS_PRIORITY_REV)
@@ -128,8 +153,7 @@ def test_get_and_process_orders_closed(data_repo_full, mock_project_data_record)
     data_master.data = {order_id: mock_project_data_record("closed")}
 
     op = order_portal.OrderPortal(config_values, data_master)
-    with mock.patch("daily_read.order_portal.OrderPortal._get", side_effect=mocked_requests_get):
-        op.get_orders(orderer=orderer)
+    op.get_orders(orderer=orderer)
 
     assert op.all_orders[1]["identifier"] == order_id
     modified_orders = op.process_orders(config_values.STATUS_PRIORITY_REV)
@@ -147,8 +171,7 @@ def test_get_and_process_orders_mult_reports(data_repo_full, mock_project_data_r
     data_master.data = {order_id: mock_project_data_record("open")}
 
     op = order_portal.OrderPortal(config_values, data_master)
-    with mock.patch("daily_read.order_portal.OrderPortal._get", side_effect=mocked_requests_get):
-        op.get_orders(orderer=orderer)
+    op.get_orders(orderer=orderer)
 
     assert op.all_orders[2]["identifier"] == order_id
     with pytest.raises(