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

Fix: LCFS - BUG AG Grid copy and paste function for Fuel codes table … #1420

Merged
merged 6 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions backend/lcfs/tests/fuel_code/test_fuel_code_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ async def test_approve_fuel_code_not_found():
repo_mock.get_fuel_code.return_value = None

# Act & Assert
with pytest.raises(ServiceException):
with pytest.raises(ValueError, match="Fuel code not found"):
await service.approve_fuel_code(fuel_code_id)
repo_mock.get_fuel_code.assert_called_once_with(fuel_code_id)

Expand All @@ -229,7 +229,7 @@ async def test_approve_fuel_code_invalid_status():
repo_mock.get_fuel_code.return_value = mock_fuel_code

# Act & Assert
with pytest.raises(ServiceException):
with pytest.raises(ValueError, match="Fuel code is not in Draft"):
await service.approve_fuel_code(fuel_code_id)

repo_mock.get_fuel_code.assert_called_once_with(fuel_code_id)
2 changes: 1 addition & 1 deletion backend/lcfs/tests/other_uses/test_other_uses_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ async def test_update_other_use_not_found(other_uses_service):

mock_repo.get_other_use_version_by_user = AsyncMock(return_value=None)

with pytest.raises(ServiceException):
with pytest.raises(ValueError, match="Other use not found"):
await service.update_other_use(other_use_data, UserTypeEnum.SUPPLIER)


