time varying conversion/ frontend #1516

minhtriet359 · 2025-08-06T23:47:36Z

Description

Finished implementing the UI changes required for the time-varying conversion feature (required backend changes are also included in this PR).

Fixes #896

Type of change

(Check the ones that apply by placing an "x" instead of the space in the [ ] so it becomes [x])

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

Checklist

(Note what you have done by placing an "x" instead of the space in the [ ] so it becomes [x]. It is hoped you do all of them.)

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

(Describe any issues that remain or work that should still be done.)

…wPage

…/hook_conversion_to_backend

…versionSegment route

…on conflict Signed-off-by: PRINCESANCHEZ <[email protected]>

…icts

…nfinity

huss

Thanks to @minhtriet359, @evan-carey & @PRINCESANCHEZ for the updates. I'm sorry this took so long as I figured out what to do and my attention was taken elsewhere. I've made a few new comments and some from before are still not resolved. I tried to resolve everything that I think was complete. If you are willing to either address the comment or say it will not be done by any of you then that would be great. I can then resolve all I think are fine and figure out if the PR can be merged. I appreciate any efforts after you have graduated. Please let me know if anything is not clear or I can help.

I also should have said that there are now merge conflicts.

src/client/app/components/conversion/EditConversionModalComponent.tsx

src/client/app/redux/api/conversionsApi.ts

Signed-off-by: PRINCESANCHEZ <[email protected]>

…s and pages Signed-off-by: PRINCESANCHEZ <[email protected]>

Signed-off-by: PRINCESANCHEZ <[email protected]>

huss

Thanks to @PRINCESANCHEZ for the update. I have now reviewed the updated code and tried to test it more systematically now that it is generally working. I've made around 10 new comments and a similar number of unresolved comments remain. At this point would it be possible to put a comment on each one (done, unsure about, do not want to do, ....) so they can be resolved in some way.

There are also merge conflicts that need careful resolution. Please let me know if you want me to try to handle them.

I welcome any thoughts or questions. Also let me know if I can help in any way. This is getting fairly close.

src/client/app/components/conversion/EditConversionModalComponent.tsx

huss · 2025-09-10T14:22:08Z

src/client/app/components/conversion/EditConversionModalComponent.tsx

 /* This Source Code Form is subject to the terms of the Mozilla Public
 * 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/. */
+


If I create a new conversion and then try to delete it, I see error message both in the web console and the terminal where I started OED. It has:

web-1 | [ERROR@2025-09-10T14:19:55.585+00:00] Unhandled Promise Rejection: Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
web-1 | Stacktrace:
web-1 | Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
web-1 | at new NodeError (node:internal/errors:405:5)
web-1 | at ServerResponse.setHeader (node:_http_outgoing:648:11)
web-1 | at ServerResponse.header (/usr/src/app/node_modules/express/lib/response.js:795:10)
web-1 | at ServerResponse.send (/usr/src/app/node_modules/express/lib/response.js:175:12)
web-1 | at success (/usr/src/app/src/server/routes/response.js:16:3)
web-1 | at /usr/src/app/src/server/routes/conversions.js:199:3
web-1 | at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
web-1 |
web-1 | (node:162) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 14)

The delete does not seem to happen (even though it is a success notification). I have not tried to figure out why.

huss · 2025-09-10T20:21:51Z

src/client/app/components/conversion/EditConversionSegmentModalComponent.tsx

+		}
+
+		if (segment.endTime !== 'infinity' && next) {
+			const endMatches = moment.utc(segment.endTime, 'YYYY-MM-DD HH:mm:ss').isSame(moment.utc(next.startTime));


If the end time of the edited segment must equal the start time of the next segment then it cannot be changed. If this is enforced then editing does not make sense. I think what is supposed to happen is that if you change the end time of the edited segment then the start time of the next segment must be changed to match that time. I believe this is what src/server/models/ConversionSegment.js does. However, the next segment's end time must be after the new start time. Effectively, the edited segment's end time must be before the next segment's end time. I think this is the desired check.

The same idea applies above for editing the start time.

I'm noting that I want to check if the start and end time of a segment can be edited once this is resolved.

I updated the logic so that editing a segment’s end or start time now also updates the neighboring segment accordingly. Added validation checks to enforce that the new end time is after the start, and the new start time is before the end

I'm getting server/DB errors when I violate the rule rather than client notices. Do you see this?

I was unable to reproduce this. When I enter invalid or overlapping start/end times and click save, no request is sent in the network tab, and the validation error shows in the modal.

We should discuss this to figure out what I am seeing as I saw it again.

src/client/app/components/conversion/EditConversionSegmentModalComponent.tsx

src/client/app/components/conversion/SplitConversionSegmentModalComponent.tsx

src/client/app/components/days/CreateDayModalComponent.tsx

src/client/app/components/conversion/SplitConversionSegmentModalComponent.tsx

src/client/app/redux/api/conversionSegmentsApi.ts

huss · 2025-09-11T18:40:40Z

I forgot to sweep all previous non-GitHub conversation records to make sure everything was resolved. Here are the few items I found that were not:

~~src/client/app/components/conversion/EditConversionSegmentModalComponent.tsx does not disable the save if no changes.~~ (resolved)
~~When you edit an overall conversion there is no toast msg if successful or not.~~ (resolved)
Changes in segments (split, delete, edit slope/intercept/pattern) will require a redo of cik. That issue is not yet being seen since that code is not yet in place for time-varying. However, the logic is needed. It would be easiest to do on every change but this process does take some time. Thus, it should use similar logic to src/client/app/components/unit/UnitsDetailComponent.tsx to show bouncing balls if RefreshingReadings is true. Note src/client/app/components/conversion/EditConversionModalComponent.tsx should also do the bouncing balls but it did and does not. This is a nontrivial change so we can discuss if it will be done now or made into an issue.

Signed-off-by: PRINCESANCHEZ <[email protected]>

…required attrs Signed-off-by: PRINCESANCHEZ <[email protected]>

Signed-off-by: PRINCESANCHEZ <[email protected]>

huss

Thanks to @PRINCESANCHEZ for quickly making updates to address my recent comments. I've updated a couple of comments and made a few new ones to consider. I have not yet looked at the ones with issues you are encountering.

I would also like to know if anyone on the team plans to address the remaining unresolved comments or if OED needs to find others to do this? Any thoughts or comments would be appreciated.

src/client/app/components/conversion/EditConversionSegmentModalComponent.tsx

src/client/app/components/conversion/SplitConversionSegmentModalComponent.tsx

huss · 2025-09-12T13:43:52Z

src/client/app/components/conversion/EditConversionSegmentModalComponent.tsx

+		}
+
+		if (segment.endTime !== 'infinity' && next) {
+			const endMatches = moment.utc(segment.endTime, 'YYYY-MM-DD HH:mm:ss').isSame(moment.utc(next.startTime));


I'm getting server/DB errors when I violate the rule rather than client notices. Do you see this?

…to-adjust Signed-off-by: PRINCESANCHEZ <[email protected]>

Signed-off-by: PRINCESANCHEZ <[email protected]>

huss

Thanks to @PRINCESANCHEZ for addressing more comments. I've made a few small comments to consider. I also noted it might be good to discuss the one error I am seeing but you are not.

I still want to understand the remaining unresolved items to figure out how to finalize this PR. Any insights would be appreciated.

huss · 2025-09-14T12:54:24Z

src/client/app/components/conversion/EditConversionSegmentModalComponent.tsx

+						},
+						originalStartTime: previous.startTime,
+						originalEndTime: previous.endTime
+					}).unwrap();


OED uses .then in the current code so I think that should be followed. Applies below too.

Updated to use .then/.catch for consistency

huss · 2025-09-14T13:04:53Z

src/client/app/components/conversion/EditConversionSegmentModalComponent.tsx

+		}
+
+		if (segment.endTime !== 'infinity' && next) {
+			const endMatches = moment.utc(segment.endTime, 'YYYY-MM-DD HH:mm:ss').isSame(moment.utc(next.startTime));


We should discuss this to figure out what I am seeing as I saw it again.

huss · 2025-09-14T13:08:45Z

src/client/app/components/conversion/EditConversionModalComponent.tsx

+											<td>{segment.startTime} to {segment.endTime}</td>
+											<td>{(segment.weekPatternsId ?? -99) === -99 ? segment.slope : ''}</td>
+											<td>{(segment.weekPatternsId ?? -99) === -99 ? segment.intercept : ''}</td>
+											<td>{weekPatterns.find(wp => wp.id === segment.weekPatternsId)?.name ?? 'No Pattern'}</td>


I just noticed that this is not translated. All the drop down menus for this do translate it.

updated to use translation string

src/client/app/components/conversion/SplitConversionSegmentModalComponent.tsx

Signed-off-by: PRINCESANCHEZ <[email protected]>

PRINCESANCHEZ and others added 30 commits July 23, 2025 11:06

Merge remote-tracking branch 'talia/taliaBranch' into edit-conversions

7a6e8ed

created UI for day overview page and create day modal

6eccbe3

Merge remote-tracking branch 'backend/taliaBranch' into triet_overvie…

f13fa05

…wPage

fixed db fail build error

745c094

hooked up day to backend

70f0da5

added check to make sure days cannot have the same name

24abf08

hooked conversion create modal - need to connect pattern

da3e438

Merge remote-tracking branch 'origin/evan-carey/weeks-api' into Triet…

7fba152

…/hook_conversion_to_backend

fix merge errors

69c2ebb

hooked up create conversion to backend

7bb4599

corrected insertData to insert into conversions and conversion segments

f09b4c7

made order of methods in models consistent

d3f5dab

revert changes to git.ignore and docker-compose.yml

e9bdc07

middleware for admin authentication

24020a5

consistent success messages from routes

2db8158

routes check id is an integer, updated validConversionSegments in con…

cdd8fc4

…versionSegment route

Merge timeVaryingConversion into edit-conversions: resolved translati…

91d3d23

…on conflict Signed-off-by: PRINCESANCHEZ <[email protected]>

start->startTime

d83c9ad

day_name -> name

f9a9f93

Merge Talia’s taliaBranch into edit-conversions after resolving confl…

cdec8e0

…icts

order conversion segments by start time

f2ec3f8

removed start and end time

850c96d

multi-value requests sent as post route instead of get

e47ed45

indentation spaces -> tabs

c2e769e

removed date-time type requirement, as it wouldn't accept -infinity/i…

89dec2f

…nfinity

delete also requires end time

83d8d78

start time and end time cannot be null

ee67bf6

delete conversion segment requires end time

4182304

edit requires name

7f8bc9b

require not null values for edit route, delete route only takes id

72f8fde

huss requested changes Sep 7, 2025

View reviewed changes

PRINCESANCHEZ added 11 commits September 8, 2025 10:16

Shorten reactstrap import and add modal divider in Days Edit

2babc47

Signed-off-by: PRINCESANCHEZ <[email protected]>

Merge branch 'edit-conversions' into evan-carey/days-edit

5a6d6c8

fix: Remove unused warning modal state when closing delete segment modal

d1dae3a

Signed-off-by: PRINCESANCHEZ <[email protected]>

Add validation to prevent split times outside segment range

685b5e9

Signed-off-by: PRINCESANCHEZ <[email protected]>

Merge branch 'edit-conversions' into evan-carey/days-edit

cb80349

Align modal table row count with other tables (20 rows)

82fe7f7

Signed-off-by: PRINCESANCHEZ <[email protected]>

Merge branch 'edit-conversions' into evan-carey/days-edit

3c8cf92

add default row count constant for consistent pagination across table…

52252c6

…s and pages Signed-off-by: PRINCESANCHEZ <[email protected]>

Merge branch 'edit-conversions' into evan-carey/days-edit

ac109a5

Update split flow to match day behavior

ad1b59e

Signed-off-by: PRINCESANCHEZ <[email protected]>

Merge branch 'edit-conversions' into evan-carey/days-edit

0102914

huss requested changes Sep 10, 2025

View reviewed changes

huss reviewed Sep 11, 2025

View reviewed changes

src/client/app/redux/api/conversionSegmentsApi.ts Outdated Show resolved Hide resolved

PRINCESANCHEZ and others added 8 commits September 11, 2025 14:25

Allow editing segment start/end with neighbor auto-update and validation

c8c849a

Signed-off-by: PRINCESANCHEZ <[email protected]>

refactor: improve datetime validation, clear errors on input, remove …

612799c

…required attrs Signed-off-by: PRINCESANCHEZ <[email protected]>

disable save when unchanged and add success/error notifications

bfaf3cd

Signed-off-by: PRINCESANCHEZ <[email protected]>

Merge branch 'edit-conversions' into evan-carey/days-edit

615b227

remove unused exports

c5ecc50

Signed-off-by: PRINCESANCHEZ <[email protected]>

Merge branch 'edit-conversions' into evan-carey/days-edit

2da5d3f

remove edit conversion warning

576c2ae

commenting change to address PR comment

26e7e0a

huss requested changes Sep 12, 2025

View reviewed changes

PRINCESANCHEZ added 4 commits September 12, 2025 12:47

Add error handling with showErrorNotification for neighbor segment au…

b8ef6b5

…to-adjust Signed-off-by: PRINCESANCHEZ <[email protected]>

Update split time placeholder to use complete and move to translations

c61046b

Signed-off-by: PRINCESANCHEZ <[email protected]>

add comments to explain git add . usage for invalid props

692ed35

Signed-off-by: PRINCESANCHEZ <[email protected]>

Use .then/.catch for neighbor segment adjustments

e2dd45f

Signed-off-by: PRINCESANCHEZ <[email protected]>

huss requested changes Sep 14, 2025

View reviewed changes

update pattern dropdown to use translation string for no pattern

94b223b

Signed-off-by: PRINCESANCHEZ <[email protected]>

time varying conversion/ frontend #1516

Are you sure you want to change the base?

time varying conversion/ frontend #1516

Uh oh!

Conversation

minhtriet359 commented Aug 6, 2025 • 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 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

huss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huss Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

huss commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

huss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

minhtriet359 commented Aug 6, 2025 •

edited

Loading

huss left a comment •

edited

Loading

huss Sep 10, 2025 •

edited

Loading

huss commented Sep 11, 2025 •

edited

Loading