Issue 1217 - CSV Upload Validation Checks by kyle-parker-1500 · Pull Request #1579 · OpenEnergyDashboard/OED

kyle-parker-1500 · 2026-02-15T22:14:49Z

Description

Partly Addresses #1217

Type of change

Added validation checks for the following 6 fields within the uploadMeters.js file according to the note left on PR1403:

Max Error: Max Error must be within the range of 0 to 75.
Area Unit & Value: If area unit is none then area cannot be specified.
Min & Max Dates: The max date cannot be before the min date & validate if they are dates.
Duplicates: Duplicates within the range of 1 to 9.
Reading Gap: Should be greater than or equal to 0.
Reading Variation: Should be greater than or equal to 0.

Note merging this changes the database configuration.
This change requires a documentation update

Checklist

I have followed the OED pull request ideas
I have removed text in ( ) from the issue request
You acknowledge that every person contributing to this work has signed the OED Contributing License Agreement and each author is listed in the Description section.

Limitations

The branch which these changes were pushed on is around 300 commits behind the development branch. Also not all meters have checks being done on them, only those listed in PR1403 and those listed here. Destiny destinyveloz@msn.com, Oscar oaviles-saldana@csumb.edu, and I kylerparker1500@gmail.com are open to continue working on this issue.

Thank you to @SageMar and @cmatthews444 for writing the previous checks, much of our code was derived from what you wrote.

…ion for checking each value towards the bottom of the uploadMeters.js file as well as calling them when the submission is sent to the server

… made sure the correct data was being sent back to the client side

…ifferent caps/lowercase

…csv-validation Boolean value validations

…idation

…csv-validation Min max validation

…gging still.

…idation

Merge branch 'Issue1217-csv-validation' of https://github.com/SageMar/OED into Issue1217-csv-validation

…ecked, now it converts strings to ints and functions

…checked for validity before calling it to check min<max

… decimals are passing through

…idation

finished isDuplicate()

corrected isValidDate() logic

…TimeFormat, isDuplicate before PR

…imeFormat, isDuplicate

Co-authored-by: Destiny Veloz <destinyveloz@msn.com> Co-authored-by: Oscar Aviles-Saldana <oaviles-saldana@csumb.edu>

Co-authored-by: Destiny Veloz destinyveloz@msn.com Co-authored-by: Oscar Aviles-Saldana oaviles-saldana@csumb.edu

Co-authored-by: Destiny Veloz <destinyveloz@msn.com> Co-authored-by: Oscar Aviles-Saldana <oaviles-saldana@csumb.edu>

…vilSal/OED into final_checks_uploadMeters

huss · 2026-02-17T01:30:43Z

@kyle-parker-1500 Thank you for this PR. I see the CLA box is checked in the description. Given this, could you help with these:

I could not find an entry for you in our CLA records. Could you either do it or let me know if you think our records are wrong.
Our CLA records are by GitHub ID. I found @DestinyVeloz & @OscarAvilSal who I think might be the two other people listed. Could you or the other people verify that I have the correct GitHub IDs.

Please let me know if anything is not clear or you have thoughts.

kyle-parker-1500 · 2026-02-19T03:11:56Z

Hi @huss ,
That may have been my fault. I use a very similar username that I may have typed in by accident. I resubmitted the CLA with my current GitHub ID so I'm hoping that works out. The other IDs are correct. Thank you!

huss · 2026-03-01T21:42:15Z

@kyle-parker-1500, @DestinyVeloz & @OscarAvilSal: First I want to apologize for how long my response has taken during an usual time. Second, I have started to review the work but I need more time to do it well. Please let me know if these are an issue.

kyle-parker-1500 · 2026-03-02T00:23:34Z

@huss don't worry about it and take your time with the review! My team and I are ready and able to fix any issues you find.

huss

Thanks to @kyle-parker-1500, @DestinyVeloz & @OscarAvilSal for this contribution. Again, I'm sorry my comments took so long. I've provided a number of comments to think about. Some are about code from earlier work as I reviewed this PR. Some are more general changes. Given this, if you don't want to address any of those you can leave a comment and OED can decide how to deal with them. Finally, I have yet to do careful testing of uploads (waiting for more finalized code) nor check that every index into the meter array is correct (didn't want you to wait while I do all of those and the method may change so doing that later may be bettert). Please let me know if anything is not clear, you have thoughts or I can help in any way.

