From cb4697080838782fdf4eec534ebc1c3eefb67e1d Mon Sep 17 00:00:00 2001 From: Matt Date: Sat, 15 Mar 2025 18:50:33 -0400 Subject: [PATCH] Restore line and subline fields --- ...ate-table-changes-implementation-issue4.md | 239 ++++++++++++++++++ ...ate-table-changes-implementation-issue8.md | 138 ++++++++++ .../components/ValidationCell.tsx | 7 + .../components/ValidationContainer.tsx | 23 +- .../components/ValidationTable.tsx | 74 ++++-- .../components/cells/SelectCell.tsx | 113 +++++++-- 6 files changed, 549 insertions(+), 45 deletions(-) create mode 100644 docs/validate-table-changes-implementation-issue4.md create mode 100644 docs/validate-table-changes-implementation-issue8.md diff --git a/docs/validate-table-changes-implementation-issue4.md b/docs/validate-table-changes-implementation-issue4.md new file mode 100644 index 0000000..0fc814f --- /dev/null +++ b/docs/validate-table-changes-implementation-issue4.md @@ -0,0 +1,239 @@ +# Validation Display Issue Implementation + +## Issue Being Addressed + +**Validation Display Issue**: Validation isn't happening beyond checking if a cell is required or not. All validation rules defined in import.tsx need to be respected. +* Required fields correctly show a red border when empty (✅ ALREADY WORKING) +* Non-empty fields with validation errors (regex, unique, etc.) should show a red border AND an alert circle icon with tooltip explaining the error (❌ NOT WORKING) + +## Implementation Attempts + +!!!!**NOTE** All previous attempts have been reverted and are no longer part of the code, please take this into account when trying a new solution. !!!! + +### Attempt 1: Fix Validation Display Logic + +**Approach**: Modified `processErrors` function to separate required errors from validation errors and show alert icons only for non-empty fields with validation errors. + +**Changes Made**: +```typescript +function processErrors(value: any, errors: ErrorObject[]) { + // ...existing code... + + // Separate required errors from other validation errors + const requiredErrors = errors.filter(error => + error.message?.toLowerCase().includes('required') + ); + const validationErrors = errors.filter(error => + !error.message?.toLowerCase().includes('required') + ); + + const isRequiredButEmpty = valueIsEmpty && requiredErrors.length > 0; + const hasValidationErrors = validationErrors.length > 0; + const shouldShowErrorIcon = hasValidationErrors && !valueIsEmpty; + + // ...more code... +} +``` + +**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip. + +### Attempt 2: Comprehensive Fix for Validation Display + +**Approach**: Completely rewrote `processErrors` function with consistent empty value detection, clear error separation, and improved error message extraction. + +**Changes Made**: +```typescript +function processErrors(value: any, errors: ErrorObject[]) { + if (!errors || errors.length === 0) { + return { filteredErrors: [], hasError: false, isRequiredButEmpty: false, + shouldShowErrorIcon: false, errorMessages: '' }; + } + + const valueIsEmpty = isEmpty(value); + const requiredErrors = errors.filter(error => + error.message?.toLowerCase().includes('required') + ); + const validationErrors = errors.filter(error => + !error.message?.toLowerCase().includes('required') + ); + + let filteredErrors = valueIsEmpty ? requiredErrors : validationErrors; + + const isRequiredButEmpty = valueIsEmpty && requiredErrors.length > 0; + const hasValidationErrors = validationErrors.length > 0; + const hasError = isRequiredButEmpty || hasValidationErrors; + const shouldShowErrorIcon = hasValidationErrors && !valueIsEmpty; + + let errorMessages = ''; + if (shouldShowErrorIcon) { + errorMessages = validationErrors.map(getErrorMessage).join('\n'); + } + + return { filteredErrors, hasError, isRequiredButEmpty, shouldShowErrorIcon, errorMessages }; +} +``` + +**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip. + +### Attempt 3: Simplified Error Processing Logic + +**Approach**: Refactored `processErrors` to use shared `isEmpty` function, simplified error icon logic, and made error message extraction more direct. + +**Changes Made**: +```typescript +function processErrors(value: any, errors: ErrorObject[]) { + if (!errors || errors.length === 0) { + return { filteredErrors: [], hasError: false, isRequiredButEmpty: false, + shouldShowErrorIcon: false, errorMessages: '' }; + } + + const valueIsEmpty = isEmpty(value); + const requiredErrors = errors.filter(error => + error.message?.toLowerCase().includes('required') + ); + const validationErrors = errors.filter(error => + !error.message?.toLowerCase().includes('required') + ); + + let filteredErrors = valueIsEmpty ? requiredErrors : validationErrors; + + const isRequiredButEmpty = valueIsEmpty && requiredErrors.length > 0; + const hasValidationErrors = !valueIsEmpty && validationErrors.length > 0; + const hasError = isRequiredButEmpty || hasValidationErrors; + const shouldShowErrorIcon = hasValidationErrors; + + let errorMessages = ''; + if (shouldShowErrorIcon) { + errorMessages = validationErrors.map(getErrorMessage).join('\n'); + } + + return { filteredErrors, hasError, isRequiredButEmpty, shouldShowErrorIcon, errorMessages }; +} +``` + +**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip. + +### Attempt 4: Consistent Error Processing Across Components + +**Approach**: Updated both `processErrors` function and `ValidationCell` component to ensure consistent error handling between components. + +**Changes Made**: +```typescript +// In processErrors function +function processErrors(value: any, errors: ErrorObject[]) { + // Similar to Attempt 3 with consistent error handling +} + +// In ValidationCell component +const ValidationCell = ({ field, value, onChange, errors, /* other props */ }) => { + // ...existing code... + + // Use the processErrors function to handle validation errors + const { hasError, isRequiredButEmpty, shouldShowErrorIcon, errorMessages } = + React.useMemo(() => processErrors(value, errors), [value, errors]); + + // ...rest of the component... +} +``` + +**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip. + +### Attempt 5: Unified Error Processing with ItemNumberCell + +**Approach**: Replaced custom error processing in `ValidationCell` with the same `processErrors` utility used by `ItemNumberCell`. + +**Changes Made**: +```typescript +const ValidationCell = ({ field, value, onChange, errors, /* other props */ }) => { + // State and context setup... + + // For item_number fields, use the specialized component + if (fieldKey === 'item_number') { + return ; + } + + // Use the same processErrors utility function that ItemNumberCell uses + const { hasError, isRequiredButEmpty, shouldShowErrorIcon, errorMessages } = + React.useMemo(() => processErrors(value, errors), [value, errors]); + + // Rest of component... +} +``` + +**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip. + +### Attempt 6: Standardize Error Processing Across Cell Types + +**Approach**: Standardized error handling across all cell types using the shared `processErrors` utility function. + +**Changes Made**: Similar to Attempt 5, with focus on standardizing the approach for determining when to show validation error icons. + +**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip. + +### Attempt 7: Replace Custom Error Processing with Shared Utility + +**Approach**: Ensured consistent error handling between `ItemNumberCell` and regular `ValidationCell` components. + +**Changes Made**: Similar to Attempts 5 and 6, with focus on using the shared utility function consistently. + +**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip. + +### Attempt 8: Improved Error Normalization and Deep Comparison + +**Approach**: Modified `MemoizedCell` in `ValidationTable.tsx` to use deep comparison for error objects and improved error normalization. + +**Changes Made**: +```typescript +// Create a memoized cell component +const MemoizedCell = React.memo(({ field, value, onChange, errors, /* other props */ }) => { + return ; +}, (prev, next) => { + // Basic prop comparison + if (prev.value !== next.value) return false; + if (prev.isValidating !== next.isValidating) return false; + if (prev.itemNumber !== next.itemNumber) return false; + + // Deep compare errors - critical for validation display + if (!prev.errors && next.errors) return false; + if (prev.errors && !next.errors) return false; + if (prev.errors && next.errors) { + if (prev.errors.length !== next.errors.length) return false; + + // Compare each error object + for (let i = 0; i < prev.errors.length; i++) { + if (prev.errors[i].message !== next.errors[i].message) return false; + if (prev.errors[i].level !== next.errors[i].level) return false; + if (prev.errors[i].source !== next.errors[i].source) return false; + } + } + + // Compare options... + + return true; +}); + +// In the field columns definition: +cell: ({ row }) => { + const rowErrors = validationErrors.get(row.index); + const cellErrors = rowErrors?.[fieldKey] || []; + + // Ensure cellErrors is always an array + const normalizedErrors = Array.isArray(cellErrors) ? cellErrors : [cellErrors]; + + return ; +} +``` + +**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip. + +## Root Causes (Revised Hypothesis) + +After multiple attempts, the issue appears more complex than initially thought. Possible root causes: + +1. **Error Object Structure**: Error objects might not have the expected structure or properties +2. **Error Propagation**: Errors might be getting filtered out before reaching cell components +3. **Validation Rules Configuration**: Validation rules in import.tsx might be incorrectly configured +4. **Error State Management**: Error state might not be properly updated or might be reset incorrectly +5. **Component Rendering Logic**: Components might not re-render when validation state changes +6. **CSS/Styling Issues**: Validation icons might be rendered but hidden due to styling issues +7. **Validation Timing**: Validation might be happening at the wrong time or getting overridden \ No newline at end of file diff --git a/docs/validate-table-changes-implementation-issue8.md b/docs/validate-table-changes-implementation-issue8.md new file mode 100644 index 0000000..314771b --- /dev/null +++ b/docs/validate-table-changes-implementation-issue8.md @@ -0,0 +1,138 @@ +# Multiple Cell Edit Issue Implementation + +## Issue Being Addressed + +**Multiple Cell Edit Issue**: When you enter values in 2+ cells before validation finishes, contents from all edited cells get erased when validation finishes. + +## Implementation Attempts + +### Attempt 1: Fix Multiple Cell Edit Issue (First Approach) + +**Approach**: +- Added a tracking mechanism using a Set to keep track of cells that are currently being edited +- Modified the `flushPendingUpdates` function to preserve values of cells being edited +- Added cleanup of editing state after validation completes + +**Changes Made**: +```typescript +// Add ref to track cells currently being edited +const currentlyEditingCellsRef = useRef(new Set()); + +// Update a row's field value +const updateRow = useCallback((rowIndex: number, key: T, value: any) => { + // Add this cell to currently editing cells + const cellKey = `${rowIndex}-${key}`; + currentlyEditingCellsRef.current.add(cellKey); + + // ...existing code... + + // After validation completes, remove this cell from currently editing list + setTimeout(() => { + currentlyEditingCellsRef.current.delete(cellKey); + }, 100); +}, []); + +// Modify flushPendingUpdates to respect currently editing cells +const flushPendingUpdates = useCallback(() => { + // ...existing code... + + if (dataUpdates.length > 0) { + setData(prev => { + // ...existing code... + + dataUpdates.forEach((row, index) => { + if (index < newData.length) { + const updatedRow = { ...row }; + + // Check if any fields in this row are currently being edited + // If so, preserve their current values in the previous data + Object.keys(prev[index] || {}).forEach(key => { + const cellKey = `${index}-${key}`; + if (currentlyEditingCellsRef.current.has(cellKey)) { + // Keep the value from the previous state for this field + updatedRow[key] = prev[index][key]; + } + }); + + newData[index] = updatedRow; + } + }); + + return newData; + }); + } +}, []); +``` + +**Result**: +- Slight improvement - the first value entered was saved, but any subsequent values still got erased + +### Attempt 2: Fix Multiple Cell Edit Issue (Second Approach) + +**Approach**: +- Completely revised the cell editing tracking system +- Used a Map with timestamps to track editing cells more accurately +- Added proper Promise-based tracking for cell validation +- Increased timeout from 100ms to 1000ms +- Made cleanup more robust by checking if it's still the same editing session + +**Changes Made**: +```typescript +// Add ref to track cells currently being edited with timestamps +const currentlyEditingCellsRef = useRef(new Map()); + +// Add ref to track validation promises +const validationPromisesRef = useRef>>(new Map()); + +// Update a row's field value +const updateRow = useCallback((rowIndex: number, key: T, value: any) => { + // Mark this cell as being edited with the current timestamp + const cellKey = `${rowIndex}-${key}`; + currentlyEditingCellsRef.current.set(cellKey, Date.now()); + + // ...existing code... + + // Create a validation promise + const validationPromise = new Promise((resolve) => { + setTimeout(() => { + try { + validateRow(rowIndex); + } finally { + resolve(); + } + }, 0); + }); + + validationPromisesRef.current.set(cellKey, validationPromise); + + // When validation is complete, remove from validating cells + validationPromise.then(() => { + // ...existing code... + + // Keep this cell in the editing state for a longer time + setTimeout(() => { + if (currentlyEditingCellsRef.current.has(cellKey)) { + currentlyEditingCellsRef.current.delete(cellKey); + } + }, 1000); // Keep as "editing" for 1 second + }); +}, []); +``` + +**Result**: +- Worse than the first approach - now all values get erased, including the first one + +## Root Causes (Hypothesized) + +- The validation process might be updating the entire data state, causing race conditions with cell edits +- The timing of validation completions might be problematic +- State updates might be happening in a way that overwrites user changes +- The cell state tracking system is not robust enough to prevent overwrites + +## Next Steps + +The issue requires a more fundamental approach than just tweaking the editing logic. We need to: + +1. Implement a more robust state management system for cell edits that can survive validation cycles +2. Consider disabling validation during active editing +3. Implement a proper "dirty state" tracking system for cells \ No newline at end of file diff --git a/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/ValidationCell.tsx b/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/ValidationCell.tsx index 62d4b50..0742f55 100644 --- a/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/ValidationCell.tsx +++ b/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/ValidationCell.tsx @@ -99,6 +99,8 @@ const BaseCellContent = React.memo(({ (field.fieldType.type === 'input' || field.fieldType.type === 'multi-input') && field.fieldType.price === true; + console.log(`BaseCellContent: field.key=${field.key}, fieldType=${fieldType}, disabled=${field.disabled}, options=`, options); + if (fieldType === 'select') { return ( ); } @@ -121,6 +124,7 @@ const BaseCellContent = React.memo(({ options={options} hasErrors={hasErrors} className={className} + disabled={field.disabled} /> ); } @@ -133,6 +137,7 @@ const BaseCellContent = React.memo(({ hasErrors={hasErrors} isMultiline={isMultiline} isPrice={isPrice} + disabled={field.disabled} /> ); }, (prev, next) => { @@ -605,6 +610,8 @@ const ValidationCell = ({ ) : (
+ {/* Log options for debugging */} + {(() => { console.log(`ValidationCell: fieldKey=${fieldKey}, options=`, options); return null; })()} ({ // Only fetch if we have a valid company ID if (!companyId) return; + console.log(`Fetching product lines for row ${rowIndex}, company ${companyId}`); + // Set loading state for this row setIsLoadingLines(prev => ({ ...prev, [rowIndex]: true })); @@ -129,6 +131,7 @@ const ValidationContainer = ({ } const productLines = await response.json(); + console.log(`Received product lines for row ${rowIndex}:`, productLines); // Store the product lines for this specific row setRowProductLines(prev => ({ ...prev, [rowIndex]: productLines })); @@ -148,6 +151,8 @@ const ValidationContainer = ({ // Only fetch if we have a valid line ID if (!lineId) return; + console.log(`Fetching sublines for row ${rowIndex}, line ${lineId}`); + // Set loading state for this row setIsLoadingSublines(prev => ({ ...prev, [rowIndex]: true })); @@ -158,6 +163,7 @@ const ValidationContainer = ({ } const sublines = await response.json(); + console.log(`Received sublines for row ${rowIndex}:`, sublines); // Store the sublines for this specific row setRowSublines(prev => ({ ...prev, [rowIndex]: sublines })); @@ -365,6 +371,7 @@ const ValidationContainer = ({ // Enhanced updateRow function - memoized const enhancedUpdateRow = useCallback(async (rowIndex: number, fieldKey: T, value: any) => { // Process value before updating data + console.log(`enhancedUpdateRow called: rowIndex=${rowIndex}, fieldKey=${fieldKey}, value=`, value); let processedValue = value; // Strip dollar signs from price fields @@ -921,9 +928,13 @@ const ValidationContainer = ({ itemNumbers={itemNumbersMap} isLoadingTemplates={isLoadingTemplates} copyDown={handleCopyDown} + rowProductLines={rowProductLines} + rowSublines={rowSublines} + isLoadingLines={isLoadingLines} + isLoadingSublines={isLoadingSublines} /> ); - }), [validatingUpcRows, itemNumbers, isLoadingTemplates, handleCopyDown, validatingCells]); + }), [validatingUpcRows, itemNumbers, isLoadingTemplates, handleCopyDown, validatingCells, rowProductLines, rowSublines, isLoadingLines, isLoadingSublines]); // Memoize the rendered validation table const renderValidationTable = useMemo(() => ( @@ -945,6 +956,10 @@ const ValidationContainer = ({ isLoadingTemplates={isLoadingTemplates} copyDown={handleCopyDown} upcValidationResults={new Map()} + rowProductLines={rowProductLines} + rowSublines={rowSublines} + isLoadingLines={isLoadingLines} + isLoadingSublines={isLoadingSublines} /> ), [ EnhancedValidationTable, @@ -961,7 +976,11 @@ const ValidationContainer = ({ applyTemplate, getTemplateDisplayText, isLoadingTemplates, - handleCopyDown + handleCopyDown, + rowProductLines, + rowSublines, + isLoadingLines, + isLoadingSublines ]); // Add scroll container ref at the container level diff --git a/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/ValidationTable.tsx b/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/ValidationTable.tsx index 3c1a4dd..2bc3331 100644 --- a/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/ValidationTable.tsx +++ b/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/ValidationTable.tsx @@ -166,10 +166,18 @@ const ValidationTable = ({ validatingCells, itemNumbers, isLoadingTemplates = false, - copyDown + copyDown, + rowProductLines = {}, + rowSublines = {}, + isLoadingLines = {}, + isLoadingSublines = {} }: ValidationTableProps) => { const { translations } = useRsi(); + // Debug logs + console.log('ValidationTable rowProductLines:', rowProductLines); + console.log('ValidationTable rowSublines:', rowSublines); + // Add state for copy down selection mode const [isInCopyDownMode, setIsInCopyDownMode] = useState(false); const [sourceRowIndex, setSourceRowIndex] = useState(null); @@ -285,7 +293,7 @@ const ValidationTable = ({ const cache = new Map(); fields.forEach((field) => { - if (field.disabled) return; + // Don't skip disabled fields if (field.fieldType.type === 'select' || field.fieldType.type === 'multi-select') { const fieldKey = String(field.key); @@ -308,7 +316,7 @@ const ValidationTable = ({ // Memoize field columns with stable handlers const fieldColumns = useMemo(() => fields.map((field): ColumnDef, any> | null => { - if (field.disabled) return null; + // Don't filter out disabled fields, just pass the disabled state to the cell component const fieldWidth = field.width || ( field.fieldType.type === "checkbox" ? 80 : @@ -327,25 +335,51 @@ const ValidationTable = ({ accessorKey: fieldKey, header: field.label || fieldKey, size: fieldWidth, - cell: ({ row }) => ( - } - value={row.original[field.key as keyof typeof row.original]} - onChange={(value) => handleFieldUpdate(row.index, field.key as T, value)} - errors={validationErrors.get(row.index)?.[fieldKey] || []} - isValidating={validatingCells.has(`${row.index}-${field.key}`)} - fieldKey={fieldKey} - options={fieldOptions} - itemNumber={itemNumbers.get(row.index)} - width={fieldWidth} - rowIndex={row.index} - copyDown={(endRowIndex?: number) => handleCopyDown(row.index, field.key as string, endRowIndex)} - totalRows={data.length} - /> - ) + cell: ({ row }) => { + // Get row-specific options for line and subline fields + let options = fieldOptions; + const rowId = row.original.__index; + + if (fieldKey === 'line' && rowId && rowProductLines[rowId]) { + options = rowProductLines[rowId]; + console.log(`Setting line options for row ${rowId}:`, options); + } else if (fieldKey === 'subline' && rowId && rowSublines[rowId]) { + options = rowSublines[rowId]; + console.log(`Setting subline options for row ${rowId}:`, options); + } + + // Determine if this cell is in loading state + let isLoading = validatingCells.has(`${row.index}-${field.key}`); + + // Add loading state for line/subline fields + if (fieldKey === 'line' && rowId && isLoadingLines[rowId]) { + isLoading = true; + console.log(`Line field for row ${rowId} is loading`); + } else if (fieldKey === 'subline' && rowId && isLoadingSublines[rowId]) { + isLoading = true; + console.log(`Subline field for row ${rowId} is loading`); + } + + return ( + } + value={row.original[field.key as keyof typeof row.original]} + onChange={(value) => handleFieldUpdate(row.index, field.key as T, value)} + errors={validationErrors.get(row.index)?.[fieldKey] || []} + isValidating={isLoading} + fieldKey={fieldKey} + options={options} + itemNumber={itemNumbers.get(row.index)} + width={fieldWidth} + rowIndex={row.index} + copyDown={(endRowIndex?: number) => handleCopyDown(row.index, field.key as string, endRowIndex)} + totalRows={data.length} + /> + ); + } }; }).filter((col): col is ColumnDef, any> => col !== null), - [fields, validationErrors, validatingCells, itemNumbers, handleFieldUpdate, handleCopyDown, optionsCache, data.length]); + [fields, validationErrors, validatingCells, itemNumbers, handleFieldUpdate, handleCopyDown, optionsCache, data.length, rowProductLines, rowSublines, isLoadingLines, isLoadingSublines]); // Combine columns const columns = useMemo(() => [selectionColumn, templateColumn, ...fieldColumns], [selectionColumn, templateColumn, fieldColumns]); diff --git a/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/cells/SelectCell.tsx b/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/cells/SelectCell.tsx index 41b2095..b386145 100644 --- a/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/cells/SelectCell.tsx +++ b/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/cells/SelectCell.tsx @@ -51,6 +51,8 @@ const SelectCell = ({ // Add state for hover const [isHovered, setIsHovered] = useState(false); + console.log(`SelectCell: field.key=${field.key}, disabled=${disabled}, options=`, options); + // Helper function to check if a class is present in the className string const hasClass = (cls: string): boolean => { const classNames = className.split(' '); @@ -66,6 +68,7 @@ const SelectCell = ({ // Memoize options processing to avoid recalculation on every render const selectOptions = useMemo(() => { + console.log(`Processing options for ${field.key}:`, options); // Fast path check - if we have raw options, just use those if (options && options.length > 0) { // Check if options already have the correct structure to avoid mapping @@ -146,31 +149,95 @@ const SelectCell = ({ if (disabled) { const displayText = displayValue; + // For debugging, let's render the Popover component even if disabled + // This will help us determine if the issue is with the disabled state return ( -
setIsHovered(true)} - onMouseLeave={() => setIsHovered(false)} + { + setOpen(isOpen); + if (isOpen && onStartEdit) onStartEdit(); + }} > - {displayText || ""} -
+ + + + + + + + No options found. + + {selectOptions.map((option) => ( + handleSelect(value)} + className="flex w-full" + > + + {option.label} + + ))} + + + + + ); }