Expand Down
22 changes: 8 additions & 14 deletions backend/lcfs/web/api/fuel_code/repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,19 +396,8 @@ async def create_fuel_code(self, fuel_code: FuelCode) -> FuelCode:
"""
self.db.add(fuel_code)
await self.db.flush()
await self.db.refresh(
fuel_code,
[
"fuel_code_status",
"fuel_code_prefix",
"fuel_type",
"feedstock_fuel_transport_modes",
"finished_fuel_transport_modes",
],
)
# Manually load nested relationships
await self.db.refresh(fuel_code.fuel_type, ["provision_1", "provision_2"])
return fuel_code
result = await self.get_fuel_code(fuel_code.fuel_code_id)
return result

@repo_handler
async def get_fuel_code(self, fuel_code_id: int) -> FuelCode:
Expand Down Expand Up @@ -593,9 +582,14 @@ async def validate_fuel_code(self, suffix: str, prefix_id: int) -> str:
result = (await self.db.execute(query)).scalar_one_or_none()
if result:
fuel_code_main_version = suffix.split(".")[0]
return await self.get_next_available_sub_version_fuel_code_by_prefix(
suffix = await self.get_next_available_sub_version_fuel_code_by_prefix(
fuel_code_main_version, prefix_id
)
if int(suffix.split(".")[1]) > 9:
return await self.get_next_available_fuel_code_by_prefix(
result.fuel_code_prefix.prefix
)
return suffix
else:
return suffix

Expand Down
4 changes: 4 additions & 0 deletions backend/lcfs/web/api/fuel_code/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ async def convert_to_model(
transport_mode_id=matching_transport_mode.transport_mode_id,
)
)
else:
raise ValueError(f"Invalid transport mode: {transport_mode}")

for transport_mode in fuel_code_schema.finished_fuel_transport_mode or []:
matching_transport_mode = next(
Expand All @@ -221,6 +223,8 @@ async def convert_to_model(
transport_mode_id=matching_transport_mode.transport_mode_id,
)
)
else:
raise ValueError(f"Invalid transport mode: {transport_mode}")

return fuel_code

Expand Down
2 changes: 1 addition & 1 deletion backend/lcfs/web/core/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ async def wrapper(*args, **kwargs):
return await func(*args, **kwargs)

# raise the error to the view layer
except (DatabaseException, HTTPException, DataNotFoundException):
except (DatabaseException, HTTPException, DataNotFoundException, ValueError):
raise
# all other errors that occur in the service layer will log an error
except Exception as e:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const ValidationRenderer2 = ({ data }) => {
)
case 'error':
return (
<Tooltip title="validation error">
<Tooltip title={data.validationMsg || 'validation error'}>
<Icon
aria-label="shows sign for validation"
data-testid="validation-sign"
Expand Down
1 change: 1 addition & 0 deletions frontend/src/constants/routes/apiRoutes.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export const apiRoutes = {
getFuelCode: '/fuel-codes/:fuelCodeId',
saveFuelCode: '/fuel-codes',
approveFuelCode: '/fuel-codes/:fuelCodeId/approve',
deleteFuelCode: '/fuel-codes/:fuelCodeId',
fuelCodeOptions: '/fuel-codes/table-options',
fuelCodeSearch: '/fuel-codes/search?',
getFuelCodes: '/fuel-codes/list',
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/hooks/useFuelCode.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export const useDeleteFuelCode = (options) => {
...options,
mutationFn: async (fuelCodeID) => {
return await client.delete(
apiRoutes.updateFuelCode.replace(':fuelCodeId', fuelCodeID)
apiRoutes.deleteFuelCode.replace(':fuelCodeId', fuelCodeID)
)
}
})
Expand Down
76 changes: 73 additions & 3 deletions frontend/src/views/FuelCodes/AddFuelCode/AddEditFuelCode.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import BCModal from '@/components/BCModal'
import BCTypography from '@/components/BCTypography'
import { FUEL_CODE_STATUSES } from '@/constants/statuses'
import { useCurrentUser } from '@/hooks/useCurrentUser'
import Papa from 'papaparse'

const AddEditFuelCodeBase = () => {
const { fuelCodeID } = useParams()
Expand Down Expand Up @@ -197,6 +198,7 @@ const AddEditFuelCodeBase = () => {
} else {
const res = await createFuelCode(updatedData)
updatedData.fuelCodeId = res.data.fuelCodeId
updatedData.fuelSuffix = res.data.fuelSuffix
}

updatedData = {
Expand All @@ -210,7 +212,9 @@ const AddEditFuelCodeBase = () => {
})
} catch (error) {
setErrors({
[params.node.data.id]: error.response.data.errors[0].fields
[params.node.data.id]:
error.response.data?.errors &&
error.response.data?.errors[0]?.fields
})

updatedData = {
Expand All @@ -229,10 +233,12 @@ const AddEditFuelCodeBase = () => {
const errMsg = `Error updating row: ${
fieldLabels.length === 1 ? fieldLabels[0] : ''
} ${message}`

updatedData.validationMsg = errMsg
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tested and received 422, when processing the date fields. They are not in correct format. Then, needed to give a proper format. Something like this:


      // Format date fields to ISO 8601
      const formattedData = { ...params.node.data };
      dateFields.forEach((field) => {
        if (formattedData[field]) {
          const date = new Date(formattedData[field]);
          if (!isNaN(date.getTime())) {
            formattedData[field] = date.toISOString().split('T')[0]; // Keep only the YYYY-MM-DD part
          }
        }
      });
      let updatedData = Object.entries(formattedData)

After that the request passes the schema validation, it can start executing create_fuel_code. However, an error is raised: ValueError: Invalid transport mode: Truck

Copy link
Collaborator Author

@prv-proton prv-proton Dec 10, 2024

Choose a reason for hiding this comment

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

Thanks for your review @areyeslo ,

  • yes, from excel , date needs to be copied in proper format otherwise it throws error.
  • Truck is no longer among the valid transport mode in the database. hence value error.

handleError(error, errMsg)
} else {
handleError(error, `Error updating row: ${error.message}`)
const errMsg = error.response?.data?.detail || error.message
updatedData.validationMsg = errMsg
handleError(error, `Error updating row: ${errMsg}`)
}
}

Expand All @@ -241,6 +247,69 @@ const AddEditFuelCodeBase = () => {
[updateFuelCode, t]
)

const handlePaste = useCallback(
(event, { api, columnApi }) => {
const newData = []
const clipboardData = event.clipboardData || window.clipboardData
const pastedData = clipboardData.getData('text/plain')
const headerRow = api
.getAllDisplayedColumns()
.map((column) => column.colDef.field)
.filter((col) => col)
.join('\t')
const parsedData = Papa.parse(headerRow + '\n' + pastedData, {
delimiter: '\t',
header: true,
transform: (value, field) => {
// Check for date fields and format them
const dateRegex = /^\d{4}-\d{2}-\d{2}$/ // Matches YYYY-MM-DD format
if (field.toLowerCase().includes('date') && !dateRegex.test(value)) {
const parsedDate = new Date(value)
if (!isNaN(parsedDate)) {
return parsedDate.toISOString().split('T')[0] // Format as YYYY-MM-DD
}
}
const num = Number(value) // Attempt to convert to a number if possible
return isNaN(num) ? value : num // Return the number if valid, otherwise keep as string
},
skipEmptyLines: true
})
if (parsedData.data?.length < 0) {
return
}
parsedData.data.forEach((row) => {
const newRow = { ...row }
newRow.id = uuid()
newRow.prefixId = optionsData?.fuelCodePrefixes?.find(
(o) => o.prefix === row.prefix
)?.fuelCodePrefixId
newRow.fuelTypeId = optionsData?.fuelTypes?.find(
(o) => o.fuelType === row.fuelType
)?.fuelTypeId
newRow.fuelSuffix = newRow.fuelSuffix.toString()
newRow.feedstockFuelTransportMode = row.feedstockFuelTransportMode
.split(',')
.map((item) => item.trim())
newRow.finishedFuelTransportMode = row.finishedFuelTransportMode
.split(',')
.map((item) => item.trim())
newRow.modified = true
newData.push(newRow)
})
const transactions = api.applyTransaction({ add: newData })
// Trigger onCellEditingStopped event to update the row in backend.
transactions.add.forEach((node) => {
onCellEditingStopped({
node,
oldValue: '',
newvalue: undefined,
...api
})
})
},
[onCellEditingStopped, optionsData]
)

const duplicateFuelCode = async (params) => {
const rowData = {
...params.data,
Expand Down Expand Up @@ -327,6 +396,7 @@ const AddEditFuelCodeBase = () => {
onAction={onAction}
showAddRowsButton={!existingFuelCode && hasRoles(roles.analyst)}
context={{ errors }}
handlePaste={handlePaste}
/>
{existingFuelCode?.fuelCodeStatus.status !==
FUEL_CODE_STATUSES.APPROVED && (
Expand Down
9 changes: 5 additions & 4 deletions frontend/src/views/FuelCodes/AddFuelCode/_schema.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ export const fuelCodeColDefs = (optionsData, errors, isCreate, canEdit) => [
const selectedPrefix = optionsData?.fuelCodePrefixes?.find(
(obj) => obj.prefix === params.newValue
)
params.data.fuelTypeId = selectedPrefix.fuelCodePrefixId
params.data.fuelCodePrefixId = selectedPrefix.fuelCodePrefixId

params.data.fuelCode = optionsData?.fuelCodePrefixes?.find(
params.data.fuelSuffix = optionsData?.fuelCodePrefixes?.find(
(obj) => obj.prefix === params.newValue
)?.nextFuelCode
params.data.company = undefined
Expand Down Expand Up @@ -327,12 +327,12 @@ export const fuelCodeColDefs = (optionsData, errors, isCreate, canEdit) => [
return selectedOption.fuelType
}
const selectedOption = optionsData?.fuelTypes?.find(
(obj) => obj.fuelType === params.data.fuel
(obj) => obj.fuelType === params.data.fuelType
)
if (selectedOption) {
params.data.fuelTypeId = selectedOption.fuelTypeId
}
return params.data.fuel
return params.data.fuelType
},
valueSetter: (params) => {
if (params.newValue) {
Expand All @@ -341,6 +341,7 @@ export const fuelCodeColDefs = (optionsData, errors, isCreate, canEdit) => [
)
params.data.fuelTypeId = selectedFuelType.fuelTypeId
}
return params.data.fuelType
},
cellEditorParams: {
options: optionsData?.fuelTypes
Expand Down
Loading