-
Notifications
You must be signed in to change notification settings - Fork 489
Internationalize csv upload #1571
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
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 |
|---|---|---|
|
|
@@ -19,10 +19,9 @@ interface ApiResponse { | |
| } | ||
|
|
||
| export const submitReadings = async (uploadPreferences: ReadingsCSVUploadPreferences, readingsFile: File, | ||
| dispatch: Dispatch): Promise<ApiResponse> => { | ||
| dispatch: Dispatch, language: string): Promise<ApiResponse> => { //added language parameter for i18n | ||
|
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. Could this be the type of the enum discussed in another comment? If so, I don't know if it should ripple to other places. |
||
| const backend = new ApiBackend(); | ||
| const formData = new FormData(); | ||
|
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. Could you put back the comment that was removed from the next line. I realize no conversion is being done here but I think it was a warning about usage. |
||
| // The Boolean values in state must be converted to the submitted values of yes and no. | ||
| const uploadPreferencesForm: ReadingsCSVUploadPreferences = { | ||
| ...uploadPreferences, | ||
| gzip: uploadPreferences.gzip, | ||
|
|
@@ -37,11 +36,12 @@ export const submitReadings = async (uploadPreferences: ReadingsCSVUploadPrefere | |
| for (const [preference, value] of Object.entries(uploadPreferencesForm)) { | ||
| formData.append(preference, value.toString()); | ||
| } | ||
| formData.append('csvfile', readingsFile); // It is important for the server that the file is attached last. | ||
| formData.append('csvfile', readingsFile); | ||
|
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. Please put back the removed comment. It can go above the line of code (as it should have been). |
||
|
|
||
| let message = ''; | ||
| try { | ||
| message = await backend.doPostRequest<string>('/api/csv/readings', formData); | ||
| //sends langauge preference to server | ||
|
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. langauge is misspelled. |
||
| message = await backend.doPostRequest<string>('/api/csv/readings', formData, {}, { 'Accept-Language': language }); | ||
| dispatch(baseApi.util.invalidateTags(['Readings'])); | ||
| return { success: true, message: message }; | ||
| } catch (error) { | ||
|
|
@@ -50,10 +50,9 @@ export const submitReadings = async (uploadPreferences: ReadingsCSVUploadPrefere | |
| }; | ||
|
|
||
| export const submitMeters = async (uploadPreferences: MetersCSVUploadPreferences, metersFile: File, | ||
| dispatch: Dispatch): Promise<ApiResponse> => { | ||
| dispatch: Dispatch, language: string): Promise<ApiResponse> => { //added language for i18n | ||
|
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. Could you remove all the comments saying you added something. It is okay to put in comments about what code is doing (as needed) but this one is associated with now and not for all time. I think it is okay to just remove. |
||
| const backend = new ApiBackend(); | ||
| const formData = new FormData(); | ||
| // The Boolean values in state must be converted to the submitted values of yes and no. | ||
| const uploadPreferencesForm: CSVUploadPreferences = { | ||
| ...uploadPreferences, | ||
| gzip: uploadPreferences.gzip, | ||
|
|
@@ -63,15 +62,14 @@ export const submitMeters = async (uploadPreferences: MetersCSVUploadPreferences | |
| for (const [preference, value] of Object.entries(uploadPreferencesForm)) { | ||
| formData.append(preference, value.toString()); | ||
| } | ||
| formData.append('csvfile', metersFile); // It is important for the server that the file is attached last. | ||
| formData.append('csvfile', metersFile); | ||
|
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. As above, add back in comment. |
||
|
|
||
| try { | ||
| const response = await backend.doPostRequest<string>('/api/csv/meters', formData); | ||
| // Meter Data was sent to the DB, invalidate meters for now | ||
| //send langauge preference to server | ||
| const response = await backend.doPostRequest<string>('/api/csv/meters', formData, {}, { 'Accept-Language': language }); | ||
| dispatch(baseApi.util.invalidateTags(['MeterData'])); | ||
| // meters were invalidated so all meter changes will now reflect in Redux state, now return | ||
| return { success: true, message: response }; | ||
| } catch (error) { | ||
| return { success: false, message: error.response.data }; | ||
| } | ||
| }; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,9 @@ | |
| * meter and readings data. | ||
| */ | ||
|
|
||
|
|
||
| const { translate, getLanguageFromRequest } = require('../translations/translate'); | ||
|
|
||
| const moment = require('moment'); | ||
| const crypto = require('crypto'); | ||
| const express = require('express'); | ||
|
|
@@ -101,13 +104,19 @@ router.use(function (req, res, next) { | |
| // We need this extra middleware because multer does not provide an option to guard against the case where no file is uploaded. | ||
| router.use(function (req, res, next) { | ||
| if (!req.file) { | ||
| failure(req, res, new CSVPipelineError('No csv file was uploaded. A csv file must be submitted via the csvfile parameter.')); | ||
| //failure(req, res, new CSVPipelineError('No csv file was uploaded. A csv file must be submitted via the csvfile parameter.')); | ||
|
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 think the commented out code is the old code that can now be removed. Also, the indenting seems off and the following lines. |
||
| const language = getLanguageFromRequest(req); | ||
| const errorMessage = translate('csv.upload.error.no.file', language); | ||
| failure(req, res, new CSVPipelineError(errorMessage)); | ||
|
|
||
| } else { | ||
| next(); | ||
| } | ||
| }); | ||
|
|
||
| router.post('/meters', validateMetersCsvUploadParams, async (req, res) => { | ||
| const language = getLanguageFromRequest(req); | ||
| const t = (key) => translate(key, language); | ||
|
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 think using very short names is prone to error. Could you refactor into a better name please. |
||
| const isGzip = normalizeBoolean(req.body.gzip); | ||
| const uploadedFilepath = req.file.path; | ||
| let csvFilepath; | ||
|
|
@@ -120,15 +129,16 @@ router.post('/meters', validateMetersCsvUploadParams, async (req, res) => { | |
| fileBuffer = zlib.gunzipSync(fileBuffer); | ||
| // We expect this directory to have been created by this stage of the pipeline. | ||
| const dir = `${__dirname}/../tmp/uploads/csvPipeline`; | ||
| csvFilepath = await saveCsv(fileBuffer, 'meters', dir); | ||
| csvFilepath = await saveCsv(fileBuffer, 'meters', dir,language); // pass lagnauge for translated error messages | ||
|
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. Comment above line and spelling issue. Also, formatting off (space missing after comma). Finally, I don't see language accepted in the called function. |
||
| log.info(`The unzipped file ${csvFilepath} was created to upload meters csv data`); | ||
| } else { | ||
| csvFilepath = uploadedFilepath; | ||
| } | ||
|
|
||
| const conn = getConnection(); | ||
| await uploadMeters(req, res, csvFilepath, conn); | ||
| success(req, res, 'Successfully inserted the meters.'); | ||
| await uploadMeters(req, res, csvFilepath, conn, language); | ||
|
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. Okay, I'm going to stop commenting on this but I again don't see the new argument used in the calling function. |
||
| //success(req, res, 'Successfully inserted the meters.'); | ||
|
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. As other comment, remove old code. |
||
| success(req, res, t('csv.upload.meters.success')); | ||
| } catch (error) { | ||
| failure(req, res, error); | ||
|
|
||
|
|
@@ -150,9 +160,15 @@ router.post('/meters', validateMetersCsvUploadParams, async (req, res) => { | |
| }); | ||
| } | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| router.post('/readings', validateReadingsCsvUploadParams, async (req, res) => { | ||
|
|
||
|
|
||
|
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. There is some space indenting but OED wants tabs. Also, there are extra blank lines. Finally see comment above about name. |
||
| const language = getLanguageFromRequest(req); | ||
| const t = (key) => translate(key, language); | ||
|
|
||
|
|
||
| const isGzip = normalizeBoolean(req.body.gzip); | ||
| const isRefreshReadings = normalizeBoolean(req.body.refreshReadings); | ||
| const uploadedFilepath = req.file.path; | ||
|
|
@@ -168,13 +184,13 @@ router.post('/readings', validateReadingsCsvUploadParams, async (req, res) => { | |
| fileBuffer = zlib.gunzipSync(fileBuffer); | ||
| // We expect this directory to have been created by this stage of the pipeline. | ||
| const dir = `${__dirname}/../tmp/uploads/csvPipeline`; | ||
| csvFilepath = await saveCsv(fileBuffer, 'readings', dir); | ||
| csvFilepath = await saveCsv(fileBuffer, 'readings', dir, language); | ||
| log.info(`The unzipped file ${csvFilepath} was created to upload readings csv data`); | ||
| } else { | ||
| csvFilepath = uploadedFilepath; | ||
| } | ||
| const conn = getConnection(); | ||
| ({ isAllReadingsOk, msgTotal } = await uploadReadings(req, res, csvFilepath, conn)); | ||
| ({ isAllReadingsOk, msgTotal } = await uploadReadings(req, res, csvFilepath, conn, language)); | ||
| if (isRefreshReadings) { | ||
| // Refresh readings so show when daily data is used. | ||
| await refreshAllReadingViews(); | ||
|
|
@@ -202,14 +218,19 @@ router.post('/readings', validateReadingsCsvUploadParams, async (req, res) => { | |
| } | ||
| let message; | ||
| if (isAllReadingsOk) { | ||
| message = '<h2>It looks like the insert of the readings was a success.</h2>' | ||
| //message = '<h2>It looks like the insert of the readings was a success.</h2>' | ||
|
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. See comments above about commented out code. Also, the indenting is off. |
||
| //if (msgTotal !== '') { | ||
| // message += '<h3>However, note that the processing of the readings returned these warning(s):</h3>' + msgTotal; | ||
| message = `<h2>${t('csv.upload.readings.success.complete')}</h2>`; | ||
| if (msgTotal !== '') { | ||
| message += '<h3>However, note that the processing of the readings returned these warning(s):</h3>' + msgTotal; | ||
| } | ||
| message += `<h3>${t('csv.upload.readings.success.with.warnings')}</h3>` + msgTotal; | ||
| } | ||
| success(req, res, message); | ||
| } else { | ||
| message = '<h2>It looks like the insert of the readings had issues with some or all of the readings where' + | ||
| ' the processing of the readings returned these warning(s)/error(s):</h2>' + msgTotal; | ||
| //message = '<h2>It looks like the insert of the readings had issues with some or all of the readings where' + | ||
| //' the processing of the readings returned these warning(s)/error(s):</h2>' + msgTotal; | ||
| message = `<h2>${t('csv.upload.readings.error.has.issues')}</h2>` + msgTotal; | ||
|
|
||
|
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. Please remove new blank line. |
||
| failure(req, res, message); | ||
| } | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,14 +53,15 @@ async function loadCsvInput( | |
| honorDst = false, | ||
| relaxedParsing = false, | ||
| useMeterZone = false, | ||
| warnOnCumulativeReset = false | ||
| warnOnCumulativeReset = false, | ||
| language = 'en' //added for internationalization: language preference from user | ||
|
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. Comment above code. Also, maybe add why en was used here. |
||
| ) { | ||
| try { | ||
| const dataRows = await readCsv(filePath, headerRow); | ||
| return loadArrayInput(dataRows, meterID, mapRowToModel, timeSort, readingRepetition, | ||
|
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 don't see the new argument of language as actually added to the function being called. |
||
| isCumulative, cumulativeReset, cumulativeResetStart, cumulativeResetEnd, | ||
| readingGap, readingLengthVariation, isEndOnly, shouldUpdate, conditionSet, conn, | ||
| honorDst, relaxedParsing, useMeterZone, warnOnCumulativeReset); | ||
| honorDst, relaxedParsing, useMeterZone, warnOnCumulativeReset, language); | ||
| } catch (err) { | ||
| log.error(`Error updating meter ${meterID} with data from ${filePath}: ${err}`, err); | ||
| } | ||
|
|
||
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.
I think this comment can go but if you want it to stay then please place above the code line. Moving the comment applies to some other lines too. Also, OED would like a blank line after the MPL header comment. These apply to other files too.