From 96a7391e886177971c68f048af57364d4495b2ce Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Mon, 9 Jun 2025 11:11:11 -0700 Subject: [PATCH 1/4] * Expect fields in procurements Google sheet based on deployment * Dynamically display fields in detail view --- .../hardware_procurement_detail.html | 25 +++++++++++++++++++ .../data_sources/backends/google_sheets.py | 15 ++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/coldfront/plugins/hardware_procurements/templates/hardware_procurements/hardware_procurement_detail.html b/coldfront/plugins/hardware_procurements/templates/hardware_procurements/hardware_procurement_detail.html index a6e7837096..274b3f6d8d 100644 --- a/coldfront/plugins/hardware_procurements/templates/hardware_procurements/hardware_procurement_detail.html +++ b/coldfront/plugins/hardware_procurements/templates/hardware_procurements/hardware_procurement_detail.html @@ -1,6 +1,7 @@ {% extends "common/base.html" %} {% load static %} {% load common_tags %} +{% load feature_flags %} {% block title %} @@ -141,12 +142,36 @@

Hardware Procurement Information

{% endif %} + {% flag_enabled 'BRC_ONLY' as brc_only %} + {% if brc_only %} JIRA Ticket {{ procurement|get_value_from_dict:"jira_ticket" }} + {% endif %} + {% flag_enabled 'LRC_ONLY' as lrc_only %} + {% if lrc_only %} + + + Project ID + + {{ procurement|get_value_from_dict:"project_id" }} + + + + Requisition ID + + {{ procurement|get_value_from_dict:"requisition_id" }} + + + + PO / PCard + + {{ procurement|get_value_from_dict:"po_pcard" }} + + {% endif %} {% endwith %} diff --git a/coldfront/plugins/hardware_procurements/utils/data_sources/backends/google_sheets.py b/coldfront/plugins/hardware_procurements/utils/data_sources/backends/google_sheets.py index cada248df2..4543c97d44 100644 --- a/coldfront/plugins/hardware_procurements/utils/data_sources/backends/google_sheets.py +++ b/coldfront/plugins/hardware_procurements/utils/data_sources/backends/google_sheets.py @@ -6,6 +6,8 @@ from datetime import date from datetime import datetime +from flags.state import flag_enabled + from ... import HardwareProcurement from .base import BaseDataSourceBackend @@ -57,12 +59,19 @@ def fetch_hardware_procurements(self, user_data=None, status=None): 'hardware_type', 'hardware_specification_details', 'procurement_start_date', - 'jira_ticket', 'order_received_date', 'installed_date', 'expected_retirement_date', ] + if flag_enabled('BRC_ONLY'): + keys.append('jira_ticket') + elif flag_enabled('LRC_ONLY'): + keys.append('buyer') + keys.append('project_id') + keys.append('requisition_id') + keys.append('po_pcard') + # Maintain HardwareProcurement "copy numbers". Refer to the # HardwareProcurement class for more details. num_copies_by_identifier = defaultdict(int) @@ -79,6 +88,10 @@ def fetch_hardware_procurements(self, user_data=None, status=None): cleaned_value = 'Unknown' entry[key] = cleaned_value + if flag_enabled('LRC_ONLY'): + if entry['buyer'] != 'PI': + continue + pi_emails_str = ','.join(sorted(entry['pi_emails'])) hardware_type_str = entry['hardware_type'] initial_inquiry_date_str = ( From df865cd7efa79d746a18058dcb12eb468c217501 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Tue, 10 Jun 2025 10:51:18 -0700 Subject: [PATCH 2/4] * Make GoogleSheetsDataSourceBackend abstract * Create concrete BRC and LRC subclasses of it --- .../data_sources/backends/google_sheets.py | 67 +++++++++++++++---- 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/coldfront/plugins/hardware_procurements/utils/data_sources/backends/google_sheets.py b/coldfront/plugins/hardware_procurements/utils/data_sources/backends/google_sheets.py index 4543c97d44..96859f4638 100644 --- a/coldfront/plugins/hardware_procurements/utils/data_sources/backends/google_sheets.py +++ b/coldfront/plugins/hardware_procurements/utils/data_sources/backends/google_sheets.py @@ -2,18 +2,17 @@ import json import os +from abc import abstractmethod from collections import defaultdict from datetime import date from datetime import datetime -from flags.state import flag_enabled - from ... import HardwareProcurement from .base import BaseDataSourceBackend class GoogleSheetsDataSourceBackend(BaseDataSourceBackend): - """A backend that fetches hardware procurement data from a Google + """An interface that fetches hardware procurement data from a Google Sheet.""" DATE_FORMAT = '%m/%d/%Y' @@ -64,13 +63,8 @@ def fetch_hardware_procurements(self, user_data=None, status=None): 'expected_retirement_date', ] - if flag_enabled('BRC_ONLY'): - keys.append('jira_ticket') - elif flag_enabled('LRC_ONLY'): - keys.append('buyer') - keys.append('project_id') - keys.append('requisition_id') - keys.append('po_pcard') + for key in self._extra_columns: + keys.append(key) # Maintain HardwareProcurement "copy numbers". Refer to the # HardwareProcurement class for more details. @@ -88,9 +82,8 @@ def fetch_hardware_procurements(self, user_data=None, status=None): cleaned_value = 'Unknown' entry[key] = cleaned_value - if flag_enabled('LRC_ONLY'): - if entry['buyer'] != 'PI': - continue + if self._should_exclude_procurement(entry): + continue pi_emails_str = ','.join(sorted(entry['pi_emails'])) hardware_type_str = entry['hardware_type'] @@ -128,7 +121,7 @@ def _clean_sheet_value(self, column_name, value): - expected_retirement_date - initial_inquiry_date - installed_date - - order_received_date + - order_received_data - pi_emails - poc_emails - procurement_start_date @@ -181,6 +174,13 @@ def _clean_sheet_value(self, column_name, value): return value + @property + @abstractmethod + def _extra_columns(self): + """Return a tuple of strs denoting the names of extra columns + specific to the deployment.""" + pass + def _fetch_sheet_data(self): """Open the spreadsheet and specific tab, and return all values strictly after the header row.""" @@ -209,3 +209,42 @@ def _load_config_from_file(self, config_file_path): a dict.""" with open(config_file_path, 'r') as f: return json.load(f) + + @abstractmethod + def _should_exclude_procurement(self, procurement): + """Return whether the given cleaned dict representing a + procurement fetched from the sheet should be excluded.""" + pass + + +class BRCGoogleSheetsDataSourceBackend(GoogleSheetsDataSourceBackend): + """A backend that fetches hardware procurement data from a Google + Sheet, specifically for BRC.""" + + @property + def _extra_columns(self): + return ( + 'jira_ticket', + ) + + def _should_exclude_procurement(self, procurement): + return False + + +class LRCGoogleSheetsDataSourceBackend(GoogleSheetsDataSourceBackend): + """A backend that fetches hardware procurement data from a Google + Sheet, specifically for LRC.""" + + @property + def _extra_columns(self): + return ( + 'buyer', + 'project_id', + 'requisition_id', + 'po_pcard', + ) + + def _should_exclude_procurement(self, procurement): + if procurement['buyer'] != 'PI': + return True + return False From ac1f7f9cfede98d24755eb9e0c3bf0ef97fb992b Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Tue, 10 Jun 2025 10:52:30 -0700 Subject: [PATCH 3/4] Adjust tests for procurements backends given new backend classes --- .../pytest/test_data_sources/conftest.py | 51 ++++--- ...oogle_sheets_data_source_backend_data.tsv} | 0 ..._source_backend_expected_output_data.json} | 0 ...google_sheets_data_source_backend_data.tsv | 7 + ...a_source_backend_expected_output_data.json | 107 ++++++++++++++ .../test_cached_data_source_backend.py | 50 +++---- .../test_google_sheets_data_source_backend.py | 138 +++++++++++++----- .../tests/pytest/test_data_sources/utils.py | 58 ++++++-- 8 files changed, 323 insertions(+), 88 deletions(-) rename coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/{test_google_sheets_data_source_backend_data.tsv => test_brc_google_sheets_data_source_backend_data.tsv} (100%) rename coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/{test_google_sheets_data_source_backend_expected_output_data.json => test_brc_google_sheets_data_source_backend_expected_output_data.json} (100%) create mode 100644 coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_lrc_google_sheets_data_source_backend_data.tsv create mode 100644 coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_lrc_google_sheets_data_source_backend_expected_output_data.json diff --git a/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/conftest.py b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/conftest.py index e7de4b2e40..9b05440a8b 100644 --- a/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/conftest.py +++ b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/conftest.py @@ -1,38 +1,49 @@ -import csv import json -import os import pytest +from .utils import get_resource_absolute_file_path +from .utils import get_tsv_data from .utils import MockUser @pytest.fixture -def expected_hardware_procurements_data(): +def brc_expected_hardware_procurements_data(): """Return a list of dicts representing the expected output of the - backend's fetch method, given the test data file as input, read from - a JSON file.""" - test_dir_path = os.path.dirname(__file__) - data_file_path = os.path.join( - test_dir_path, - 'resources', - 'test_google_sheets_data_source_backend_expected_output_data.json') + backend's fetch method, given the BRC test data file as input, read + from a JSON file.""" + data_file_path = get_resource_absolute_file_path( + 'test_brc_google_sheets_data_source_backend_expected_output_data.json') with open(data_file_path, 'r') as f: return json.load(f) @pytest.fixture -def google_sheet_data(): - """Return a list of lists containing test data, read from a TSV +def brc_google_sheet_data(): + """Return a list of lists containing BRC test data, read from a TSV file, excluding the header.""" - test_dir_path = os.path.dirname(__file__) - data_file_path = os.path.join( - test_dir_path, - 'resources', - 'test_google_sheets_data_source_backend_data.tsv') + data_file_path = get_resource_absolute_file_path( + 'test_brc_google_sheets_data_source_backend_data.tsv') + return get_tsv_data(data_file_path) + + +@pytest.fixture +def lrc_expected_hardware_procurements_data(): + """Return a list of dicts representing the expected output of the + backend's fetch method, given the LRC test data file as input, read + from a JSON file.""" + data_file_path = get_resource_absolute_file_path( + 'test_lrc_google_sheets_data_source_backend_expected_output_data.json') with open(data_file_path, 'r') as f: - reader = csv.reader(f, delimiter='\t') - data = [row for row in reader] - return data[1:] + return json.load(f) + + +@pytest.fixture +def lrc_google_sheet_data(): + """Return a list of lists containing LRC test data, read from a TSV + file, excluding the header.""" + data_file_path = get_resource_absolute_file_path( + 'test_lrc_google_sheets_data_source_backend_data.tsv') + return get_tsv_data(data_file_path) @pytest.fixture diff --git a/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_google_sheets_data_source_backend_data.tsv b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_brc_google_sheets_data_source_backend_data.tsv similarity index 100% rename from coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_google_sheets_data_source_backend_data.tsv rename to coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_brc_google_sheets_data_source_backend_data.tsv diff --git a/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_google_sheets_data_source_backend_expected_output_data.json b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_brc_google_sheets_data_source_backend_expected_output_data.json similarity index 100% rename from coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_google_sheets_data_source_backend_expected_output_data.json rename to coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_brc_google_sheets_data_source_backend_expected_output_data.json diff --git a/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_lrc_google_sheets_data_source_backend_data.tsv b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_lrc_google_sheets_data_source_backend_data.tsv new file mode 100644 index 0000000000..6578e56a3d --- /dev/null +++ b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_lrc_google_sheets_data_source_backend_data.tsv @@ -0,0 +1,7 @@ +Status Initial Inquiry Date PI Name (first last) PI Email POC Name (optional) POC Email (optional) Hardware Type Hardware Specification Details Procurement Start Date Project ID Requisition ID PO/PCard Order Received Date Installed Date Expected Retirement Date Buyer +Active 01/01/1970 PI One pi1@email.com POC One poc1@email.com CPU CPU Details 1 01/02/1970 000000-001 00001 00001 01/03/1970 PI +Complete 01/01/1970 PI Two pi2@email.com Storage Storage Details 1 01/02/1970 000000-002 00002 00002 01/03/1970 01/04/1970 01/04/1975 PI +Active 01/01/1970 PI One pi1@email.com POC Two poc2@email.com CPU CPU Details 2 000000-003 00003 00003 PI +Inactive 01/01/1970 PI One, PI Two pi1@email.com, pi2@email.com GPU GPU Details 1 PI +Retired 01/01/1970 PI Two pi2@email.com POC One, POC Two poc1@email.com, poc2@email.com Storage Storage Details 2 01/02/1970 000000-004 00004 00004 01/03/1970 01/04/1970 01/04/1972 PI +Retired 01/01/1970 Non PI One non_pi1@email.com Storage Storage Details 2 01/02/1970 000000-005 00005 00005 01/03/1970 01/04/1970 01/04/1972 Non-PI diff --git a/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_lrc_google_sheets_data_source_backend_expected_output_data.json b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_lrc_google_sheets_data_source_backend_expected_output_data.json new file mode 100644 index 0000000000..69f793de67 --- /dev/null +++ b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/resources/test_lrc_google_sheets_data_source_backend_expected_output_data.json @@ -0,0 +1,107 @@ +[ + { + "id": "9b8aaee5", + "data": { + "status": "Pending", + "initial_inquiry_date": "1970-01-01", + "pi_names": "PI One", + "pi_emails": ["pi1@email.com"], + "poc_names": "POC One", + "poc_emails": ["poc1@email.com"], + "hardware_type": "CPU", + "hardware_specification_details": "CPU Details 1", + "procurement_start_date": "1970-01-02", + "project_id": "000000-001", + "requisition_id": "00001", + "po_pcard": "00001", + "order_received_date": "1970-01-03", + "installed_date": null, + "expected_retirement_date": null, + "buyer": "PI" + } + }, + { + "id": "a4eecf6e", + "data": { + "status": "Complete", + "initial_inquiry_date": "1970-01-01", + "pi_names": "PI Two", + "pi_emails": ["pi2@email.com"], + "poc_names": "", + "poc_emails": [], + "hardware_type": "Storage", + "hardware_specification_details": "Storage Details 1", + "procurement_start_date": "1970-01-02", + "project_id": "000000-002", + "requisition_id": "00002", + "po_pcard": "00002", + "order_received_date": "1970-01-03", + "installed_date": "1970-01-04", + "expected_retirement_date": "1975-01-04", + "buyer": "PI" + } + }, + { + "id": "7b79f48f", + "data": { + "status": "Pending", + "initial_inquiry_date": "1970-01-01", + "pi_names": "PI One", + "pi_emails": ["pi1@email.com"], + "poc_names": "POC Two", + "poc_emails": ["poc2@email.com"], + "hardware_type": "CPU", + "hardware_specification_details": "CPU Details 2", + "procurement_start_date": null, + "project_id": "000000-003", + "requisition_id": "00003", + "po_pcard": "00003", + "order_received_date": null, + "installed_date": null, + "expected_retirement_date": null, + "buyer": "PI" + } + }, + { + "id": "160d5698", + "data": { + "status": "Inactive", + "initial_inquiry_date": "1970-01-01", + "pi_names": "PI One, PI Two", + "pi_emails": ["pi1@email.com", "pi2@email.com"], + "poc_names": "", + "poc_emails": [], + "hardware_type": "GPU", + "hardware_specification_details": "GPU Details 1", + "procurement_start_date": null, + "project_id": "", + "requisition_id": "", + "po_pcard": "", + "order_received_date": null, + "installed_date": null, + "expected_retirement_date": null, + "buyer": "PI" + } + }, + { + "id": "cc32e9da", + "data": { + "status": "Retired", + "initial_inquiry_date": "1970-01-01", + "pi_names": "PI Two", + "pi_emails": ["pi2@email.com"], + "poc_names": "POC One, POC Two", + "poc_emails": ["poc1@email.com", "poc2@email.com"], + "hardware_type": "Storage", + "hardware_specification_details": "Storage Details 2", + "procurement_start_date": "1970-01-02", + "project_id": "000000-004", + "requisition_id": "00004", + "po_pcard": "00004", + "order_received_date": "1970-01-03", + "installed_date": "1970-01-04", + "expected_retirement_date": "1972-01-04", + "buyer": "PI" + } + } +] \ No newline at end of file diff --git a/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/test_cached_data_source_backend.py b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/test_cached_data_source_backend.py index 082898e36c..81eba5fa68 100644 --- a/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/test_cached_data_source_backend.py +++ b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/test_cached_data_source_backend.py @@ -2,21 +2,21 @@ from unittest.mock import patch -from .conftest import expected_hardware_procurements_data -from .conftest import google_sheet_data +from .conftest import brc_expected_hardware_procurements_data +from .conftest import brc_google_sheet_data from .conftest import MockUser from .conftest import mock_look_up_user_by_email from .utils import assert_procurement_expected from .utils import CACHE_KEY from .utils import CACHE_MODULE -from .utils import GOOGLE_SHEET_COLUMNS +from .utils import BRC_GOOGLE_SHEET_COLUMNS from .utils import LOOK_UP_FUNC_MODULE from .utils import MockCache from ....utils.data_sources.backends.cached import CachedDataSourceBackend from ....utils.data_sources.backends.cached import HardwareProcurementsCacheManager -from ....utils.data_sources.backends.google_sheets import GoogleSheetsDataSourceBackend +from ....utils.data_sources.backends.google_sheets import BRCGoogleSheetsDataSourceBackend as GoogleSheetsDataSourceBackend @pytest.fixture @@ -27,7 +27,7 @@ def cached_data_source_kwargs(): 'credentials_file_path': '', 'sheet_id': '', 'sheet_tab': '', - 'sheet_columns': GOOGLE_SHEET_COLUMNS, + 'sheet_columns': BRC_GOOGLE_SHEET_COLUMNS, 'header_row_index': 1, } return { @@ -39,7 +39,7 @@ def cached_data_source_kwargs(): @pytest.fixture def cached_google_sheets_backend(mock_cache, mock_look_up_user_by_email, - google_sheet_data, cached_data_source_kwargs): + brc_google_sheet_data, cached_data_source_kwargs): """Return a CachedDataSourceBackend that caches data from a GoogleSheetsDataSourceBackend (with the given columns and data) under the given cache key. Mock the cache and a the function for @@ -47,7 +47,7 @@ def cached_google_sheets_backend(mock_cache, mock_look_up_user_by_email, """ with patch.object( GoogleSheetsDataSourceBackend, '_fetch_sheet_data', - return_value=google_sheet_data): + return_value=brc_google_sheet_data): with patch('os.path.isfile', return_value=True) as mock_isfile: with patch(CACHE_MODULE, mock_cache): with patch(LOOK_UP_FUNC_MODULE, mock_look_up_user_by_email): @@ -57,11 +57,11 @@ def cached_google_sheets_backend(mock_cache, mock_look_up_user_by_email, @pytest.fixture -def expected_hardware_procurements_data_by_id(expected_hardware_procurements_data): +def brc_expected_hardware_procurements_data_by_id(brc_expected_hardware_procurements_data): """Return a dict mapping procurement IDs to the procurements - returned by the `expected_hardware_procurements_data` fixture.""" + returned by the `brc_expected_hardware_procurements_data` fixture.""" procurement_data_by_id = {} - for procurement_dict in expected_hardware_procurements_data: + for procurement_dict in brc_expected_hardware_procurements_data: _id = procurement_dict['id'] procurement_data_by_id[_id] = procurement_dict return procurement_data_by_id @@ -81,7 +81,7 @@ def setup_method(self): self._look_up_func_module = LOOK_UP_FUNC_MODULE def _assert_fetch_output(self, mock_cache, mock_look_up_user_by_email, - backend, expected_hardware_procurements_data_by_id, + backend, brc_expected_hardware_procurements_data_by_id, expected_ids, status=None, user_data=None): """Assert that the given backend's `fetch_hardware_procurements` method returns entries with the expected IDs from the given @@ -98,7 +98,7 @@ def _assert_fetch_output(self, mock_cache, mock_look_up_user_by_email, assert expected_index < num_expected _id = hardware_procurement.get_id() expected_hardware_procurement = \ - expected_hardware_procurements_data_by_id[_id] + brc_expected_hardware_procurements_data_by_id[_id] assert_procurement_expected( hardware_procurement, expected_hardware_procurement) expected_index += 1 @@ -124,17 +124,17 @@ def test_init_instantiates_cache_manager(self, @pytest.mark.component def test_init_populates_cache(self, mock_cache, cached_google_sheets_backend, - expected_hardware_procurements_data_by_id): + brc_expected_hardware_procurements_data_by_id): cache_manager = cached_google_sheets_backend._cache_manager with patch(self._cache_module, mock_cache): for hardware_procurement in cache_manager.get_cached_procurements(): _id = hardware_procurement.get_id() - assert _id in expected_hardware_procurements_data_by_id + assert _id in brc_expected_hardware_procurements_data_by_id expected_hardware_procurement = \ - expected_hardware_procurements_data_by_id.pop(_id) + brc_expected_hardware_procurements_data_by_id.pop(_id) assert_procurement_expected( hardware_procurement, expected_hardware_procurement) - assert not expected_hardware_procurements_data_by_id + assert not brc_expected_hardware_procurements_data_by_id @pytest.mark.component def test_init_skips_cache_populate_if_not_needed(self, mock_cache, @@ -176,14 +176,14 @@ def test_clear_cache(self, mock_cache, cached_google_sheets_backend): def test_fetch_hardware_procurements_multiple_filters(self, mock_cache, mock_look_up_user_by_email, cached_google_sheets_backend, - expected_hardware_procurements_data_by_id, + brc_expected_hardware_procurements_data_by_id, status, user_data, expected_ids): self._assert_fetch_output( mock_cache, mock_look_up_user_by_email, cached_google_sheets_backend, - expected_hardware_procurements_data_by_id, + brc_expected_hardware_procurements_data_by_id, expected_ids, status=status, user_data=user_data) @@ -192,13 +192,13 @@ def test_fetch_hardware_procurements_multiple_filters(self, mock_cache, def test_fetch_hardware_procurements_no_filters(self, mock_cache, mock_look_up_user_by_email, cached_google_sheets_backend, - expected_hardware_procurements_data_by_id): - expected_ids = expected_hardware_procurements_data_by_id.keys() + brc_expected_hardware_procurements_data_by_id): + expected_ids = brc_expected_hardware_procurements_data_by_id.keys() self._assert_fetch_output( mock_cache, mock_look_up_user_by_email, cached_google_sheets_backend, - expected_hardware_procurements_data_by_id, + brc_expected_hardware_procurements_data_by_id, expected_ids) @pytest.mark.component @@ -214,13 +214,13 @@ def test_fetch_hardware_procurements_no_filters(self, mock_cache, def test_fetch_hardware_procurements_status_filter(self, mock_cache, mock_look_up_user_by_email, cached_google_sheets_backend, - expected_hardware_procurements_data_by_id, + brc_expected_hardware_procurements_data_by_id, status, expected_ids): self._assert_fetch_output( mock_cache, mock_look_up_user_by_email, cached_google_sheets_backend, - expected_hardware_procurements_data_by_id, + brc_expected_hardware_procurements_data_by_id, expected_ids, status=status) @@ -249,13 +249,13 @@ def test_fetch_hardware_procurements_status_filter(self, mock_cache, def test_fetch_hardware_procurements_user_data_filter(self, mock_cache, mock_look_up_user_by_email, cached_google_sheets_backend, - expected_hardware_procurements_data_by_id, + brc_expected_hardware_procurements_data_by_id, user_data, expected_ids): self._assert_fetch_output( mock_cache, mock_look_up_user_by_email, cached_google_sheets_backend, - expected_hardware_procurements_data_by_id, + brc_expected_hardware_procurements_data_by_id, expected_ids, user_data=user_data) diff --git a/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/test_google_sheets_data_source_backend.py b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/test_google_sheets_data_source_backend.py index 45724bee91..df2622d018 100644 --- a/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/test_google_sheets_data_source_backend.py +++ b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/test_google_sheets_data_source_backend.py @@ -1,38 +1,28 @@ import pytest +from abc import ABC from datetime import date from unittest.mock import MagicMock from unittest.mock import mock_open from unittest.mock import patch -from .conftest import expected_hardware_procurements_data -from .conftest import google_sheet_data +from .conftest import brc_expected_hardware_procurements_data +from .conftest import lrc_expected_hardware_procurements_data +from .conftest import brc_google_sheet_data +from .conftest import lrc_google_sheet_data from .utils import assert_procurement_expected -from .utils import GOOGLE_SHEET_COLUMNS +from .utils import BRC_GOOGLE_SHEET_COLUMNS +from .utils import LRC_GOOGLE_SHEET_COLUMNS -from ....utils import HardwareProcurement -from ....utils.data_sources.backends.google_sheets import GoogleSheetsDataSourceBackend +from ....utils.data_sources.backends.google_sheets import BRCGoogleSheetsDataSourceBackend +from ....utils.data_sources.backends.google_sheets import LRCGoogleSheetsDataSourceBackend -@pytest.fixture -def backend_from_google_sheet_columns(): - """Return a GoogleSheetsDataSourceBackend based on the columns - defined in GOOGLE_SHEET_COLUMNS, and with a header row index of - 1.""" - backend = GoogleSheetsDataSourceBackend( - credentials_file_path='', - sheet_id='', - sheet_tab='', - sheet_columns=GOOGLE_SHEET_COLUMNS, - header_row_index=1) - return backend - - -@pytest.mark.component -class TestGoogleSheetsDataSourceBackendComponent(object): - """Component tests for GoogleSheetsDataSourceBackend.""" +class GoogleSheetsDataSourceBackendComponentTestBase(ABC): + """Abstract component tests for subclasses of + GoogleSheetsDataSourceBackend.""" def _assert_fetch_output(self, input_data, backend, expected_output_data, expected_ids=None, status=None, user_data=None): @@ -156,6 +146,62 @@ def test_fetch_hardware_procurements_user_data_filter(self, user_data=user_data) +@pytest.mark.component +class TestBRCGoogleSheetsDataSourceBackendComponent( + GoogleSheetsDataSourceBackendComponentTestBase): + """Component tests for BRCGoogleSheetsDataSourceBackend.""" + + @pytest.fixture + def backend_from_google_sheet_columns(self): + """Return a BRCGoogleSheetsDataSourceBackend based on the columns + defined in BRC_GOOGLE_SHEET_COLUMNS, and with a header row index + of 1.""" + backend = BRCGoogleSheetsDataSourceBackend( + credentials_file_path='', + sheet_id='', + sheet_tab='', + sheet_columns=BRC_GOOGLE_SHEET_COLUMNS, + header_row_index=1) + return backend + + @pytest.fixture + def expected_hardware_procurements_data(self, + brc_expected_hardware_procurements_data): + return brc_expected_hardware_procurements_data + + @pytest.fixture + def google_sheet_data(self, brc_google_sheet_data): + return brc_google_sheet_data + + +@pytest.mark.component +class TestLRCGoogleSheetsDataSourceBackendComponent( + GoogleSheetsDataSourceBackendComponentTestBase): + """Component tests for LRCGoogleSheetsDataSourceBackend.""" + + @pytest.fixture + def backend_from_google_sheet_columns(self): + """Return an LRCGoogleSheetsDataSourceBackend based on the + columns defined in LRC_GOOGLE_SHEET_COLUMNS, and with a header + row index of 1.""" + backend = LRCGoogleSheetsDataSourceBackend( + credentials_file_path='', + sheet_id='', + sheet_tab='', + sheet_columns=LRC_GOOGLE_SHEET_COLUMNS, + header_row_index=1) + return backend + + @pytest.fixture + def expected_hardware_procurements_data(self, + lrc_expected_hardware_procurements_data): + return lrc_expected_hardware_procurements_data + + @pytest.fixture + def google_sheet_data(self, lrc_google_sheet_data): + return lrc_google_sheet_data + + @pytest.mark.unit class TestGoogleSheetsDataSourceBackendUnit(object): """Unit tests for GoogleSheetsDataSourceBackend.""" @@ -179,7 +225,7 @@ class TestGoogleSheetsDataSourceBackendUnit(object): def test_clean_sheet_value_dates(self, column_name, value, expected_cleaned_value): kwargs = self._get_backend_kwargs() - backend = GoogleSheetsDataSourceBackend(**kwargs) + backend = BRCGoogleSheetsDataSourceBackend(**kwargs) cleaned_value = backend._clean_sheet_value(column_name, value) assert cleaned_value == expected_cleaned_value @@ -202,7 +248,7 @@ def test_clean_sheet_value_dates(self, column_name, value, def test_clean_sheet_value_emails(self, column_name, value, expected_cleaned_value): kwargs = self._get_backend_kwargs() - backend = GoogleSheetsDataSourceBackend(**kwargs) + backend = BRCGoogleSheetsDataSourceBackend(**kwargs) cleaned_value = backend._clean_sheet_value(column_name, value) assert cleaned_value == expected_cleaned_value @@ -225,18 +271,18 @@ def test_clean_sheet_value_emails(self, column_name, value, def test_clean_sheet_value_status(self, column_name, value, expected_cleaned_value): kwargs = self._get_backend_kwargs() - backend = GoogleSheetsDataSourceBackend(**kwargs) + backend = BRCGoogleSheetsDataSourceBackend(**kwargs) cleaned_value = backend._clean_sheet_value(column_name, value) assert cleaned_value == expected_cleaned_value def test_clean_sheet_value_status_unexpected(self): kwargs = self._get_backend_kwargs() - backend = GoogleSheetsDataSourceBackend(**kwargs) + backend = BRCGoogleSheetsDataSourceBackend(**kwargs) with pytest.raises(ValueError, match='Unexpected status') as exc_info: backend._clean_sheet_value('status', 'unknown') def test_fetch_sheet_data_file_not_found(self): - backend = GoogleSheetsDataSourceBackend( + backend = BRCGoogleSheetsDataSourceBackend( credentials_file_path='nonexistent.json', sheet_id='mock_sheet_id', sheet_tab='mock_tab', @@ -262,7 +308,7 @@ def test_fetch_sheet_data_returned_values(self): credentials_file_path = 'mock_credentials.json' sheet_id = 'mock_sheet_id' sheet_tab = 'mock_tab' - backend = GoogleSheetsDataSourceBackend( + backend = BRCGoogleSheetsDataSourceBackend( credentials_file_path=credentials_file_path, sheet_id=sheet_id, sheet_tab=sheet_tab, @@ -298,7 +344,7 @@ def test_fetch_sheet_data_returned_values(self): ) def test_gsheet_column_to_index(self, column_str, expected_index): kwargs = self._get_backend_kwargs() - backend = GoogleSheetsDataSourceBackend(**kwargs) + backend = BRCGoogleSheetsDataSourceBackend(**kwargs) index = backend._gsheet_column_to_index(column_str) assert index == expected_index @@ -314,7 +360,7 @@ def test_init_from_file_sets_attributes(self): with patch('builtins.open', mock_open(read_data='')) as mock_file: with patch('json.load', return_value=mock_config) as mock_json_load: - backend = GoogleSheetsDataSourceBackend( + backend = BRCGoogleSheetsDataSourceBackend( config_file_path=mock_config_file_path) assert ( backend._credentials_file_path == @@ -343,7 +389,7 @@ def test_init_kwarg_missing(self, kwarg): kwargs = self._get_backend_kwargs() del kwargs[kwarg] with pytest.raises(KeyError): - GoogleSheetsDataSourceBackend(**kwargs) + BRCGoogleSheetsDataSourceBackend(**kwargs) @pytest.mark.parametrize( ['kwarg', 'value'], @@ -359,17 +405,43 @@ def test_init_kwarg_type_unexpected(self, kwarg, value): kwargs = self._get_backend_kwargs() kwargs[kwarg] = value with pytest.raises(AssertionError): - GoogleSheetsDataSourceBackend(**kwargs) + BRCGoogleSheetsDataSourceBackend(**kwargs) def test_init_sets_attributes(self): kwargs = self._get_backend_kwargs() - backend = GoogleSheetsDataSourceBackend(**kwargs) + backend = BRCGoogleSheetsDataSourceBackend(**kwargs) assert backend._credentials_file_path == kwargs['credentials_file_path'] assert backend._sheet_id == kwargs['sheet_id'] assert backend._sheet_tab == kwargs['sheet_tab'] assert backend._sheet_columns == kwargs['sheet_columns'] assert backend._header_row_index == kwargs['header_row_index'] + @pytest.mark.parametrize( + [ + 'procurement_data', + 'expected_exclusion_brc', + 'expected_exclusion_lrc' + ], + [ + ({'buyer': 'PI'}, False, False), + ({'buyer': 'Not PI'}, False, True), + ] + ) + def test_should_exclude_procurement_brc(self, procurement_data, + expected_exclusion_brc, + expected_exclusion_lrc): + kwargs = self._get_backend_kwargs() + + brc_backend = BRCGoogleSheetsDataSourceBackend(**kwargs) + assert ( + brc_backend._should_exclude_procurement(procurement_data) == + expected_exclusion_brc) + + lrc_backend = LRCGoogleSheetsDataSourceBackend(**kwargs) + assert ( + lrc_backend._should_exclude_procurement(procurement_data) == + expected_exclusion_lrc) + @staticmethod def _get_backend_kwargs(credentials_file_path='', sheet_id='', sheet_tab='', diff --git a/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/utils.py b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/utils.py index bd66537064..c46c88741b 100644 --- a/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/utils.py +++ b/coldfront/plugins/hardware_procurements/tests/pytest/test_data_sources/utils.py @@ -1,16 +1,11 @@ -from datetime import datetime - +import csv +import os -# The cache key to be used. -CACHE_KEY = 'cache_key' +from datetime import datetime -# The module path to the cache module to be mocked. -CACHE_MODULE = ( - 'coldfront.plugins.hardware_procurements.utils.data_sources.backends.' - 'cached.cache') -# A dict of columns matching those in the test data file. -GOOGLE_SHEET_COLUMNS = { +# A dict of columns matching those in the BRC test data file. +BRC_GOOGLE_SHEET_COLUMNS = { 'status_col': 'A', 'initial_inquiry_date_col': 'B', 'pi_names_col': 'C', @@ -26,11 +21,39 @@ 'expected_retirement_date_col': 'M', } +# The cache key to be used. +CACHE_KEY = 'cache_key' + +# The module path to the cache module to be mocked. +CACHE_MODULE = ( + 'coldfront.plugins.hardware_procurements.utils.data_sources.backends.' + 'cached.cache') + # The module path to the function for looking up users to be mocked. LOOK_UP_FUNC_MODULE = ( 'coldfront.plugins.hardware_procurements.utils.data_sources.backends.' 'cached.look_up_user_by_email') +# A dict of columns matching those in the LRC test data file. +LRC_GOOGLE_SHEET_COLUMNS = { + 'status_col': 'A', + 'initial_inquiry_date_col': 'B', + 'pi_names_col': 'C', + 'pi_emails_col': 'D', + 'poc_names_col': 'E', + 'poc_emails_col': 'F', + 'hardware_type_col': 'G', + 'hardware_specification_details_col': 'H', + 'procurement_start_date_col': 'I', + 'project_id_col': 'J', + 'requisition_id_col': 'K', + 'po_pcard_col': 'L', + 'order_received_date_col': 'M', + 'installed_date_col': 'N', + 'expected_retirement_date_col': 'O', + 'buyer_col': 'P', +} + class MockCache(object): """A mock for Django's cache.""" @@ -79,3 +102,18 @@ def assert_procurement_expected(actual, expected): if k.endswith('_date') and v is not None: v = datetime.strptime(v, '%Y-%m-%d').date() assert v == actual_data[k] + + +def get_tsv_data(file_path): + """Return a list of lists containing data from a TSV file, excluding + the header.""" + with open(file_path, 'r') as f: + reader = csv.reader(f, delimiter='\t') + data = [row for row in reader] + return data[1:] + + +def get_resource_absolute_file_path(resource_file_name): + """Return the absolute file path of a resource file.""" + test_dir_path = os.path.dirname(__file__) + return os.path.join(test_dir_path, 'resources', resource_file_name) From 8a1110dc01a38437fb7168f73b886e3d51a5c9a4 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Tue, 10 Jun 2025 11:00:56 -0700 Subject: [PATCH 4/4] Update YML config given changed backends --- bootstrap/ansible/main.copyme | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/bootstrap/ansible/main.copyme b/bootstrap/ansible/main.copyme index 002020c076..59a0c77401 100644 --- a/bootstrap/ansible/main.copyme +++ b/bootstrap/ansible/main.copyme @@ -166,9 +166,10 @@ plugin_hardware_procurements_enabled: false # A mapping from short names of data sources to associated module paths. plugin_hardware_procurements_data_source_modules_by_short_name: + brc_google_sheets: 'coldfront.plugins.hardware_procurements.utils.data_sources.backends.google_sheets.BRCGoogleSheetsDataSourceBackend' cached: 'coldfront.plugins.hardware_procurements.utils.data_sources.backends.cached.CachedDataSourceBackend' dummy: 'coldfront.plugins.hardware_procurements.utils.data_sources.backends.dummy.DummyDataSourceBackend' - google_sheets: 'coldfront.plugins.hardware_procurements.utils.data_sources.backends.google_sheets.GoogleSheetsDataSourceBackend' + lrc_google_sheets: 'coldfront.plugins.hardware_procurements.utils.data_sources.backends.google_sheets.LRCGoogleSheetsDataSourceBackend' # # TODO: Uncomment only the section relevant to the specified data source. @@ -177,13 +178,13 @@ plugin_hardware_procurements_data_source_modules_by_short_name: # data_source: 'cached' # data_source_options: # cache_key: 'hardware_procurements_data' -# cached_data_source: 'google_sheets' +# cached_data_source: 'brc_google_sheets' # cached_data_source_options: # config_file_path: '/path/to/data-source-options-file.json' -# # Settings: 'google_sheets' backend. +# # Settings: 'brc_google_sheets' backend. # plugin_hardware_procurements: -# data_source: 'google_sheets' +# data_source: 'brc_google_sheets' # data_source_options: # config_file_path: '/path/to/data-source-options-file.json' @@ -192,6 +193,12 @@ plugin_hardware_procurements_data_source_modules_by_short_name: # data_source: 'dummy' # data_source_options: +# # Settings: 'lrc_google_sheets' backend. +# plugin_hardware_procurements: +# data_source: 'lrc_google_sheets' +# data_source_options: +# config_file_path: '/path/to/data-source-options-file.json' + #------------------------------------------------------------------------------ # User-facing strings #------------------------------------------------------------------------------