This document tracks all TODO comments in the codebase, categorizing them by status and linking related GitHub issues. It serves as a reference for contributors and will help with existing issues that still need to be created and tracked.
Note: This list was last updated on 11/14/24, which is the last time the codebase was checked for TODO comments. When a new scan is completed, please update this date to reflect the most recent review.
This table lists the comments found in the Development branch of https://github.com/OpenEnergyDashboard/OED
The TODO comments here are grouped by which file they were found in, in order of when they appear
- Needs Issue Created - An issue needs to be created for this comment
- Related To An Issue - The comment is related to an issue
- Issue Is Created - An open issue has already been created for the comment
- Unsure - Needs more research
- No Issue Needed - This status may occur for different reasons. Try to describe it in the notes
For any TODO with an associated GitHub issue, link the issue number directly in the document. For example, Github issue #1191 will make it easier for contributors to find details.
Any important or useful notes can be listed in the notes column. If an issue is confusing or related to a separate issue and needs explaining, list it in the notes.
| # | TODO Comment | File | Status | Related Issue (If one exists already) | Notes |
|---|---|---|---|---|---|
| 1 | Remove this override once these components have been converted to use hooks | .eslintrc.json | Needs Issue Created | - | - |
| 2 | Remove this override once containers have been converted | .eslintrc.json | Needs Issue Created | - | - |
| 3 | This seems like a weird hack and there may be a better way to avoid the error that cannot write onto these directories. | .github/workflows/build.yml | Needs Issue Created | - | chmod 777 is being used to grant write permissions but it's a hack |
| 4 | Remove once final fix is in for GitHub workflows. | .github/workflows/build.yml | Issue Is Created | actions/runner#2033, and related issues: actions/checkout#1169, actions/checkout#1048, crytic/slither-action#50 | Waiting for fix for issue actions/runner#2033 before it can be removed |
| 5 | eventually move away and delete Dispatch Type entirely | src/client/app/store.ts | Needs Issue Created | - | Uses Dispatch type for older style thunks. Needs to eventually be removed |
| 6 | This is a function that deals with confirm/reject that may be useful in other places. | src/client/app/components/ConfirmActionModalComponent.tsx | Unsure | - | Listed in front of ConfirmActionModalComponent. TODO might be done already. |
| 7 | Re-implement AFTER RTK Migration hard-coded for the time being. Rework w/admin pages | src/client/app/components/HeaderButtonsComponent.tsx | Related To An Issue | Github issue #1191 & Github issue #1145 | RTK Migration Needed: unsavedChangesState is hardcoded until RTK Migration is finished. |
| 8 | Re-implement AFTER RTK Migration dispatch(unsavedWarningSlice.actions.flipLogOutState()); | src/client/app/components/HeaderButtonsComponent.tsx | Related To An Issue | Github issue #1191 & Github issue #1145 | RTK Migration Needed: Handle unsaved changes needs to be implemented after RTK Migration (Unsure if this is finished) |
| 9 | Compare is currently not ready for the custom option. | src/client/app/components/IntervalControlsComponent.tsx | Related To An Issue | Github issue #1293 | Custom Compare Interval doesn't work with ChartTypes yet |
| 10 | It might be better to do this similarly to compare. (See GitHub issue) | src/client/app/components/MapChartComponent.tsx | Unsure | - | Can't find the github issue mentioned. Unsure if an issue needs to be created or not |
| 11 | It might be better to do this similarly to compare. (See GitHub issue) | src/client/app/components/MapChartComponent.tsx | Unsure | - | Can't find the github issue mentioned. Unsure if an issue needs to be created or not |
| 12 | Using the following (if (size.length > 0) ) seems to have no impact on the code. It has been noticed that this function is called many times for each change. Someone should look at why that is happening and why some have no items in the arrays. | src/client/app/components/MapChartComponent.tsx | Needs Issue Created | - | Someone needs to investigate this bug to see why it is occurring |
| 13 | As also noted above, this component is rerendering multiple times. This is causing the information message to appear multiple times. This needs to be figured out. Debugging indicates that this happens when selecting a meter/group not recently used so the data is not in Redux store. Each time it has readingsData as undefined so it does not process that meter. Need to see how can avoid this. | src/client/app/components/MapChartComponent.tsx | Needs Issue Created | - | Similar to the above comment, someone needs to investigate this bug to see why it is occurring |
| 14 | Must be internationalized. | src/client/app/components/MapChartComponent.tsx | Needs Issue Created | - | Notification string needs to be added to translationData in src/client/app/translations/data.ts ("All values are negative so the circle sizes act as if value range was positive which may change the relative sizes."). After string is in translationData, useTranslate can be used in notification message |
| 15 | Must be internationalized. | src/client/app/components/MapChartComponent.tsx | Needs Issue Created | - | Notification string needs to be added to translationData in src/client/app/translations/data.ts ("Some values are close to zero so the small circle sizes may be a little larger to be visible") After string is in translationData, useTranslate can be used in notification message |
| 16 | Must be internationalized. Same as message above. | src/client/app/components/MapChartComponent.tsx | Needs Issue Created | - | Notification string needs to be added to translationData in src/client/app/translations/data.ts ("Some values are close to zero so the small circle sizes may be a little larger to be visible") After string is in translationData, useTranslate can be used in notification message |
| 17 | Must be internationalized. | src/client/app/components/MapChartComponent.tsx | Needs Issue Created | - | Notification string needs to be added to translationData in src/client/app/translations/data.ts ("All values are negative so the circle sizes act as if value range was positive which may change the relative sizes.") After string is in translationData, useTranslate can be used in notification message |
| 18 | When this is converted to RTK then should use useAppDispatch(). | src/client/app/components/MapChartSelectComponent.tsx | Related To An Issue | Github issue #1145 | RTK Migration Needed: After migration, useAppDispatch() should be used instead of the current useDispatch() |
| 19 | would be nice if relevant message was derived from uiSelectors, which currently only tracks / trims non-compatible ids | src/client/app/components/MeterAndGroupSelectComponent.tsx | Needs Issue Created | - | The tooltip message for disabled items is currently hardcoded as "help.home.area.normalize". To dynamically generate more relevant messages, uiSelectors needs to be extended to include a function for retrieving specific disable reasons based on component state. |
| 20 | Add meta data along chain? i.e. disabled due to chart type, area norm... etc. and display relevant message. | src/client/app/components/MeterAndGroupSelectComponent.tsx | Needs Issue Created | - | Once tooltip disabled reason/messages can extracted from uiSelectors, it should be added as META data in order to be shown to the user |
| 21 | Verify behavior, and set proper message/ translate | src/client/app/components/MeterAndGroupSelectComponent.tsx | Needs Issue Created | - | Once tooltip META data is added to MultiValueLabel ensure the message is shown correctly as well as Internationalized |
| 22 | SEEMS UNUSED, kept due to uncertainty when migrating to RTK VERIFY BEHAVIOR | src/client/app/components/MultiCompareChartComponent.tsx | Unsure | - | const errorEntities: string[] is initialized, but nothing is added to the array. MultiCompareChartComponent.tsx returns a row of errorEntities mapped to div columns, but this never shows since nothing is added to the array. Most likely can be removed. |
| 23 | These two blocks for meters & groups could probably be combined. | src/client/app/components/MultiCompareChartComponent.tsx | Needs Issue Created | - | meterReading entities and groupReading entities are pushed to the selectedCompareEntities array in two separate forEach loops, but a single function could be used for both reading arrays. Looks like a low priority task |
| 24 | These types of plotly containers expect a lot of passed values and it gives a TS error. Given we plan to replace this code anyway, we are not fixing the error. | src/client/app/components/MultiCompareChartComponent.tsx | Issue Is Created | Github issue #1195 addresses CompareChartContainer react hook upgrade. Github issue #888 addressed the Plotly old style props | CompareChartContainer TS errors for plotly old style props (mapStateToProps) are currently being suppressed and do not cause any errors. CompareChartContainer still needs to be replaced with a react hook before this TODO can be removed |
| 25 | If we are sure the data is always defined then remove this commented out code. Be consistent for all graphing and groups below. | src/client/app/components/RadarChartComponent.tsx | No Issue Needed | - | Used to throw an error when readingsData.readings was undefined, but it looks like someone added a const readings = values(readingsData) from 'lodash', so even if the data is null or undefined, the values() will return an empty array. If no data is returned, rData will just be an empty array and nothing will be plotted. Comment should be okay to remove |
| 26 | See 3D code for functions that can be used for layout and notices. | src/client/app/components/RadarChartComponent.tsx | Needs Issue Created | - | RadarChartComponent.tsx could use an upgrade for how it sets its layouts. ThreeDComponent.tsx contains two functions(setHelpLayout and setThreeDLayout) that could be implemented similarly in RadarChartComponent.tsx |
| 27 | Attempts to format the dates to remove the time did not work with plotly choosing the tick values which is desirable. Also want time if limited time range. | src/client/app/components/RadarChartComponent.tsx | Unsure | - | This one may need a new issue if it is still desired to have the time removed from the date format while viewing the Radar Chart. If the time is removed from the radar chart values, an option to include them back in would be needed if the user has selected a limited time range |
| 28 | Error Component needs to be implemented, Its currently a bare bones placeholder | src/client/app/components/RouteComponent.tsx | No Issue Needed | - | This looks like a duplicate TODO to make a better ErrorComponent. There is already a TODO in src/client/app/components/router/ErrorComponent.tsx. This TODO should be able to be removed |
| 29 | Styling for the component, may need to be converted into .css files | src/client/app/components/ThreeDPillComponent.tsx | Related To An Issue | Github issue #1203 Will decide how CSS is implemented across all pages | This should be decided in Issue 1203. |
| 30 | ISSUE when many meters selected they are cut off. | src/client/app/components/ThreeDPillComponent.tsx | Needs Issue Created | - | Pill Components go below the container and out of view when many meters are selected and the width of the screen is below a certain point. Allowing vertical scroll here could be a potential fix. |
| 31 | Provide default link when there are issues fetching version number | src/client/app/components/TooltipHelpComponent.tsx | Needs Issue Created | - | If there is a problem with fetching the OED version, Tooltip links will display "..." in place of the <a> tag link. A default link could be used instead of the "..." |
| 32 | This is trying to fix the fact that when the rate menu appears and disappears and you don't go to a new page then the help pop up does not occur. This works reasonably well but you have to click the help icon twice to see the pop up in this case. A better fix would be desirable in the long term since there have been issues with tooltips in multiple places. | src/client/app/components/TooltipMarkerComponent.tsx | Related To An Issue | Github issue #988 | Issue 988 react-tooltip upgrade from v4 to v5 may solve this problem |
| 33 | migrate ReactRouter v6 & hooks | src/client/app/components/UnsavedWarningComponent.tsx | Unsure | - | It looks like react-router has already been upgraded to v6. Unsure if there are specific hooks that were in mind that still need to be implemented but this seems okay. React-router does suggest not blocking users requests and to try to use a different method |
| 36 | Add warning for invalid data | src/client/app/components/admin/PreferencesComponent.tsx | Needs Issue Created | - | If preferencesApi.useGetPreferencesQuery() has a problem fetching data, an error may be thrown while trying to set preferences. |
| 37 | looks kind of bad make a better visible notification | src/client/app/components/conversion/CreateConversionModalComponent.tsx | Needs Issue Created | - | The error reason message in the footer of the modal needs an update to how it looks while showing the reason of the conversion error |
| 38 | 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. | src/client/app/components/csv/ReadingsCSVUploadComponent.tsx | No Issue Needed | - | This feature was added to help a site import data. It works but can have issues. Thus, it is only allowed for script uploading since that was the need. It was 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. |
| 39 | isValidGPSInput currently pops up an alert so not doing it here, may change so leaving code commented out. | src/client/app/components/groups/CreateGroupModalComponent.tsx | Unsure | - | isValidGPSInput currently returns the showErrorNotification, so the showErrorNotification was commented out to avoid having 2 of the same error messages being returned. |
| 40 | We should probably go all to modal or popups for messages. | src/client/app/components/groups/EditGroupModalComponent.tsx | Needs Issue Created | - | - |
| 41 | isValidGPSInput currently pops up an alert so not doing it here, may change so leaving code commented out. | src/client/app/components/groups/EditGroupModalComponent.tsx | Unsure | - | isValidGPSInput currently returns the showErrorNotification, so the showErrorNotification was commented out to avoid having 2 of the same error messages being returned. |
| 42 | This component still displays a dropdown arrow, even though a user cannot use the dropdown | src/client/app/components/groups/EditGroupModalComponent.tsx | Needs Issue Created | - | This form group has an input field, with a 'select' type. This shows a dropdown arrow, but the user cannot use it (not sure if the user shouldn't use it or if it's broken and the user can't use it because of that). |
| 43 | The following line should work but does not (it does for meters). The Redux state has the name of hidden groups but it should not. A quick fix is to check if the name is null. This occurs because the group tree code is passing groups even if not displayable. | src/client/app/components/groups/EditGroupModalComponent.tsx | Needs Issue Created | - | - |
| 44 | These types of plotly containers expect a lot of passed values and it gives a TS error. Given we plan to replace this with the react hooks version and it does not seem to cause any issues, this TS error is being suppressed for now. | src/client/app/components/maps/MapCalibrationComponent.tsx | No Issue Needed | - | Typescript is causing an issue because of a lot of values being passed, react hook versions planned to be used, typescript error currently suppressed. I don't believe an issue is needed as it'll be fixed when the react hook versions are used. |
| 45 | Maybe should do as a single popup? | src/client/app/components/meters/CreateMeterModalComponent.tsx | Needs Issue Created | - | This refers to the fact that the checks are done separately so the messages are popped up by each one. Ideally, the tests would return the message and then the component would display when desired. These checks are used in mulitple places so requires a number of changes to do this. |
| 46 | should this state be used for the meterState above or would that cause issues? | src/client/app/components/meters/EditMeterModalComponent.tsx | Needs Issue Created | - | Might need testing, seems like meterDataByID could be used for meterState but it might cause issues |
| 47 | isValidGPSInput currently tops up an alert so not doing it here, may change so leaving code commented out. | src/client/app/components/meters/EditMeterModalComponent.tsx | Unsure | - | This TODO is present in both the EditMeterModalComponent and the CreateMeterModalComponent, it comments out the showErrorNotification because the call to isValidGPSInput already returns a showErrorNotification |
| 48 | Would like line break between messages. See below on issue. | src/client/app/components/meters/EditMeterModalComponent.tsx | Needs Issue Created | - | Needs a line break between the different messages inside the error_message. Should be simple, add a "\n" |
| 49 | Attempts to add a line break with \n, <br />, etc. failed when using showErrorNotification. This is going to be a general problem. See fkhadra/react-toastify#687 | src/client/app/components/meters/EditMeterModalComponent.tsx | No Issue Needed | - | The issue for the link provided is closed now, but I'm not sure if the issue was actually fixed. It said at the end of it that the user @vitaliyhayda added a line to the toastify code that would fix the issue, but there isn't a clear statement that it's been fixed. I don't think the issue is related to the OED dashboard, it needs to be fixed by the Toastify github repo group. |
| 50 | make a good looking error page. This is a placeholder for now. | src/client/app/components/router/ErrorComponent.tsx | Needs Issue Created | - | Not seeing any issues for this, seems like the error page needs some visual improvements |
| 51 | VALIDATE HOTLINK | src/client/app/components/router/GraphLinkComponent.tsx | Needs Issue Created | - | URLSearchParams need link validation |
| 52 | see EditMeterModalComponent for issue with line breaks. Same issue in strings below. | src/client/app/components/unit/EditUnitModalComponent.tsx | Needs Issue Created | - | Has the same line break issues as the EditMeterModalComponent |
| 53 | It might be better to do this similarly to compare. (See GitHub issue) | src/client/app/containers/MapChartContainer.ts | Unsure | - | Can't find the github issue mentioned. Unsure if an issue needs to be created or not |
| 54 | It might be better to do this similarly to compare. (See GitHub issue) | src/client/app/containers/MapChartContainer.ts | Unsure | - | Can't find the github issue mentioned. Unsure if an issue needs to be created or not |
| 55 | Using the following seems to have no impact on the code. It has been noticed that this function is called many times for each change. Someone should look at why that is happening and why some have no items in the arrays. | src/client/app/containers/MapChartContainer.ts | Needs Issue Created | - | size.length > 0 doesn't seem to be working as expected |
| 56 | The max circle diameter should come from admin/DB. | src/client/app/containers/MapChartContainer.ts | Needs Issue Created | - | - |
| 57 | Should be env variable? | src/client/app/redux/api/baseApi.ts | Unsure | - | - |
| 58 | Validate Behavior against all endpoints | src/client/app/redux/api/baseApi.ts | Needs Issue Created | - | Currently no validation in place for the endpoints |
| 59 | write more robust logic for error handling, and manually invalidate tags instead? | src/client/app/redux/api/conversionsApi.ts | Needs Issue Created | - | the deleteConversion function needs to have error handling and behavior verification, this is also true for the editConversion below it in the code |
| 60 | Verify Behavior w/ Maintainers | src/client/app/redux/api/conversionsApi.ts | Related To An Issue | - | related to the error/handling and behavior verification |
| 61 | write more robust logic for error handling, and manually invalidate tags instead? | src/client/app/redux/api/conversionsApi.ts | Related To An Issue | - | related to the error/handling and behavior verification |
| 62 | Verify Behavior w/ Maintainers | src/client/app/redux/api/conversionsApi.ts | Related To An Issue | - | related to the error/handling and behavior verification |
| 63 | Some of these can go away when we make the menus dynamic. | src/client/app/redux/selectors/adminSelectors.ts | Needs Issue Created | - | Didn't find any issues that mentioned this behavior in the admin section |
| 64 | If a meter in a group is not visible to this user then it is not in Redux state and this fails. | src/client/app/redux/selectors/uiSelectors.ts | Needs Issue Created | - | No issues mentioning this problem, it seems that this section fails as certain meters in a group are not visible, leading to them not being in Redux state. |
| 65 | Omitted for the time being re-implement slider range later. | src/client/app/redux/selectors/uiSelectors.ts | Issue Is Created | Github issue #853 | I think that this TODO might be part of this already created issue, the actual slider range code is commented out and mentions that it needs to be re-implemented later, perhaps is the reason behind the issues with the slider range. |
| 66 | Under development so in future | src/client/app/redux/slices/graphSlice.ts | Needs Issue Created | - | This involves getting a more user friendly default when no time range is selected by the user. It needs to be done at some point. |
| 67 | Currently only tracks when on 3d, Verify that this is the desired behavior | src/client/app/redux/slices/graphSlice.ts | Needs Issue Created | - | Needs a decision made, if the intended behavior is for it to track only when 3d is on, then this TODO is done and can be closed. |
| 68 | Needs to be refactored into a single dispatch/reducer pair. | src/client/app/redux/slices/graphSlice.ts | Needs Issue Created | - | The switch needs to be refactored into a single reducer to reduce dispatch calls. |
| 69 | validation could be implemented across all cases similar to compare period and sorting order | src/client/app/redux/slices/graphSlice.ts | Related To An Issue | - | related to the previous TODO, some of the cases need validation in place. ComparePeriod and compareSortingOrder both have validation, so everything else needs to be similar to them. |
| 70 | omitted for now re-implement later. | src/client/app/redux/slices/graphSlice.ts | Needs Issue Created | - | Wasn't able to find any issues related to this, but it seems like someone did work on this and decided to omit it for the time being. Not sure but this might have to do with the range Sliders issues, |
| 71 | The new line readings route for graphs allows one to get the raw data. Maybe we should try to switch to that and then modify this code to use the unix timestamp that is returned. It is believed that the unix timestamp will be smaller than this string. | src/client/app/redux/thunks/exportThunk.ts | Needs Issue Created | - | Didn't find any issues related to this. This should definitely be looked at as implementing the switch to unix timestamp will reduce data payload and increase performance |
| 72 | find better approach? | src/client/app/styles/DateRangeCustom.css | Needs Issue Created | - | there seems to be issues with the dataRange width sizing. The min-width is currently set to auto, but this might yield inconsistent sizing depending on the input, could add a defined base size to have a consistent size |
| 73 | with area? you get a TS error but without it lets null through (see web console). | src/client/app/types/redux/groups.ts | Needs Issue Created | - | Type script error produced from the area attribute. |
| 74 | this duplicates two fields in ones above so decide if should somehow merge. | src/client/app/types/redux/groups.ts | Needs Issue Created | - | GroupData and GroupChildren both use the childMeters, childGroups attributes. This might lead to an inconsistency, if the values ever differ in each interface. Either merging them or creating a base interface could solve this issue. |
| 75 | This function does not appear to be used - should it be removed? | src/client/app/utils/calculateCompare.ts | Needs Issue Created | - | Should be a simple issue, needs a decision made whether to keep this function in or not. There are no implementations or calls to this function anywhere, so it's currently un-used |
| 76 | It would be nice to tell user that comma is missing but need to check all uses to be sure don't get ''. | src/client/app/utils/calibration.ts | Needs Issue Created | - | Currently only returns false, and doesn't notify the user that there needs to be a comma. This definitely should be looked at as just returning false might cause users to be confused. |
| 77 | It would be nice to return the error and then notify as desired. | src/client/app/utils/calibration.ts | Unsure | - | This TODO actually displays a showErrorNotification, but the showErrorNotification doesn't actually return the exact error. It would seem that this TODO wants the actual error to be returned as well. Not sure. |
| 78 | Is it okay to use the one point or would an average be better? | src/client/app/utils/calibration.ts | No Issue Needed | - | Needs a decision to be made regarding whether the one point or average is better for this. Averaging could improve accuracy, but using a one point is still sufficient I believe. If average is decided to be better than this will most likely need an issue created. It currently seems to be working so doesn't need an issue. |
| 79 | it is a bad practice to import store anywhere other than index.tsx These utils need to be converted into selectors. | src/client/app/utils/determineCompatibleUnits.ts | Needs Issue Created | - | This seems like something that would be nice to have. May be a large undertaking if being implemented, so creating its own issue is suggested. |
| 80 | should be internationalized | src/client/app/utils/exportData.ts | Needs Issue Created | - | While there are other issues regarding internationalization, this todo seems unique and does not fit into existing issues |
| 81 | This isn't a general solution. For example, Wh or W would not be fixed. The y-axis label is the kW. | src/client/app/utils/graphics.ts | Needs Issue Created | - | - |
| 82 | BANDAID FIX Application wasn't loading due to store.getState() returning undefined after adding call to translation in GraphicRateMenuComponent | src/client/app/utils/translate.ts | Needs Issue Created | - | Not related to any other translation-related issues. May need its own issue created. |
| 83 | Its a bad practice to import store anywhere other than index.tsx migrate to useTranslate() from componentHooks.ts | src/client/app/utils/translate.ts | Needs Issue Created | - | See line 80 |
| 84 | This should be moved to the error case below once issues with this process are under control. If all is well it could go inside the case where an unknown error occurred. | src/scripts/installOED.sh | Needs Issue Created | - | - |
| 85 | Verify that user see the message returned, see https://express-rate-limit.mintlify.app/reference/configuration#message | src/server/app.js | Issue Is Created | Github issue #1206 | Directly related to issue 1206 |
| 86 | This was causing tests to fail for 3D rejection. This limit seems to be okay but we should find a better solution than upping values just for tests. | src/server/app.js | Needs Issue Created | - | Could not find any related issues. Underlying issues seems to be with the automatic test |
| 87 | Potentially modify the Morgan logger to use the log API, thus unifying all our logging. | src/server/app.js | Needs Issue Created | - | No related issues |
| 88 | Check if valid env variables are actually loaded despite the lack of a file, only log if they are not | src/server/config.js | Needs Issue Created | - | TODO is self-explanatory. No implementation in code yet. Needs an issue created. |
| 89 | These likely need to be updated. | src/server/data/automatedTestingData.js | Unsure | - | References a series of commands to remove all the readings and meters associated with the test data. This is valuable if you want to run the process again and the meters and readings already exist. No related issues found. |
| 90 | When this is converted to key/value pairs, the area should be set to the comparable meter(s) and campus is the sum of all 121,000. | src/server/data/websiteData.js | Unsure | - | - |
| 92 | Someday consider averaging days if too many. | src/server/migrations/0.8.0-1.0.0/sql/readings/create_reading_views.sql | Unsure | - | This is a multifaceted issue that requires more guidance. How many days is too many days? How do we want to go about averaging this data? |
| 94 | This function can probably be improved for two reasons: 1) While it was noted that you don't look at too many days, it does limit its usage to modest time lengths. | src/server/migrations/1.0.0-2.0.0/sql/readings/create_function_get_compare_readings.sql | Needs Issue Created | - | TODO message goes into detail about what should be done. This 100% needs to be its own issue |
| 95 | Someday consider averaging days if too many. | src/server/migrations/1.0.0-2.0.0/sql/readings/create_reading_views.sql | Unsure | - | This is a multifaceted issue that requires more guidance. How many days is too many days? How do we want to go about averaging this data? |
| 96 | This should be a transaction to avoid issues for any request made to the database. | src/server/models/Cik.js | Needs Issue Created | - | Using a transaction to remove and insert all elements of the conversion array will keep the database stable in case it goes wrong. |
| 97 | This will be removed once we completely transition to the unit version. | src/server/models/Reading.js | Unsure | - | Have we transitioned to the unit version? |
| 98 | Returns a special value if it doesn't exist | src/server/models/Unit.js | Unsure | - | Change return type of getByName function to something special if the unit name does not exist. Not sure what something special is. |
| 99 | Allowing for backwards compatibility if previous eGauge meters are using the 'email' parameter instead of the 'username' parameter to login. Developers need to decide in the future if we should deprecate this behavior. | src/server/models/eGauge/eGaugeRequestor.js | Needs Issue Created | - | I feel this todo ties into the larger issue of making a choice as to what needs to be deprecated in regards to backwards compatibility features. |
| 100 | what is the 's' parameter for? I removed it and it still works. | src/server/models/eGauge/eGaugeRequestor.js | Needs Issue Created | - | No related issues found. |
| 101 | in the future with units we should use the raw meter value and have it converted when graphing. We also need to deal with other unit types. | src/server/models/eGauge/eGaugeRequestor.js | Unsure | - | This should probably be split into two separate issues. The first being to allow for raw meter data to be handled at the moment of graphing. The second being to allow for unit types other than "P" |
| 102 | We should allow the admin to decide the frequency of readings acquired. | src/server/models/eGauge/eGaugeRequestor.js | Needs Issue Created | - | Frequency of readings acquired is currently hardcoded to be 900 |
| 103 | We should check when and why Obvius sends us the same meter twice. It might be the case that we need to update the meter in this case. | src/server/models/obvius/Configfile.js | Needs Issue Created | - | - |
| 104 | the unit name needs to come from the config file | src/server/models/obvius/processConfigFile.js | Needs Issue Created | - | Unit lookup is currently hardcoded to "kwh" |
| 105 | need a warning log | src/server/models/obvius/processConfigFile.js | Unsure | - | No related issues found. Implementation likely depends on completion of to TODO directly above. |
| 106 | Allowing for backwards compatibility if any users are still using the 'email' parameter instead of the 'username' parameter to login. Developers need to decide in the future if we should deprecate this behavior. | src/server/routes/csv.js | Needs Issue Created | - | I feel this todo ties into the larger issue of making a choice as to what needs to be deprecated in regards to backwards compatibility features. |
| 107 | This is not using the success function since it needs to return values. At some point we probably should fuse the success and returning values. | src/server/routes/meters.js | Needs Issue Created | - | formatMeterForResponse function should have the return values merged with the success function values. |
| 108 | This is not using the success function since it needs to return values. At some point we probably should fuse the success and returning values. | src/server/routes/meters.js | Needs Issue Created | - | formatMeterForResponse function should have the return values merged with the success function values. |
| 109 | This is allowing for backwards compatibility if previous obvius meters are using the'email' parameter instead of the 'username' parameter to login. Developers need to decide in the future if we should deprecate this behavior. | src/server/routes/obvius.js | Needs Issue Created | - | I feel this todo ties into the larger issue of making a choice as to what needs to be deprecated in regards to backwards compatibility features. |
| 110 | Do not have minLength: 8 because this is optional. Nice if could check if present. | src/server/routes/users.js | Needs Issue Created | - | Minimum password length of 8 is not enforced, but it would be nice to check if the entered password is at least 8 characters. Perhaps as a warning to the user. |
| 111 | maybe we should cache this somewhere so only do once. | src/server/services/meterTimezone.js | Needs Issue Created | - | When determining a timezone for a meter, if the selected meter has no timezone, a call is made to get the default timezone being used. Perhaps the default timezone should be cached somewhere to prevent these calls from having to be made. |
| 112 | get the unit when the MAMAC meter is first probed and created. For now, we assume they are kWh as before resource generalization. | src/server/services/readMamacMeters.js | Needs Issue Created | - | getMeterInfo currently has the meter unit hardcoded as "kWh". Need to fetch the unit data from the meter itself. No related issues found. |
| 113 | Allowing for backwards compatibility if any users are still using the 'meterName' parameter instead of 'meter'. Developers need to decide in the future if we should deprecate this behavior. | src/server/services/csvPipeline/uploadReadings.js | Unsure | - | TODO comment is self explanatory. This is something that I am not qualified to decide |
| 114 | We made the assumption that in the DB, the cumulative and cumulativeReset columns (and maybe other boolean ones) is either true or false. | src/server/services/csvPipeline/uploadReadings.js | Needs Issue Created | - | Likely requires further testing to see how null values affect the CSV upload pipeline |
| 115 | All of this is less relevant when using the webapp, because it would load these external runtime defaults for the user. | src/server/services/csvPipeline/validateCsvUploadParams.js | Unsure | - | TODO is basically saying that there are some default parameters in the CSV upload pipeline that should be loaded externally for the user. This has not yet been implemented and could probably be an issue. |
| 116 | We use strings to represent the true/false values of an option. The pipeline should really not be doing checks against raw strings, but instead normalize the inputs first. | src/server/services/csvPipeline/validateCsvUploadParams.js | Needs Issue Created | - | TODO comment is self explanatory in associated file |
| 117 | Allowing for backwards compatibility to allow for curl users to use the 'email' parameter instead of 'username'. Developers need to decide in the future if we should deprecate this behavior. | src/server/services/csvPipeline/validateCsvUploadParams.js | Unsure | - | I feel this todo ties into the larger issue of making a choice as to what needs to be deprecated in regards to backwards compatibility features. |
| 118 | If the system is getting other types of meters this may cause the refresh to happen multiple times. Might want to work on this in the future. | src/server/services/eGauge/updateEgaugeMeters.js | Needs Issue Created | - | No related issues found. |
| 119 | The table in the database for the logical Cik needs to be wiped and these values stored. This code will be added once the database table for using it to get readings is set. | src/server/services/graph/createConversionArrays.js | Needs Issue Created | - | No implementation in code yet. Needs an issue created. |
| 120 | We have an issue that this will not return all the values unless admin but we have a CSV login. | src/server/services/pipeline-in-progress/processData.js | Needs Issue Created | - | No related issues found. |
| 121 | fix this up so better. | src/server/services/pipeline-in-progress/processData.js | Needs Issue Created | - | Need to fix redundant code & declare the variables once. |
| 122 | This is a big hack because we don't do inDst for this case so reset. Similar to issue above. | src/server/services/pipeline-in-progress/processData.js | Unsure | - | This problem is related to the one above I think an issue would best. |
| 123 | Ensure that no cycles are inserted into the graph. This will likely require a BEFORE INSERT trigger. | src/server/sql/group/create_groups_tables.sql | Needs Issue Created | - | No related issues found. |
| 124 | Ensure that the graph structure is actually a multitree. This will probably require a BEFORE INSERT trigger. | src/server/sql/group/create_groups_tables.sql | Needs Issue Created | - | - |
| 125 | Deal with parent meters that are installed after their children. They only shadow them from a start-date onwards. The above to-do is probably going to require a significant reworking of some stuff. | src/server/sql/group/create_groups_tables.sql | Needs Issue Created | - | - |
| 126 | Can we optimize the code so this is not needed or the slowdown is less? | src/server/sql/reading/create_function_get_3d_readings.sql | Unsure | - | - |
| 127 | This function can probably be improved for two reasons: 1) While it was noted that you don't look at too many days, it does limit its usage to modest time lengths. There has been thought to allowing comparisons, for example, or a whole year. It also assumes that the frequency of readings is not too high so the number of readings looked at could be large even over modest time frames. 2) Readings are only included if they are within the current time period. This means that if you have a reading that crosses the time frame they are excluded. This can be viewed as either a good thing or a bad thing. However, if the frequency of readings is high, then large segments of time can be excluded. For example, monthly readings would not be includes in either day, week or four week comparisons that OED currently does. Also note that the daily and hourly views do include readings that span a time frame so including them would be consistent. | src/server/sql/reading/create_function_get_compare_readings.sql | Needs Issue Created | - | - |
| 128 | Check if needed and when to use as not done for hourly. | src/server/sql/reading/create_reading_views.sql | Unsure | - | - |
| 129 | Someday consider averaging days if too many. | src/server/sql/reading/create_reading_views.sql | Needs Issue Created | - | - |
| 130 | Deal with conflicts due to overlapping date ranges here. | src/server/sql/reading/insert_or_update_reading.sql | Unsure | - | - |
| 131 | Move logging disabling to a better place. | src/server/test/common.js | Needs Issue Created | - | centralize the logging configuration to avoid scattering logging settings within test files. set up logging configurations specifically for the test environment in a configuration file |
| 132 | needs to be tested | src/server/test/rateTesting.sh | Needs Issue Created | - | no related issue found |
| 133 | example for getting raw readings. | src/server/test/rateTesting.sh | Needs Issue Created | - | - |
| 134 | If used note this does not have the more recent meter values. | src/server/test/db/baselineTests.js | Needs Issue Created | - | no related issues found |
| 135 | test readings, units. | src/server/test/db/compareTests.js | Needs Issue Created | - | no related issues found |
| 136 | Each of these tests generate one or more warning that say: "kWh not found while processing Obvius data" | src/server/test/db/configfileTests.js | Needs Issue Created | - | inserting a "kWh" unit into the database before running the tests should help after moving the warning message |
| 137 | add 2 new unit parameters for meters. | src/server/test/db/meterTests.js | Needs Issue Created | - | no related issue found |
| 138 | save until sure not needed. | src/server/test/db/meterTests.js | Needs Issue Created | - | no related issue found |
| 139 | add tests that check flow readings. | src/server/test/db/unitReadingsTests.js | Needs Issue Created | - | no related issues found |
| 140 | This test no longer does what is desired because so few readings will return the 1 raw point. Until we have an interface to allow setting the frequency desired this test is commented out. | src/server/test/db/unitReadingsTests.js | Needs Issue Created | - | - |
| 141 | Test infinite range with bounded timestamp to ensure proper shrink | src/server/test/db/unitReadingsTests.js | Needs Issue Created | - | No related issue found |
| 142 | modify so checks values too. | src/server/test/db/unitReadingsTests.js | Needs Issue Created | - | No related issue found |
| 143 | modify so checks values too. | src/server/test/db/unitReadingsTests.js | Unsure | - | - |
| 144 | check values and daily/hourly readings. | src/server/test/db/unitReadingsTests.js | Needs Issue Created | - | - |
| 145 | groups too | src/server/test/db/unitReadingsTests.js | Unsure | - | - |
| 146 | is this actually used anywhere? | src/server/test/routes/unitReadingsRouteTests.js | Unsure | - | This function isn't used elsewhere |
| 147 | Maybe check for invalid for each value in validateLineReadingsQueryParams (also in Bar below). | src/server/test/routes/unitReadingsRouteTests.js | Needs Issue Created | - | Needs additional test for checking if its invalid, no related issues found. |
| 148 | Maybe check for invalid for each value in validateLineReadingsQueryParams (also in Bar below). | src/server/test/routes/unitReadingsRouteTests.js | Unsure | - | same as above |
| 149 | Maybe check for invalid for each value in validateLineReadingsQueryParams (also in Bar below). | src/server/test/routes/unitReadingsRouteTests.js | Unsure | - | same as 148 |
| 150 | It would be nice if this was not an eval. Tried a function with closure but could not get it to work as did not find chai. | src/server/test/web/csvPipelineTest.js | Needs Issue Created | - | replace the evalString with a direct call to chaiRequest. |
| 151 | It would be nice if this was not an eval. Tried a function with closure but could not get it to work as did not find chai. | src/server/test/web/csvPipelineTest.js | Unsure | - | same as above |
| 152 | It would be nice to make this use the code in src/server/test/db/meterTests.js and make all meter tests use one common function for meter comparison. | src/server/test/web/csvPipelineTest.js | Needs Issue Created | - | no related issue found |
| 153 | Why is there an empty body here? | src/server/test/web/maps.js | Unsure | - | exists as a placeholder, possibly to add setup steps for each test in the future. |
| 154 | These tests are not as good as they should be now that information on meters is returned to all users. They should be updated. | src/server/test/web/meters.js | Needs Issue Created | - | no related issue found |
| 155 | Unsure why this check for the file existing does not work. Only delete if it exists. | src/server/util/insertData.js | Needs Issue Created | - | no related issue found |
| 156 | make into if/else: new mapping function and add else for error Create loadGeneratedInput and call it here | src/server/util/insertData.js | Needs Issue Created | - | no related issue found |