From b6d035573bac85eb97a95c8331545c25b695da4e Mon Sep 17 00:00:00 2001 From: Daniel Haselhan Date: Thu, 19 Dec 2024 15:44:45 -0800 Subject: [PATCH] fix: Use compliance period from the Compliance Report * Pass the compliance period from the report instead of reading from the front end data * Add unique constraint on 1 TCI per Category per Period * Change query to return a single TCI and fail otherwise * Update tests --- .../db/models/fuel/TargetCarbonIntensity.py | 13 +++++-- .../tests/fuel_code/test_fuel_code_repo.py | 28 ++++---------- .../test_fuel_supplies_actions_service.py | 38 +++++++++---------- .../test_fuel_supplies_services.py | 24 +++--------- .../web/api/compliance_report/validation.py | 13 ++++++- backend/lcfs/web/api/fuel_code/repo.py | 15 +++----- .../web/api/fuel_supply/actions_service.py | 29 ++++++++++---- backend/lcfs/web/api/fuel_supply/schema.py | 1 - backend/lcfs/web/api/fuel_supply/views.py | 12 ++++-- 9 files changed, 87 insertions(+), 86 deletions(-) diff --git a/backend/lcfs/db/models/fuel/TargetCarbonIntensity.py b/backend/lcfs/db/models/fuel/TargetCarbonIntensity.py index edf7c5ef8..ddc78921a 100644 --- a/backend/lcfs/db/models/fuel/TargetCarbonIntensity.py +++ b/backend/lcfs/db/models/fuel/TargetCarbonIntensity.py @@ -1,13 +1,18 @@ -from sqlalchemy import Column, Integer, Float, ForeignKey, Numeric +from sqlalchemy import Column, Integer, Float, ForeignKey, Numeric, UniqueConstraint from lcfs.db.base import BaseModel, Auditable, EffectiveDates from sqlalchemy.orm import relationship class TargetCarbonIntensity(BaseModel, Auditable, EffectiveDates): __tablename__ = "target_carbon_intensity" - __table_args__ = { - "comment": "Target carbon intensity values for various fuel categories" - } + __table_args__ = ( + UniqueConstraint( + "compliance_period_id", + "fuel_category_id", + name="uq_target_carbon_intensity_compliance_fuel", + ), + {"comment": "Target carbon intensity values for various fuel categories"}, + ) target_carbon_intensity_id = Column(Integer, primary_key=True, autoincrement=True) compliance_period_id = Column( diff --git a/backend/lcfs/tests/fuel_code/test_fuel_code_repo.py b/backend/lcfs/tests/fuel_code/test_fuel_code_repo.py index 99bf2341d..729ac6f12 100644 --- a/backend/lcfs/tests/fuel_code/test_fuel_code_repo.py +++ b/backend/lcfs/tests/fuel_code/test_fuel_code_repo.py @@ -579,17 +579,16 @@ async def test_get_energy_effectiveness_ratio(fuel_code_repo, mock_db): @pytest.mark.anyio -async def test_get_target_carbon_intensities(fuel_code_repo, mock_db): +async def test_get_target_carbon_intensity(fuel_code_repo, mock_db): tci = TargetCarbonIntensity( target_carbon_intensity_id=1, target_carbon_intensity=50.0 ) mock_result = MagicMock() - mock_result.scalars.return_value.all.return_value = [tci] + mock_result.scalar_one.return_value = tci mock_db.execute.return_value = mock_result - result = await fuel_code_repo.get_target_carbon_intensities(1, "2024") - assert len(result) == 1 - assert result[0] == tci + result = await fuel_code_repo.get_target_carbon_intensity(1, "2024") + assert result == tci @pytest.mark.anyio @@ -618,14 +617,8 @@ async def test_get_standardized_fuel_data(fuel_code_repo, mock_db): ), # target carbon intensities MagicMock( - scalars=MagicMock( - return_value=MagicMock( - all=MagicMock( - return_value=[ - TargetCarbonIntensity(target_carbon_intensity=50.0) - ] - ) - ) + scalar_one=MagicMock( + return_value=TargetCarbonIntensity(target_carbon_intensity=50.0) ) ), # additional carbon intensity @@ -656,7 +649,6 @@ async def test_get_standardized_fuel_data_unrecognized(fuel_code_repo, mock_db): mock_fuel_type = FuelType( fuel_type_id=1, fuel_type="UnknownFuel", - default_carbon_intensity=None, unrecognized=True, ) @@ -690,12 +682,8 @@ async def test_get_standardized_fuel_data_unrecognized(fuel_code_repo, mock_db): ) # Target Carbon Intensities tci_result = MagicMock( - scalars=MagicMock( - return_value=MagicMock( - all=MagicMock( - return_value=[TargetCarbonIntensity(target_carbon_intensity=50.0)] - ) - ) + scalar_one=MagicMock( + return_value=TargetCarbonIntensity(target_carbon_intensity=50.0) ) ) # Additional Carbon Intensity diff --git a/backend/lcfs/tests/fuel_supply/test_fuel_supplies_actions_service.py b/backend/lcfs/tests/fuel_supply/test_fuel_supplies_actions_service.py index df6d6eee0..1b16bda9a 100644 --- a/backend/lcfs/tests/fuel_supply/test_fuel_supplies_actions_service.py +++ b/backend/lcfs/tests/fuel_supply/test_fuel_supplies_actions_service.py @@ -21,7 +21,6 @@ FUEL_SUPPLY_EXCLUDE_FIELDS = { "id", "fuel_supply_id", - "compliance_period", "deleted", "group_uuid", "user_type", @@ -129,14 +128,12 @@ def create_sample_fs_data(): fuel_category_id=1, end_use_id=1, fuel_code_id=1, - compliance_period="2024", # Schema-only field - quantity=1000.0, + quantity=1000, units="L", energy_density=35.0, group_uuid=str(uuid4()), version=0, - provisionOfTheActId=123, - exportDate=datetime.now().date(), + provision_of_the_act_id=123, ) @@ -184,7 +181,9 @@ async def test_create_fuel_supply_success( mock_repo.create_fuel_supply.return_value = created_supply # Call the method under test - result = await fuel_supply_action_service.create_fuel_supply(fe_data, user_type) + result = await fuel_supply_action_service.create_fuel_supply( + fe_data, user_type, "2024" + ) # Assign mocked related objects for schema validation result.fuel_type = { @@ -201,7 +200,7 @@ async def test_create_fuel_supply_success( fuel_category_id=fe_data.fuel_category_id, end_use_id=fe_data.end_use_id, fuel_code_id=fe_data.fuel_code_id, - compliance_period=fe_data.compliance_period, + compliance_period="2024", ) mock_repo.create_fuel_supply.assert_awaited_once() # Ensure compliance units were calculated correctly @@ -266,7 +265,9 @@ async def test_update_fuel_supply_success_existing_report( mock_repo.update_fuel_supply.return_value = updated_supply # Call the method under test - result = await fuel_supply_action_service.update_fuel_supply(fe_data, user_type) + result = await fuel_supply_action_service.update_fuel_supply( + fe_data, user_type, "2024" + ) # Assign mocked related objects for schema validation result.fuel_type = { @@ -348,7 +349,9 @@ async def test_update_fuel_supply_create_new_version( mock_repo.create_fuel_supply.return_value = new_supply # Call the method under test - result = await fuel_supply_action_service.update_fuel_supply(fe_data, user_type) + result = await fuel_supply_action_service.update_fuel_supply( + fe_data, user_type, "2024" + ) # Assign mocked related objects for schema validation result.fuel_type = { @@ -489,7 +492,7 @@ async def test_populate_fuel_supply_fields( # Call the method under test populated_supply = await fuel_supply_action_service._populate_fuel_supply_fields( - fuel_supply, fe_data + fuel_supply, fe_data, "2024" ) # Assertions @@ -507,13 +510,13 @@ async def test_populate_fuel_supply_fields( fuel_category_id=fuel_supply.fuel_category_id, end_use_id=fuel_supply.end_use_id, fuel_code_id=fuel_supply.fuel_code_id, - compliance_period=fe_data.compliance_period, + compliance_period="2024", ) @pytest.mark.anyio @pytest.mark.parametrize("case", test_cases) -async def test_compliance_units_calculation( +async def test_create_compliance_units_calculation( case, fuel_supply_action_service, mock_repo, mock_fuel_code_repo ): fe_data = FuelSupplyCreateUpdateSchema( @@ -522,14 +525,12 @@ async def test_compliance_units_calculation( fuel_category_id=2, # Adjusted to match the mock fuel_category end_use_id=1, fuel_code_id=1, - compliance_period="2024", # Schema-only field quantity=case["input"]["quantity"], units=case["input"]["units"], energy_density=case["input"]["energy_density"], group_uuid=str(uuid4()), version=0, - provisionOfTheActId=123, - exportDate=datetime.now().date(), + provision_of_the_act_id=123, ) # Mock standardized fuel data @@ -541,9 +542,6 @@ async def test_compliance_units_calculation( uci=None, ) - # Exclude invalid fields and set related objects - fe_data_dict = fe_data.model_dump(exclude=FUEL_SUPPLY_EXCLUDE_FIELDS) - # Mock the create_fuel_supply method to perform actual calculation async def create_fuel_supply_side_effect(fuel_supply): fuel_supply.fuel_supply_id = 1 @@ -561,7 +559,7 @@ async def create_fuel_supply_side_effect(fuel_supply): # Call the service to create the fuel supply result = await fuel_supply_action_service.create_fuel_supply( - fe_data, UserTypeEnum.SUPPLIER + fe_data, UserTypeEnum.SUPPLIER, "2024" ) # Assign mocked related objects for schema validation @@ -582,7 +580,7 @@ async def create_fuel_supply_side_effect(fuel_supply): fuel_category_id=fe_data.fuel_category_id, end_use_id=fe_data.end_use_id, fuel_code_id=fe_data.fuel_code_id, - compliance_period=fe_data.compliance_period, + compliance_period="2024", ) mock_repo.create_fuel_supply.assert_awaited_once() diff --git a/backend/lcfs/tests/fuel_supply/test_fuel_supplies_services.py b/backend/lcfs/tests/fuel_supply/test_fuel_supplies_services.py index bfe03cfb2..ab4fa7b50 100644 --- a/backend/lcfs/tests/fuel_supply/test_fuel_supplies_services.py +++ b/backend/lcfs/tests/fuel_supply/test_fuel_supplies_services.py @@ -113,7 +113,7 @@ async def test_update_fuel_supply_not_found(fuel_supply_action_service): user_type = UserTypeEnum.SUPPLIER with pytest.raises(HTTPException) as exc_info: - await service.update_fuel_supply(fs_data, user_type) + await service.update_fuel_supply(fs_data, user_type, "2024") assert exc_info.value.status_code == 404 assert exc_info.value.detail == "Fuel supply record not found." @@ -133,7 +133,6 @@ async def test_update_fuel_supply_success(fuel_supply_action_service): end_use_id=1, quantity=1000, units="L", - fuel_type_other=None, ci_of_fuel=10.5, energy_density=30.0, eer=1.0, @@ -226,7 +225,7 @@ async def test_update_fuel_supply_success(fuel_supply_action_service): user_type = UserTypeEnum.SUPPLIER # Call the service method - response = await service.update_fuel_supply(fs_data, user_type) + response = await service.update_fuel_supply(fs_data, user_type, "2024") # Assertions assert isinstance(response, FuelSupplyResponseSchema) @@ -245,7 +244,7 @@ async def test_update_fuel_supply_success(fuel_supply_action_service): fuel_category_id=fs_data.fuel_category_id, end_use_id=fs_data.end_use_id, fuel_code_id=fs_data.fuel_code_id, - compliance_period=fs_data.compliance_period, + compliance_period="2024", ) mock_repo.update_fuel_supply.assert_awaited_once_with(existing_fuel_supply) @@ -263,19 +262,6 @@ async def test_create_fuel_supply(fuel_supply_action_service): fuel_type_other=None, units="L", ) - new_fuel_supply = FuelSupply( - compliance_report_id=1, - fuel_type_id=1, - fuel_category_id=1, - provision_of_the_act_id=1, - quantity=2000, - units="L", - fuel_type_other=None, - group_uuid=str(uuid.uuid4()), - version=0, - user_type=UserTypeEnum.SUPPLIER, - action_type=ActionTypeEnum.CREATE, - ) mock_repo.create_fuel_supply = AsyncMock( return_value=MagicMock( fuel_supply_id=1, @@ -310,7 +296,7 @@ async def test_create_fuel_supply(fuel_supply_action_service): user_type = UserTypeEnum.SUPPLIER - response = await service.create_fuel_supply(fs_data, user_type) + response = await service.create_fuel_supply(fs_data, user_type, "2024") assert isinstance(response, FuelSupplyResponseSchema) mock_repo.create_fuel_supply.assert_awaited_once() @@ -319,7 +305,7 @@ async def test_create_fuel_supply(fuel_supply_action_service): fuel_category_id=fs_data.fuel_category_id, end_use_id=fs_data.end_use_id, fuel_code_id=fs_data.fuel_code_id, - compliance_period=fs_data.compliance_period, + compliance_period="2024", ) diff --git a/backend/lcfs/web/api/compliance_report/validation.py b/backend/lcfs/web/api/compliance_report/validation.py index 4f11e4a1c..91bd315d2 100644 --- a/backend/lcfs/web/api/compliance_report/validation.py +++ b/backend/lcfs/web/api/compliance_report/validation.py @@ -25,10 +25,19 @@ async def validate_organization_access(self, compliance_report_id: int): ) organization_id = compliance_report.organization_id - user_organization_id = self.request.user.organization.organization_id if self.request.user.organization else None + user_organization_id = ( + self.request.user.organization.organization_id + if self.request.user.organization + else None + ) - if not user_has_roles(self.request.user, [RoleEnum.GOVERNMENT]) and organization_id != user_organization_id: + if ( + not user_has_roles(self.request.user, [RoleEnum.GOVERNMENT]) + and organization_id != user_organization_id + ): raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail="User does not have access to this compliance report.", ) + + return compliance_report diff --git a/backend/lcfs/web/api/fuel_code/repo.py b/backend/lcfs/web/api/fuel_code/repo.py index 325c3579b..174ee126b 100644 --- a/backend/lcfs/web/api/fuel_code/repo.py +++ b/backend/lcfs/web/api/fuel_code/repo.py @@ -806,9 +806,9 @@ async def get_energy_effectiveness_ratio( return energy_effectiveness_ratio @repo_handler - async def get_target_carbon_intensities( + async def get_target_carbon_intensity( self, fuel_category_id: int, compliance_period: str - ) -> List[TargetCarbonIntensity]: + ) -> TargetCarbonIntensity: compliance_period_id_subquery = ( select(CompliancePeriod.compliance_period_id) @@ -830,7 +830,7 @@ async def get_target_carbon_intensities( ) result = await self.db.execute(stmt) - return result.scalars().all() + return result.scalar_one() @repo_handler async def get_standardized_fuel_data( @@ -876,15 +876,10 @@ async def get_standardized_fuel_data( eer = energy_effectiveness.ratio if energy_effectiveness else 1.0 # Fetch target carbon intensity (TCI) - target_ci = None - target_carbon_intensities = await self.get_target_carbon_intensities( + target_carbon_intensity = await self.get_target_carbon_intensity( fuel_category_id, compliance_period ) - if target_carbon_intensities: - target_ci = next( - (tci.target_carbon_intensity for tci in target_carbon_intensities), - 0.0, - ) + target_ci = target_carbon_intensity.target_carbon_intensity # Additional Carbon Intensity (UCI) uci = await self.get_additional_carbon_intensity(fuel_type_id, end_use_id) diff --git a/backend/lcfs/web/api/fuel_supply/actions_service.py b/backend/lcfs/web/api/fuel_supply/actions_service.py index 642e97f7b..38efcba22 100644 --- a/backend/lcfs/web/api/fuel_supply/actions_service.py +++ b/backend/lcfs/web/api/fuel_supply/actions_service.py @@ -49,7 +49,10 @@ def __init__( self.fuel_repo = fuel_repo async def _populate_fuel_supply_fields( - self, fuel_supply: FuelSupply, fs_data: FuelSupplyCreateUpdateSchema + self, + fuel_supply: FuelSupply, + fs_data: FuelSupplyCreateUpdateSchema, + compliance_period: str, ) -> FuelSupply: """ Populate additional calculated and referenced fields for a FuelSupply instance. @@ -66,8 +69,8 @@ async def _populate_fuel_supply_fields( fuel_type_id=fuel_supply.fuel_type_id, fuel_category_id=fuel_supply.fuel_category_id, end_use_id=fuel_supply.end_use_id, + compliance_period=compliance_period, fuel_code_id=fuel_supply.fuel_code_id, - compliance_period=fs_data.compliance_period, ) # Set units @@ -105,7 +108,10 @@ async def _populate_fuel_supply_fields( @service_handler async def create_fuel_supply( - self, fs_data: FuelSupplyCreateUpdateSchema, user_type: UserTypeEnum + self, + fs_data: FuelSupplyCreateUpdateSchema, + user_type: UserTypeEnum, + compliance_period: str, ) -> FuelSupplyResponseSchema: """ Create a new fuel supply record. @@ -117,6 +123,7 @@ async def create_fuel_supply( Args: fs_data (FuelSupplyCreateUpdateSchema): The data for the new fuel supply. user_type (UserTypeEnum): The type of user creating the record. + compliance_period (int): The compliance period for the new record. Returns: FuelSupplyResponseSchema: The newly created fuel supply record as a response schema. @@ -132,7 +139,9 @@ async def create_fuel_supply( ) # Populate calculated and referenced fields - fuel_supply = await self._populate_fuel_supply_fields(fuel_supply, fs_data) + fuel_supply = await self._populate_fuel_supply_fields( + fuel_supply, fs_data, compliance_period + ) # Save the populated fuel supply record created_supply = await self.repo.create_fuel_supply(fuel_supply) @@ -140,7 +149,10 @@ async def create_fuel_supply( @service_handler async def update_fuel_supply( - self, fs_data: FuelSupplyCreateUpdateSchema, user_type: UserTypeEnum + self, + fs_data: FuelSupplyCreateUpdateSchema, + user_type: UserTypeEnum, + compliance_period: str, ) -> FuelSupplyResponseSchema: """ Update an existing fuel supply record or create a new version if necessary. @@ -153,6 +165,7 @@ async def update_fuel_supply( Args: fs_data (FuelSupplyCreateUpdateSchema): The data for the fuel supply update. user_type (UserTypeEnum): The type of user performing the update. + compliance_period (str): The compliance period for the new record. Returns: FuelSupplyResponseSchema: The updated or new version of the fuel supply record. @@ -177,7 +190,7 @@ async def update_fuel_supply( # Populate calculated fields existing_fuel_supply = await self._populate_fuel_supply_fields( - existing_fuel_supply, fs_data + existing_fuel_supply, fs_data, compliance_period ) updated_supply = await self.repo.update_fuel_supply(existing_fuel_supply) @@ -204,7 +217,9 @@ async def update_fuel_supply( setattr(fuel_supply, field, value) # Populate calculated fields - fuel_supply = await self._populate_fuel_supply_fields(fuel_supply, fs_data) + fuel_supply = await self._populate_fuel_supply_fields( + fuel_supply, fs_data, compliance_period + ) # Save the new version new_supply = await self.repo.create_fuel_supply(fuel_supply) diff --git a/backend/lcfs/web/api/fuel_supply/schema.py b/backend/lcfs/web/api/fuel_supply/schema.py index 60592dffe..3ca83878c 100644 --- a/backend/lcfs/web/api/fuel_supply/schema.py +++ b/backend/lcfs/web/api/fuel_supply/schema.py @@ -114,7 +114,6 @@ class FuelSupplyCreateUpdateSchema(BaseSchema): fuel_supply_id: Optional[int] = None group_uuid: Optional[str] = None version: Optional[int] = None - compliance_period: Optional[str] = None fuel_type_id: int fuel_category_id: int end_use_id: Optional[int] = None diff --git a/backend/lcfs/web/api/fuel_supply/views.py b/backend/lcfs/web/api/fuel_supply/views.py index 3f5674408..9058d4586 100644 --- a/backend/lcfs/web/api/fuel_supply/views.py +++ b/backend/lcfs/web/api/fuel_supply/views.py @@ -81,7 +81,9 @@ async def save_fuel_supply_row( ): """Endpoint to save single fuel supply row""" compliance_report_id = request_data.compliance_report_id - await report_validate.validate_organization_access(compliance_report_id) + compliance_report = await report_validate.validate_organization_access( + compliance_report_id + ) # Determine user type for record creation current_user_type = request.user.user_type @@ -102,12 +104,16 @@ async def save_fuel_supply_row( if request_data.fuel_supply_id: # Update existing fuel supply row using actions service return await action_service.update_fuel_supply( - request_data, current_user_type + request_data, + current_user_type, + compliance_report.compliance_period.description, ) else: # Create new fuel supply row using actions service return await action_service.create_fuel_supply( - request_data, current_user_type + request_data, + current_user_type, + compliance_report.compliance_period.description, )