Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Filter between legacy and not legacy fuel types #1599

Merged
merged 3 commits into from
Jan 6, 2025

Conversation

dhaselhan
Copy link
Collaborator

  • Pass compliance period for get other use table options
  • Add filter on compliance period < 2024 to enable the is_legacy filter.
  • Add new is_legacy to FuelTypes and default to false

@dhaselhan dhaselhan requested a review from AlexZorkin January 3, 2025 21:24
Copy link

github-actions bot commented Jan 3, 2025

Frontend Test Results

  1 files  ±0  121 suites  ±0   41s ⏱️ -2s
419 tests ±0  399 ✅ ±0  20 💤 ±0  0 ❌ ±0 
421 runs  ±0  401 ✅ ±0  20 💤 ±0  0 ❌ ±0 

Results for commit d641a93. ± Comparison against base commit 23eec01.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jan 3, 2025

Backend Test Results

503 tests  ±0   503 ✅ ±0   2m 3s ⏱️ +18s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit d641a93. ± Comparison against base commit 23eec01.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@AlexZorkin AlexZorkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, just one minor suggestion. Thannks.

"""Get all table options"""
fuel_categories = await self.fuel_code_repo.get_fuel_categories()
fuel_types = await self.fuel_code_repo.get_formatted_fuel_types()
fuel_types = await self.fuel_code_repo.get_formatted_fuel_types(
include_legacy=compliance_period < "2024"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's create a constant for "2024" and call it LEGISLATION_TRANSITION_YEAR or something like that. Add a comment to the constant explaining what its for and then use this throughout the backend wherever we need to differentiate on this cutoff.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@@ -164,6 +164,10 @@ async def get_fuel_export_table_options(self, compliancePeriod: str):
)
)

include_legacy = compliance_period < "2024"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use constant as suggested above, and do the same for any other uses of the year.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@dhaselhan dhaselhan force-pushed the feat/daniel-add-is-legacy-1550 branch 2 times, most recently from 57021b8 to fe52b18 Compare January 6, 2025 18:09
@dhaselhan dhaselhan requested a review from AlexZorkin January 6, 2025 18:09
Copy link
Collaborator

@AlexZorkin AlexZorkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good ty

* Pass compliance period for get other use table options
* Add filter on compliance period < 2024 to enable the is_legacy filter.
* Add new is_legacy to FuelTypes and default to false
* Use constant for 2024
@dhaselhan dhaselhan force-pushed the feat/daniel-add-is-legacy-1550 branch from b9260b5 to d641a93 Compare January 6, 2025 21:03
@dhaselhan dhaselhan merged commit 59e57de into release-0.2.0 Jan 6, 2025
1 check passed
@dhaselhan dhaselhan deleted the feat/daniel-add-is-legacy-1550 branch January 6, 2025 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants