-
Notifications
You must be signed in to change notification settings - Fork 489
fix: enhanced CSV readings upload for new meter values #1613
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
base: development
Are you sure you want to change the base?
Changes from all commits
3d06acc
4611a82
14dbf2e
75eec8f
25b5393
c33dc57
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,11 +13,13 @@ import { selectIsAdmin } from '../../redux/slices/currentUserSlice'; | |
| import { ReadingsCSVUploadPreferences } from '../../types/csvUploadForm'; | ||
| import { TrueFalseType } from '../../types/items'; | ||
| import { MeterData, MeterTimeSortType } from '../../types/redux/meters'; | ||
| import { DisableChecksType } from '../../types/redux/units'; | ||
| import { submitReadings } from '../../utils/api/UploadCSVApi'; | ||
| import { ReadingsCSVUploadDefaults } from '../../utils/csvUploadDefaults'; | ||
| import { showErrorNotification, showSuccessNotification } from '../../utils/notifications'; | ||
| import { useTranslate } from '../../redux/componentHooks'; | ||
| import FormFileUploaderComponent from '../FormFileUploaderComponent'; | ||
| import TimeZoneSelect from '../TimeZoneSelect'; | ||
| import TooltipHelpComponent from '../TooltipHelpComponent'; | ||
| import TooltipMarkerComponent from '../TooltipMarkerComponent'; | ||
| import CreateMeterModalComponent from '../meters/CreateMeterModalComponent'; | ||
|
|
@@ -116,7 +118,7 @@ export default function ReadingsCSVUploadComponent() { | |
| })); | ||
| }; | ||
|
|
||
| const handleFileChange = (file: File) => { | ||
| const handleFileChange = (file: File | null) => { | ||
| if (file) { | ||
| setSelectedFile(file); | ||
| if (file.name.slice(-4) === '.csv' || file.name.slice(-3) === '.gz') { | ||
|
|
@@ -132,6 +134,13 @@ export default function ReadingsCSVUploadComponent() { | |
| } | ||
| } | ||
| }; | ||
|
|
||
| const handleTimeZoneChange = (timeZone: string) => { | ||
| setReadingsData(prevData => ({ | ||
| ...prevData, | ||
| timeZone: timeZone || '' | ||
| })); | ||
| }; | ||
| /* END of Handlers for each type of input change */ | ||
|
|
||
| useEffect(() => { | ||
|
|
@@ -672,6 +681,124 @@ export default function ReadingsCSVUploadComponent() { | |
| </Input> | ||
| </FormGroup> | ||
| </Col> | ||
| <Col> | ||
| <FormGroup> | ||
| <Label> | ||
| <div className='pb-1'> | ||
| {translate('meter.time.zone')} | ||
| </div> | ||
| </Label> | ||
| <TimeZoneSelect current={readingsData.timeZone ?? null} handleClick={timeZone => handleTimeZoneChange(timeZone)} /> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Asking about the null here and not used with some other TimeZoneSelect uses. I think all these null/''/undefined are probably related. |
||
| </FormGroup> | ||
| </Col> | ||
| </Row> | ||
| <Row xs='1' lg='2'> | ||
| <Col> | ||
| <FormGroup> | ||
| <Label for='minVal'> | ||
| <div className='pb-1'> | ||
| {translate('meter.minVal')} | ||
| </div> | ||
| </Label> | ||
| <Input | ||
| type='number' | ||
| id='minVal' | ||
| name='minVal' | ||
| value={readingsData.minVal || ''} | ||
| onChange={e => {handleChange(e);}} | ||
| /> | ||
| </FormGroup> | ||
| </Col> | ||
| <Col> | ||
| <FormGroup> | ||
| <Label for='maxVal'> | ||
| <div className='pb-1'> | ||
| {translate('meter.maxVal')} | ||
| </div> | ||
| </Label> | ||
| <Input | ||
| type='number' | ||
| id='maxVal' | ||
| name='maxVal' | ||
| value={readingsData.maxVal || ''} | ||
| onChange={e => {handleChange(e);}} | ||
| /> | ||
| </FormGroup> | ||
| </Col> | ||
| </Row> | ||
| <Row xs='1' lg='2'> | ||
| <Col> | ||
| <FormGroup> | ||
| <Label for='minDate'> | ||
| <div className='pb-1'> | ||
| {translate('meter.minDate')} | ||
| </div> | ||
| </Label> | ||
| <Input | ||
| type='text' | ||
| id='minDate' | ||
| name='minDate' | ||
| value={readingsData.minDate || ''} | ||
| onChange={e => {handleChange(e);}} | ||
| /> | ||
| </FormGroup> | ||
| </Col> | ||
| <Col> | ||
| <FormGroup> | ||
| <Label for='maxDate'> | ||
| <div className='pb-1'> | ||
| {translate('meter.maxDate')} | ||
| </div> | ||
| </Label> | ||
| <Input | ||
| type='text' | ||
| id='maxDate' | ||
| name='maxDate' | ||
| value={readingsData.maxDate || ''} | ||
| onChange={e => {handleChange(e);}} | ||
| /> | ||
| </FormGroup> | ||
| </Col> | ||
| </Row> | ||
| <Row xs='1' lg='2'> | ||
| <Col> | ||
| <FormGroup> | ||
| <Label for='maxError'> | ||
| <div className='pb-1'> | ||
| {translate('meter.maxError')} | ||
| </div> | ||
| </Label> | ||
| <Input | ||
| type='number' | ||
| id='maxError' | ||
| name='maxError' | ||
| value={readingsData.maxError || ''} | ||
| onChange={e => {handleChange(e);}} | ||
| /> | ||
| </FormGroup> | ||
| </Col> | ||
| <Col> | ||
| <FormGroup> | ||
| <Label for='disableChecks'> | ||
| <div className='pb-1'> | ||
| {translate('meter.disableChecks')} | ||
| </div> | ||
| </Label> | ||
| <Input | ||
| type='select' | ||
| id='disableChecks' | ||
| name='disableChecks' | ||
| value={readingsData.disableChecks || DisableChecksType.reject_all} | ||
| onChange={e => {handleChange(e);}} | ||
| > | ||
| {Object.values(DisableChecksType).map(disableChecks => { | ||
| return ( | ||
| <option value={disableChecks} key={disableChecks}>{translate(`DisableChecksType.${disableChecks}`)}</option> | ||
| ); | ||
| })} | ||
| </Input> | ||
| </FormGroup> | ||
| </Col> | ||
| </Row> | ||
| {/* TODO This feature is not working perfectly so disabling from web page but allowing in curl. | ||
| Rest of changes left so easy to add back in. | ||
|
|
@@ -680,7 +807,7 @@ export default function ReadingsCSVUploadComponent() { | |
| originally added to the web page input of import but decided to not allow | ||
| it at this time. Thus, the code was commented out. As of now | ||
| there is no plan to make this generally available due to its limitations.*/} | ||
| {/* | ||
| {/** | ||
| <Label check> | ||
| <Input | ||
| checked={useMeterZone} | ||
|
|
@@ -689,11 +816,11 @@ export default function ReadingsCSVUploadComponent() { | |
| onChange={handleCheckboxChange} | ||
| /> | ||
| <div className='ps-2'> | ||
| {translate('csv.readings.param.use.meter.zone' /> | ||
| {translate('csv.readings.param.use.meter.zone')} | ||
| </div> | ||
| </Label> | ||
| */} | ||
| {/* | ||
| {/** | ||
| <Label check> | ||
| <Input | ||
| checked={warnOnCumulativeReset} | ||
|
|
@@ -702,7 +829,7 @@ export default function ReadingsCSVUploadComponent() { | |
| onChange={handleCheckboxChange} | ||
| /> | ||
| <div className='ps-2'> | ||
| {translate('csv.readings.param.use.meter.zone' /> | ||
| {translate('csv.readings.param.use.meter.zone')} | ||
| </div> | ||
| </Label> | ||
| */} | ||
|
|
@@ -720,11 +847,11 @@ export default function ReadingsCSVUploadComponent() { | |
| </div> | ||
| <div className='pb-5'> | ||
| </div> | ||
| </Col> | ||
| </Row> | ||
| </Form> | ||
| </>)} | ||
| </Container> | ||
| </> | ||
| ); | ||
| } | ||
| </Col> | ||
| </Row> | ||
| </Form> | ||
| </>)} | ||
| </Container> | ||
| </> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,12 +3,20 @@ | |
| * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| import { MeterTimeSortType } from '../types/redux/meters'; | ||
| import { DisableChecksType } from './redux/units'; | ||
|
|
||
| export interface CSVUploadPreferences { | ||
| meterIdentifier: string; | ||
| gzip: boolean; | ||
| headerRow: boolean; | ||
| update: boolean; | ||
| timeZone?: string; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm curious why these have a ? and the others don't. |
||
| minVal?: string; | ||
| maxVal?: string; | ||
| minDate?: string; | ||
| maxDate?: string; | ||
| maxError?: string; | ||
| disableChecks?: DisableChecksType; | ||
| } | ||
|
|
||
| export interface ReadingsCSVUploadPreferences extends CSVUploadPreferences { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,7 +35,9 @@ export const submitReadings = async (uploadPreferences: ReadingsCSVUploadPrefere | |
| warnOnCumulativeReset: uploadPreferences.warnOnCumulativeReset | ||
| }; | ||
| for (const [preference, value] of Object.entries(uploadPreferencesForm)) { | ||
| formData.append(preference, value.toString()); | ||
| if (value !== undefined && value !== null) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would hope that undefined/null are not the value for these but if it was would sending them cause an issue. I wanted your insight into this change. |
||
| formData.append(preference, value.toString()); | ||
| } | ||
| } | ||
| formData.append('csvfile', readingsFile); // It is important for the server that the file is attached last. | ||
|
|
||
|
|
@@ -61,7 +63,9 @@ export const submitMeters = async (uploadPreferences: MetersCSVUploadPreferences | |
| update: uploadPreferences.update | ||
| }; | ||
| for (const [preference, value] of Object.entries(uploadPreferencesForm)) { | ||
| formData.append(preference, value.toString()); | ||
| if (value !== undefined && value !== null) { | ||
| formData.append(preference, value.toString()); | ||
| } | ||
| } | ||
| formData.append('csvfile', metersFile); // It is important for the server that the file is attached last. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| * License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| import { DisableChecksType } from '../types/redux/units'; | ||
| import { ReadingsCSVUploadPreferences, MetersCSVUploadPreferences } from '../types/csvUploadForm'; | ||
| import { MeterTimeSortType } from '../types/redux/meters'; | ||
|
|
||
|
|
@@ -25,7 +26,14 @@ export const ReadingsCSVUploadDefaults: ReadingsCSVUploadPreferences = { | |
| timeSort: MeterTimeSortType.increasing, | ||
| update: false, | ||
| useMeterZone: false, | ||
| warnOnCumulativeReset: false | ||
| warnOnCumulativeReset: false, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This did not start with this PR but I wanted to note for a probable issue. This is only used in src/client/app/components/csv/ReadingsCSVUploadComponent.tsx. In the meter component it uses |
||
| timeZone: '', | ||
| minVal: '', | ||
| maxVal: '', | ||
| minDate: '', | ||
| maxDate: '', | ||
| maxError: '', | ||
| disableChecks: DisableChecksType.reject_all | ||
| }; | ||
|
|
||
| export const MetersCSVUploadDefaults: MetersCSVUploadPreferences = { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this directly related to the changes you are making or something you noticed since it checks if file is okay on the next line?