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

LCFS-1280: Validation for Quantity Supplied #1335

Merged
merged 6 commits into from
Dec 17, 2024

Conversation

areyeslo
Copy link
Collaborator

@areyeslo areyeslo commented Dec 2, 2024

  • The default Quantity supplied value being 0 should not be allow.
  • Zero should not be a valid amount in a fuel supply record and the row should not save until the user has entered a value.

PR related: #1312

Story

@areyeslo areyeslo changed the title LCFS-1280: Validation for Quantity Supplied in Fuel Supply. LCFS-1280: Validation for Quantity Supplied in Fuel Supply (1/2) Dec 2, 2024
Copy link

github-actions bot commented Dec 2, 2024

Backend Test Results

493 tests  ±0   490 ✅ ±0   1m 48s ⏱️ -2s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     3 ❌ ±0 

For more details on these failures, see this check.

Results for commit a14d1ec. ± Comparison against base commit 8edd0af.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 2, 2024

Frontend Test Results

  1 files  ±0  116 suites  ±0   38s ⏱️ -1s
390 tests ±0  370 ✅ ±0  20 💤 ±0  0 ❌ ±0 
392 runs  ±0  372 ✅ ±0  20 💤 ±0  0 ❌ ±0 

Results for commit a14d1ec. ± Comparison against base commit 8edd0af.

♻️ This comment has been updated with latest results.

@areyeslo areyeslo requested a review from prv-proton December 2, 2024 20:25
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, I would just like to see a frontend test covering this 0 quantity state and that it shows the error correctly.

@areyeslo areyeslo force-pushed the LCFS-LCFS-1280-FuelSupplyValidation_v2 branch 2 times, most recently from 0290b58 to ea5efa5 Compare December 4, 2024 02:14
@areyeslo areyeslo changed the title LCFS-1280: Validation for Quantity Supplied in Fuel Supply (1/2) LCFS-1280: Validation for Quantity Supplied Dec 4, 2024
@areyeslo areyeslo force-pushed the LCFS-LCFS-1280-FuelSupplyValidation_v2 branch 2 times, most recently from 7602b3e to 59b6b62 Compare December 4, 2024 20:33
@AlexZorkin
Copy link
Collaborator

Let's get this merged, just need to fix the conflicts, otherwise looks good to go.

@areyeslo areyeslo force-pushed the LCFS-LCFS-1280-FuelSupplyValidation_v2 branch 7 times, most recently from 85f723e to c4544a3 Compare December 11, 2024 18:34
@areyeslo areyeslo requested a review from AlexZorkin December 11, 2024 18:50
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.

We don't want to add this validation across quantity fields, i think just focus on fuel_supply. If you hear differently from business area then let me know.

backend/lcfs/web/api/transaction/schema.py Outdated Show resolved Hide resolved
backend/lcfs/web/api/notional_transfer/schema.py Outdated Show resolved Hide resolved
@areyeslo areyeslo marked this pull request as draft December 12, 2024 22:54
@areyeslo areyeslo force-pushed the LCFS-LCFS-1280-FuelSupplyValidation_v2 branch from 5c75487 to 8350dca Compare December 13, 2024 21:19
@areyeslo areyeslo marked this pull request as ready for review December 13, 2024 21:40
@areyeslo areyeslo requested a review from AlexZorkin December 13, 2024 21:40
@areyeslo areyeslo force-pushed the LCFS-LCFS-1280-FuelSupplyValidation_v2 branch from ce1737f to a14d1ec Compare December 17, 2024 19:19
@areyeslo areyeslo merged commit 4dab704 into release-0.2.0 Dec 17, 2024
1 check passed
@kevin-hashimoto kevin-hashimoto deleted the LCFS-LCFS-1280-FuelSupplyValidation_v2 branch January 6, 2025 20:30
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