huss · 2026-03-01T20:54:50Z

 const readCsv = require('../pipeline-in-progress/readCsv');
 const Unit = require('../../models/Unit');
-const { normalizeBoolean } = require('./validateCsvUploadParams');
+const { normalizeBoolean, MeterTimeSortTypesJS, MeterTimeSortTypesJS } = require('./validateCsvUploadParams');


Is MeterTimeSortTypesJS included twice?

huss · 2026-03-01T21:06:11Z

+			return { maxErrorMsg: msg, value: false };
+		}
+	}
+	return { maxErrorMsg: '', value: true };


I'm wondering about returning this as value: true in the case that it is a NaN above. It should be a number so should this case also fail? I think this applies to other new tests.

huss · 2026-03-01T21:34:18Z

+
+	if (areaUnit && areaUnit.toLowerCase() === 'none') {
+		if(!isNaN(areaValue) && areaValue !== 0) {
+      msg = `Invalid area value in row ${rowIndex + 1}: area="${meter[9]}". ` + `Area must be empty when area unit is 'none'.`;


The indenting seems off and is using spaces instead of tabs as OED wants.

I looks like the test is for a non-zero area value so I wonder if the msg should say that too.

huss · 2026-03-09T15:15:51Z

- * @param {express.Response} res 
+ * @param {express.Request} req
+ * @param {express.Response} res
+ * @param {express.Request} req


I'm wondering about listing req & res twice as params since they are once in the function. I don't think you made this change but is from the original PR. Thoughts?

huss · 2026-03-09T15:27:21Z

+ */
+function isValidTimeSort(timeSortValue) {
+	const validTimes = Object.values(MeterTimeSortTypesJS);
+	// must be one of the three values


(old code) This is not correct as there are currently two values. I would put "must be one of the allowed enum values."

huss · 2026-03-09T18:16:26Z

+function validateVariation(meter, rowIndex){
+	const variationValue = Number(meter[15]);
+	if(!isNaN(variationValue)){
+		if(variationValue < 0)


Please format so { is on same line as if statement.

huss · 2026-03-09T18:16:58Z

+	if(!isNaN(variationValue)){
+		if(variationValue < 0)
+		{
+			throw new CSVPipelineError(


Similar to other comment, this throws error inside check.

huss · 2026-03-09T18:17:35Z

+		{
+			throw new CSVPipelineError(
+			`Invalid variation value in row ${rowIndex + 1}: VariationValue="${meter[15]}". ` +
+			`Variation Value must be a number larger than 0.`,


Formally reading variation so should say that.

huss · 2026-03-09T18:18:37Z

+			const variationInput = meter[15];
+			if (variationInput) {
+				if (!validateVariation(variationInput)) {
+					let msg = `For meter ${meter[0]} the Gap entry of ${variationInput} is invalid. Gap must be a number greater than 0.`;


See other function about not returning value. Also, the msg should not be referring to gap.

huss · 2026-03-09T18:23:03Z

+ */
+function validateBooleanFields(meter, rowIndex) {
+	// all inputs that involve a true or false all being validated together.
+	const booleanFields = {


I like defining the index via a key. I'm thinking it would be better to define them all at the start to isolate this ugly fact and to make it clearer what is being accessed since it would be by kay name. Then all the rest of the code gets rid of the index value and uses the key. This is a new change so let me know your thoughts and if you want to do this.

SageMar and others added 30 commits November 27, 2024 15:51

added in the tests for area and meter type. These include a new funct…

187dde5

…ion for checking each value towards the bottom of the uploadMeters.js file as well as calling them when the submission is sent to the server

fixed issue within the added validation for area and meter type, also…

02712f0

… made sure the correct data was being sent back to the client side

Added validation for all boolean value types.

55db711

added in area unit type checking, still need to adjust to allow for d…

9722c00

…ifferent caps/lowercase

Removed old initial testing code.

67b8078

Merge pull request OpenEnergyDashboard#1 from cmatthews444/Issue1217-…

62288b6

…csv-validation Boolean value validations

Merge branch 'OpenEnergyDashboard:development' into Issue1217-csv-val…

034e7b4

…idation

added in a time sort check and fixed some linter errors

4f12d03

Add validation for minmax values

f4b60f3

tweaked timedout

5f30318

Merge pull request OpenEnergyDashboard#2 from cmatthews444/Issue1217-…

143a96d

…csv-validation Min max validation

Timezone validation completed

6c612b8

added time zone validation

30d213e

clarified error messages and removed trailing spaces. Working on debu…

f6b4c1c

…gging still.

Merge branch 'OpenEnergyDashboard:development' into Issue1217-csv-val…

06751f8

…idation

Updating to match current OED vers

45ae817

Merge branch 'Issue1217-csv-validation' of https://github.com/SageMar/OED into Issue1217-csv-validation

allow empty min/max values and booleanfields

7c317f5

Fixed errors in tests! It was an issue with the way area was being ch…

4d85514

…ecked, now it converts strings to ints and functions

fixed spacing + spelling, removed unused import, max value now being …

9b9e51d

…checked for validity before calling it to check min<max

Enums added where requested. Working on fixing area check now

ee6c066

Fixed issues with areaUnit validity check. Zero now returns false and…

f11ba17

… decimals are passing through

Timezone checks done using moment now

57f219d

Merge branch 'OpenEnergyDashboard:development' into Issue1217-csv-val…

454397d

…idation

added huss's logic and double checked it

96949fa

created validMaxError function (range 0-75)

33e9a15

Created validation for areaValue and areaUnit

4de6eb1

finished isValidDate() and validateYear() functions

2cc9c58

finished isDuplicate()

refactored isValidDate() and added correctDateTimeFormat()

c3b8129

corrected isValidDate() logic

final commit, Gap & Variation validation

c454a63

working on unit testing

21bfe0e

kyle-parker-1500 and others added 15 commits February 14, 2026 15:35

final changes to upload meters isValidDate, validateYear, correctDate…

d0186d7

…TimeFormat, isDuplicate before PR

removed unit testing file for isValidDate, validateYear, correctDateT…

4f020a9

…imeFormat, isDuplicate

Merge branch 'destiny' into final_checks_uploadMeters

3b777a7

calling functions for validateMaxError and validateArea

54c6820

Merge branch 'destiny' final changes into final_checks_uploadMeters

c625b76

final commit for validateMaxError and ValidateArea functions

83bbc46

Actual final commit for validateArea and validateMaxError

f23f363

Merge branch 'destiny' into final_checks_uploadMeters

f63f6df

Calls for Gap and Variation added

05aa418

Merge branch 'oscar' into final_checks_uploadMeters

0a1f366

final javadoc comments and changes before PR

6de3093

final javadoc comments and changes before PR

11c451c

Co-authored-by: Destiny Veloz <destinyveloz@msn.com> Co-authored-by: Oscar Aviles-Saldana <oaviles-saldana@csumb.edu>

Final changes to uploadMeters.

2b1b14a

Co-authored-by: Destiny Veloz destinyveloz@msn.com Co-authored-by: Oscar Aviles-Saldana oaviles-saldana@csumb.edu

Final changes to uploadMeters.

2ffbd38

Co-authored-by: Destiny Veloz <destinyveloz@msn.com> Co-authored-by: Oscar Aviles-Saldana <oaviles-saldana@csumb.edu>

Merge branch 'final_checks_uploadMeters' of https://github.com/OscarA…

ed57413

…vilSal/OED into final_checks_uploadMeters

huss requested changes Mar 9, 2026

View reviewed changes

huss mentioned this pull request May 20, 2026

WIP: Refactor Meter Validation (#1217) #1622

Draft

12 tasks

Conversation

kyle-parker-1500 commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Checklist

Limitations

Uh oh!

huss commented Feb 17, 2026

Uh oh!

kyle-parker-1500 commented Feb 19, 2026

Uh oh!

huss commented Mar 1, 2026

Uh oh!

kyle-parker-1500 commented Mar 2, 2026

Uh oh!

huss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kyle-parker-1500 commented Feb 15, 2026 •

edited

Loading